commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From ozeigerm...@apache.org
Subject cvs commit: jakarta-commons-sandbox/transaction/src/java/org/apache/commons/transaction/memory OptimisticMapWrapper.java TransactionalMapWrapper.java
Date Sun, 06 Jun 2004 12:36:29 GMT
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


Mime
View raw message