You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@commons.apache.org by oz...@apache.org on 2004/06/06 14:36:29 UTC
cvs commit: jakarta-commons-sandbox/transaction/src/java/org/apache/commons/transaction/memory OptimisticMapWrapper.java TransactionalMapWrapper.java
ozeigermann 2004/06/06 05:36:29
Modified: transaction/src/java/org/apache/commons/transaction/memory
OptimisticMapWrapper.java
TransactionalMapWrapper.java
Log:
Removed unnecessary syncrhonized blocks to increase concurrency
Revision Changes Path
1.7 +17 -17 jakarta-commons-sandbox/transaction/src/java/org/apache/commons/transaction/memory/OptimisticMapWrapper.java
Index: OptimisticMapWrapper.java
===================================================================
RCS file: /home/cvs/jakarta-commons-sandbox/transaction/src/java/org/apache/commons/transaction/memory/OptimisticMapWrapper.java,v
retrieving revision 1.6
retrieving revision 1.7
diff -u -r1.6 -r1.7
--- OptimisticMapWrapper.java 5 Jun 2004 16:42:11 -0000 1.6
+++ OptimisticMapWrapper.java 6 Jun 2004 12:36:29 -0000 1.7
@@ -27,6 +27,7 @@
import java.util.Iterator;
import java.util.Map;
import java.util.Set;
+import java.util.Collections;
/**
* Wrapper that adds transactional control to all kinds of maps that implement the {@link Map} interface. By using
@@ -35,7 +36,7 @@
*
* <br>
* Start a transaction by calling {@link #startTransaction()}. Then perform the normal actions on the map and
- * finally either calls {@link #commitTransaction()} to make your changes permanent or {@link #rollbackTransaction()} to
+ * finally either call {@link #commitTransaction()} to make your changes permanent or {@link #rollbackTransaction()} to
* undo them.
* <br>
* <em>Caution:</em> Do not modify values retrieved by {@link #get(Object)} as this will circumvent the transactional mechanism.
@@ -73,10 +74,10 @@
*/
public OptimisticMapWrapper(Map wrapped, MapFactory mapFactory, SetFactory setFactory) {
super(wrapped, mapFactory, setFactory);
- activeTransactions = new HashSet();
+ activeTransactions = Collections.synchronizedSet(new HashSet());
}
- public synchronized void startTransaction() {
+ public void startTransaction() {
if (getActiveTx() != null) {
throw new IllegalStateException(
"Active thread " + Thread.currentThread() + " already associated with a transaction!");
@@ -86,17 +87,17 @@
setActiveTx(context);
}
- public synchronized void rollbackTransaction() {
+ public void rollbackTransaction() {
TxContext txContext = getActiveTx();
super.rollbackTransaction();
activeTransactions.remove(txContext);
}
- public synchronized void commitTransaction() throws ConflictException {
+ public void commitTransaction() throws ConflictException {
commitTransaction(false);
}
- public synchronized void commitTransaction(boolean force) throws ConflictException {
+ public void commitTransaction(boolean force) throws ConflictException {
TxContext txContext = getActiveTx();
if (txContext == null) {
@@ -115,10 +116,12 @@
}
}
- copyChangesToConcurrentTransactions();
-
- super.commitTransaction();
- activeTransactions.remove(txContext);
+ // be sure commit is atomic
+ synchronized (this) {
+ activeTransactions.remove(txContext);
+ copyChangesToConcurrentTransactions();
+ super.commitTransaction();
+ }
}
// TODO: Shouldn't we return a collection rather than a single key here?
@@ -142,9 +145,6 @@
for (Iterator it = activeTransactions.iterator(); it.hasNext();) {
CopyingTxContext otherTxContext = (CopyingTxContext) it.next();
-
- if (otherTxContext == thisTxContext)
- continue;
// no need to copy data if the other transaction does not access global map anyway
if (otherTxContext.cleared)
1.18 +81 -74 jakarta-commons-sandbox/transaction/src/java/org/apache/commons/transaction/memory/TransactionalMapWrapper.java
Index: TransactionalMapWrapper.java
===================================================================
RCS file: /home/cvs/jakarta-commons-sandbox/transaction/src/java/org/apache/commons/transaction/memory/TransactionalMapWrapper.java,v
retrieving revision 1.17
retrieving revision 1.18
diff -u -r1.17 -r1.18
--- TransactionalMapWrapper.java 6 Jun 2004 11:37:46 -0000 1.17
+++ TransactionalMapWrapper.java 6 Jun 2004 12:36:29 -0000 1.18
@@ -25,6 +25,7 @@
import java.util.ArrayList;
import java.util.Collection;
+import java.util.Collections;
import java.util.HashSet;
import java.util.Iterator;
import java.util.Map;
@@ -54,15 +55,15 @@
*/
public class TransactionalMapWrapper implements Map, Status {
- /** The map wrapped. */
+ /** The map wrapped. */
protected Map wrapped;
- /** Factory to be used to create temporary maps for transactions. */
+ /** Factory to be used to create temporary maps for transactions. */
protected MapFactory mapFactory;
- /** Factory to be used to create temporary sets for transactions. */
+ /** Factory to be used to create temporary sets for transactions. */
protected SetFactory setFactory;
- private ThreadLocal activeTx = new ThreadLocal();
+ private ThreadLocal activeTx = new ThreadLocal();
/**
* Creates a new transactional map wrapper. Temporary maps and sets to store transactional
@@ -83,7 +84,7 @@
* @param setFactory factory for temporary sets
*/
public TransactionalMapWrapper(Map wrapped, MapFactory mapFactory, SetFactory setFactory) {
- this.wrapped = wrapped;
+ this.wrapped = Collections.synchronizedMap(wrapped);
this.mapFactory = mapFactory;
this.setFactory = setFactory;
}
@@ -94,7 +95,7 @@
* @return <code>true</code> if no write opertation has been performed inside the current transaction,
* <code>false</code> otherwise
*/
- public synchronized boolean isReadOnly() {
+ public boolean isReadOnly() {
TxContext txContext = getActiveTx();
if (txContext == null) {
@@ -115,7 +116,7 @@
* @return <code>true</code> if this transaction has been marked for a roll back
* @see #markTransactionForRollback()
*/
- public synchronized boolean isTransactionMarkedForRollback() {
+ public boolean isTransactionMarkedForRollback() {
TxContext txContext = getActiveTx();
if (txContext == null) {
@@ -126,12 +127,12 @@
return (txContext.status == Status.STATUS_MARKED_ROLLBACK);
}
- /**
- * Marks the current transaction to allow only a rollback as valid outcome.
- *
- * @see #isTransactionMarkedForRollback()
- */
- public synchronized void markTransactionForRollback() {
+ /**
+ * Marks the current transaction to allow only a rollback as valid outcome.
+ *
+ * @see #isTransactionMarkedForRollback()
+ */
+ public void markTransactionForRollback() {
TxContext txContext = getActiveTx();
if (txContext == null) {
@@ -139,25 +140,25 @@
"Active thread " + Thread.currentThread() + " not associated with a transaction!");
}
- txContext.status = Status.STATUS_MARKED_ROLLBACK;
+ txContext.status = Status.STATUS_MARKED_ROLLBACK;
}
- /**
- * Suspends the transaction associated to the current thread. I.e. the associated between the
- * current thread and the transaction is deleted. This is useful when you want to continue the transaction
- * in another thread later. Call {@link #resumeTransaction(TxContext)} - possibly in another thread than the current -
- * to resume work on the transaction.
- * <br><br>
- * <em>Caution:</em> When calling this method the returned identifier
- * for the transaction is the only remaining reference to the transaction, so be sure to remember it or
- * the transaction will be eventually deleted (and thereby rolled back) as garbage.
- *
- * @return an identifier for the suspended transaction, will be needed to later resume the transaction by
- * {@link #resumeTransaction(TxContext)}
- *
- * @see #resumeTransaction(TxContext)
- */
- public synchronized TxContext suspendTransaction() {
+ /**
+ * Suspends the transaction associated to the current thread. I.e. the associated between the
+ * current thread and the transaction is deleted. This is useful when you want to continue the transaction
+ * in another thread later. Call {@link #resumeTransaction(TxContext)} - possibly in another thread than the current -
+ * to resume work on the transaction.
+ * <br><br>
+ * <em>Caution:</em> When calling this method the returned identifier
+ * for the transaction is the only remaining reference to the transaction, so be sure to remember it or
+ * the transaction will be eventually deleted (and thereby rolled back) as garbage.
+ *
+ * @return an identifier for the suspended transaction, will be needed to later resume the transaction by
+ * {@link #resumeTransaction(TxContext)}
+ *
+ * @see #resumeTransaction(TxContext)
+ */
+ public TxContext suspendTransaction() {
TxContext txContext = getActiveTx();
if (txContext == null) {
@@ -165,19 +166,19 @@
"Active thread " + Thread.currentThread() + " not associated with a transaction!");
}
- txContext.suspended = true;
- setActiveTx(null);
+ txContext.suspended = true;
+ setActiveTx(null);
return txContext;
}
- /**
- * Resumes a transaction in the current thread that has previously been suspened by {@link #suspendTransaction()}.
- *
- * @param suspendedTx the identifier for the transaction to be resumed, delivered by {@link #suspendTransaction()}
- *
- * @see #suspendTransaction()
- */
- public synchronized void resumeTransaction(TxContext suspendedTx) {
+ /**
+ * Resumes a transaction in the current thread that has previously been suspened by {@link #suspendTransaction()}.
+ *
+ * @param suspendedTx the identifier for the transaction to be resumed, delivered by {@link #suspendTransaction()}
+ *
+ * @see #suspendTransaction()
+ */
+ public void resumeTransaction(TxContext suspendedTx) {
if (getActiveTx() != null) {
throw new IllegalStateException(
"Active thread " + Thread.currentThread() + " already associated with a transaction!");
@@ -186,21 +187,21 @@
if (suspendedTx == null) {
throw new IllegalStateException("No transaction to resume!");
}
-
+
if (!suspendedTx.suspended) {
- throw new IllegalStateException("Transaction to resume needs to be suspended!");
+ throw new IllegalStateException("Transaction to resume needs to be suspended!");
}
- suspendedTx.suspended = false;
+ suspendedTx.suspended = false;
setActiveTx(suspendedTx);
}
- /**
- * Returns the state of the current transaction.
- *
- * @return state of the current transaction as decribed in the {@link Status} interface.
- */
- public synchronized int getTransactionState() {
+ /**
+ * Returns the state of the current transaction.
+ *
+ * @return state of the current transaction as decribed in the {@link Status} interface.
+ */
+ public int getTransactionState() {
TxContext txContext = getActiveTx();
if (txContext == null) {
@@ -214,14 +215,14 @@
* thread made to the map are invisible from other threads until {@link #commitTransaction()} is called.
* Use {@link #rollbackTransaction()} to discard your changes. After calling either method there will be
* no transaction associated to the current thread any longer.
- * <br><br>
+ * <br><br>
* <em>Caution:</em> Be careful to finally call one of those methods,
* as otherwise the transaction will lurk around for ever.
*
* @see #commitTransaction()
* @see #rollbackTransaction()
*/
- public synchronized void startTransaction() {
+ public void startTransaction() {
if (getActiveTx() != null) {
throw new IllegalStateException(
"Active thread " + Thread.currentThread() + " already associated with a transaction!");
@@ -236,7 +237,7 @@
* @see #startTransaction()
* @see #commitTransaction()
*/
- public synchronized void rollbackTransaction() {
+ public void rollbackTransaction() {
TxContext txContext = getActiveTx();
if (txContext == null) {
@@ -256,7 +257,7 @@
* @see #startTransaction()
* @see #rollbackTransaction()
*/
- public synchronized void commitTransaction() {
+ public void commitTransaction() {
TxContext txContext = getActiveTx();
if (txContext == null) {
@@ -277,10 +278,10 @@
// Map methods
//
- /**
- * @see Map#clear()
- */
- public synchronized void clear() {
+ /**
+ * @see Map#clear()
+ */
+ public void clear() {
TxContext txContext = getActiveTx();
if (txContext != null) {
txContext.clear();
@@ -292,7 +293,7 @@
/**
* @see Map#size()
*/
- public synchronized int size() {
+ public int size() {
TxContext txContext = getActiveTx();
if (txContext != null) {
return txContext.size();
@@ -304,7 +305,7 @@
/**
* @see Map#isEmpty()
*/
- public synchronized boolean isEmpty() {
+ public boolean isEmpty() {
TxContext txContext = getActiveTx();
if (txContext == null) {
return wrapped.isEmpty();
@@ -316,14 +317,14 @@
/**
* @see Map#containsKey(java.lang.Object)
*/
- public synchronized boolean containsKey(Object key) {
+ public boolean containsKey(Object key) {
return (get(key) != null);
}
/**
* @see Map#containsValue(java.lang.Object)
*/
- public synchronized boolean containsValue(Object value) {
+ public boolean containsValue(Object value) {
TxContext txContext = getActiveTx();
if (txContext == null) {
@@ -336,7 +337,7 @@
/**
* @see Map#values()
*/
- public synchronized Collection values() {
+ public Collection values() {
TxContext txContext = getActiveTx();
@@ -348,7 +349,10 @@
for (Iterator it = keySet().iterator(); it.hasNext();) {
Object key = it.next();
Object value = get(key);
- values.add(value);
+ // XXX we have no isolation, so get entry might have been deleted in the meantime
+ if (value != null) {
+ values.add(value);
+ }
}
return values;
}
@@ -357,7 +361,7 @@
/**
* @see Map#putAll(java.util.Map)
*/
- public synchronized void putAll(Map map) {
+ public void putAll(Map map) {
TxContext txContext = getActiveTx();
if (txContext == null) {
@@ -373,7 +377,7 @@
/**
* @see Map#entrySet()
*/
- public synchronized Set entrySet() {
+ public Set entrySet() {
TxContext txContext = getActiveTx();
if (txContext == null) {
return wrapped.entrySet();
@@ -383,7 +387,10 @@
for (Iterator it = keySet().iterator(); it.hasNext();) {
Object key = it.next();
Object value = get(key);
- entrySet.add(new HashEntry(key, value));
+ // XXX we have no isolation, so get entry might have been deleted in the meantime
+ if (value != null) {
+ entrySet.add(new HashEntry(key, value));
+ }
}
return entrySet;
}
@@ -392,7 +399,7 @@
/**
* @see Map#keySet()
*/
- public synchronized Set keySet() {
+ public Set keySet() {
TxContext txContext = getActiveTx();
if (txContext == null) {
@@ -405,7 +412,7 @@
/**
* @see Map#get(java.lang.Object)
*/
- public synchronized Object get(Object key) {
+ public Object get(Object key) {
TxContext txContext = getActiveTx();
if (txContext != null) {
@@ -418,7 +425,7 @@
/**
* @see Map#remove(java.lang.Object)
*/
- public synchronized Object remove(Object key) {
+ public Object remove(Object key) {
TxContext txContext = getActiveTx();
if (txContext == null) {
@@ -433,7 +440,7 @@
/**
* @see Map#put(java.lang.Object, java.lang.Object)
*/
- public synchronized Object put(Object key, Object value) {
+ public Object put(Object key, Object value) {
TxContext txContext = getActiveTx();
if (txContext == null) {
---------------------------------------------------------------------
To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-dev-help@jakarta.apache.org