[jboss-cvs] JBossCache/src/org/jboss/cache ...

[Brian Stansberry]

  User: bstansberry
  Date: 06/12/07 22:49:28

  Modified:    src/org/jboss/cache  TransactionEntry.java
  Log:
  [JBCACHE-874] Store locks in a LinkedHashSet
  
  Revision  Changes    Path
  1.16      +22 -24    JBossCache/src/org/jboss/cache/TransactionEntry.java
  
  (In the diff below, changes in quantity of whitespace are not shown.)
  
  Index: TransactionEntry.java
  ===================================================================
  RCS file: /cvsroot/jboss/JBossCache/src/org/jboss/cache/TransactionEntry.java,v
  retrieving revision 1.15
  retrieving revision 1.16
  diff -u -b -r1.15 -r1.16
  --- TransactionEntry.java	7 Dec 2006 20:03:23 -0000	1.15
  +++ TransactionEntry.java	8 Dec 2006 03:49:28 -0000	1.16
  @@ -32,7 +32,7 @@
    * </ul>
    *
    * @author <a href="mailto:bela at jboss.org">Bela Ban</a> Apr 14, 2003
  - * @version $Revision: 1.15 $
  + * @version $Revision: 1.16 $
    */
   public class TransactionEntry {
   
  @@ -59,9 +59,12 @@
      private List undo_list=new LinkedList();
   
      /**
  -    * List<IdentityLock> of locks acquired by the transaction ({@link IdentityLock})
  +    * LinkedHashSet<IdentityLock> of locks acquired by the transaction. We use
  +    * a LinkedHashSet because we need efficient Set semantics (same lock can
  +    * be added multiple times) but also need guaranteed ordering for use
  +    * by lock release code (see JBCCACHE-874).
       */
  -   private List locks = new LinkedList();
  +   private LinkedHashSet locks = new LinkedHashSet();
   
      /**
       * A list of dummy uninitialised nodes created by the cache loader interceptor to load data for a
  @@ -132,18 +135,15 @@
      }
   
      /**
  -    * Adds a lock to the end of the lock list.
  +    * Adds a lock to the end of the lock list, if it isn't already present.
       */
      public void addLock(NodeLock l) {
         if(l != null) {
            synchronized(locks) {
  -             if (!locks.contains(l))
  -             {
                    locks.add(l);
                }
            }
         }
  -   }
   
      /**
       * Add multiple locks to the lock list.
  @@ -151,25 +151,22 @@
       */
      public void addLocks(Collection newLocks) {
         if(newLocks != null) {
  -         IdentityLock tmp;
            synchronized(locks) {
  -            for(Iterator it=newLocks.iterator(); it.hasNext();) {
  -               tmp=(IdentityLock)it.next();
  -                if (!locks.contains(tmp))
  -                {
  -                    locks.add(tmp);
  -                }
  -            }
  +            locks.addAll(newLocks);
            }
         }
      }
   
      /**
       * Returns the locks in use.
  -    * Note:  This list may be concurrently modified.
  +    * 
  +    * @return a defensive copy of the internal data structure.
       */
      public List getLocks() {
  -      return locks;
  +      synchronized(locks)
  +      {
  +         return new ArrayList(locks);
  +      }
      }
   
      /**
  @@ -188,16 +185,17 @@
       * Clears the list of locks held.
       */
      public void releaseAllLocksLIFO(Object owner) {
  -      // I guess a copy would work as well
  -      // This seems fairly safe though
  +
         synchronized (locks) {
  -         for (ListIterator i = locks.listIterator(locks.size()); i.hasPrevious();) {
  -            IdentityLock lock = (IdentityLock)i.previous();
  +         // Copying out to an array is faster than creating an ArrayList and iterating,
  +         // since list creation will just copy out to an array internally
  +         IdentityLock[] lockArray = (IdentityLock[]) locks.toArray(new IdentityLock[locks.size()]);
  +         for (int i = lockArray.length -1; i >= 0; i--) {
                if (log.isTraceEnabled())
                {
  -                 log.trace("releasing lock for " + lock.getFqn() + " (" + lock + ")");
  +                 log.trace("releasing lock for " + lockArray[i].getFqn() + " (" + lockArray[i] + ")");
                }
  -             lock.release(owner);
  +             lockArray[i].release(owner);
            }
            locks.clear();
         }