[jbosscache-commits] JBoss Cache SVN: r7274 - in core/branches/flat/src: main/java/org/jboss/starobrno/interceptors and 2 other directories.

jbosscache-commits at lists.jboss.org jbosscache-commits at lists.jboss.org
Wed Dec 10 07:51:42 EST 2008


Author: manik.surtani at jboss.com
Date: 2008-12-10 07:51:42 -0500 (Wed, 10 Dec 2008)
New Revision: 7274

Modified:
   core/branches/flat/src/main/java/org/jboss/starobrno/context/EntryLookup.java
   core/branches/flat/src/main/java/org/jboss/starobrno/context/InvocationContextImpl.java
   core/branches/flat/src/main/java/org/jboss/starobrno/context/TransactionContextImpl.java
   core/branches/flat/src/main/java/org/jboss/starobrno/interceptors/LockingInterceptor.java
   core/branches/flat/src/test/java/org/jboss/starobrno/api/mvcc/read_committed/ReadCommittedLockTest.java
   core/branches/flat/src/test/java/org/jboss/starobrno/profiling/TreeProfileTest.java
Log:
Perf improvements + added tests

Modified: core/branches/flat/src/main/java/org/jboss/starobrno/context/EntryLookup.java
===================================================================
--- core/branches/flat/src/main/java/org/jboss/starobrno/context/EntryLookup.java	2008-12-10 11:04:37 UTC (rev 7273)
+++ core/branches/flat/src/main/java/org/jboss/starobrno/context/EntryLookup.java	2008-12-10 12:51:42 UTC (rev 7274)
@@ -41,5 +41,7 @@
 
    void putLookedUpEntries(Map<Object, MVCCEntry> lookedUpEntries);
 
+   void removeLookedUpEntry(Object key);
+
    void clearLookedUpEntries();
 }

Modified: core/branches/flat/src/main/java/org/jboss/starobrno/context/InvocationContextImpl.java
===================================================================
--- core/branches/flat/src/main/java/org/jboss/starobrno/context/InvocationContextImpl.java	2008-12-10 11:04:37 UTC (rev 7273)
+++ core/branches/flat/src/main/java/org/jboss/starobrno/context/InvocationContextImpl.java	2008-12-10 12:51:42 UTC (rev 7274)
@@ -28,9 +28,13 @@
 import org.jboss.starobrno.container.MVCCEntry;
 import org.jboss.starobrno.transaction.GlobalTransaction;
 import org.jboss.starobrno.transaction.TransactionTable;
+import org.jboss.starobrno.util.FastCopyHashMap;
 
 import javax.transaction.Transaction;
-import java.util.*;
+import java.util.Collections;
+import java.util.LinkedHashSet;
+import java.util.List;
+import java.util.Map;
 
 /**
  * // TODO: MANIK: Document this
@@ -59,15 +63,15 @@
     * once we drop support for opt/pess locks we can genericise this to contain Fqns. - Manik Surtani, June 2008
     */
    protected LinkedHashSet<Object> invocationLocks;
-   private HashMap<Object, MVCCEntry> lookedUpEntries = null;
+   private FastCopyHashMap<Object, MVCCEntry> lookedUpEntries = null;
 
    /**
     * Retrieves a node from the registry of looked up nodes in the current scope.
     * <p/>
-    * If a transaction is in progress, implementations should delegate to {@link org.jboss.cache.transaction.MVCCTransactionContext#lookUpNode(Fqn)}
+    * If a transaction is in progress, implementations should delegate to {@link org.jboss.starobrno.context.TransactionContext#lookupEntry(Object)}
     * <p/>
     *
-    * @param fqn fqn to look up
+    * @param k key to look up
     * @return a node, or null if it cannot be found.
     */
    public MVCCEntry lookupEntry(Object k)
@@ -82,10 +86,22 @@
       }
    }
 
+   public void removeLookedUpEntry(Object key)
+   {
+      if (transactionContext != null)
+      {
+         transactionContext.removeLookedUpEntry(key);
+      }
+      else
+      {
+         if (lookedUpEntries != null) lookedUpEntries.remove(key);
+      }
+   }
+
    /**
     * Puts an entry in the registry of looked up nodes in the current scope.
     * <p/>
-    * If a transaction is in progress, implementations should delegate to {@link org.jboss.cache.transaction.MVCCTransactionContext#putLookedUpNode(Fqn, NodeSPI)}
+    * If a transaction is in progress, implementations should delegate to {@link TransactionContext#putLookedUpEntry(Object, org.jboss.starobrno.container.MVCCEntry)}
     * <p/>
     *
     * @param key
@@ -96,7 +112,7 @@
          transactionContext.putLookedUpEntry(key, e);
       else
       {
-         if (lookedUpEntries == null) lookedUpEntries = new HashMap<Object, MVCCEntry>(4);
+         if (lookedUpEntries == null) lookedUpEntries = new FastCopyHashMap<Object, MVCCEntry>(4);
          lookedUpEntries.put(key, e);
       }
    }
@@ -108,7 +124,7 @@
       else
       {
          if (this.lookedUpEntries == null)
-            this.lookedUpEntries = new HashMap<Object, MVCCEntry>();
+            this.lookedUpEntries = new FastCopyHashMap<Object, MVCCEntry>();
 
          this.lookedUpEntries.putAll(lookedUpEntries);
       }
@@ -142,7 +158,7 @@
    {
       InvocationContextImpl copy = new InvocationContextImpl();
       doCopy(copy);
-      if (lookedUpEntries != null) copy.lookedUpEntries = (HashMap<Object, MVCCEntry>) lookedUpEntries.clone();
+      if (lookedUpEntries != null) copy.lookedUpEntries = (FastCopyHashMap<Object, MVCCEntry>) lookedUpEntries.clone();
       return copy;
    }
 
@@ -314,9 +330,6 @@
     * Note that if a transaction is in scope, implementations should record this lock on the {@link org.jboss.cache.transaction.TransactionContext}.
     * Using this method should always ensure that the appropriate scope is used.
     * <p/>
-    * Note that currently (as of 3.0.0) this lock is weakly typed.  This is to allow support for both MVCC (which uses {@link Fqn}s as locks)
-    * as well as legacy Optimistic and Pessimistic Locking schemes (which use {@link org.jboss.cache.lock.NodeLock} as locks).  Once support for
-    * legacy node locking schemes are dropped, this method will be more strongly typed to accept {@link Fqn}.
     *
     * @param lock lock to add
     */
@@ -342,9 +355,6 @@
     * Note that if a transaction is in scope, implementations should remove this lock from the {@link org.jboss.cache.transaction.TransactionContext}.
     * Using this method should always ensure that the lock is removed from the appropriate scope.
     * <p/>
-    * Note that currently (as of 3.0.0) this lock is weakly typed.  This is to allow support for both MVCC (which uses {@link Fqn}s as locks)
-    * as well as legacy Optimistic and Pessimistic Locking schemes (which use {@link org.jboss.cache.lock.NodeLock} as locks).  Once support for
-    * legacy node locking schemes are dropped, this method will be more strongly typed to accept {@link Fqn}.
     *
     * @param lock lock to remove
     */
@@ -369,9 +379,6 @@
     * Note that if a transaction is in scope, implementations should clear locks from the {@link org.jboss.cache.transaction.TransactionContext}.
     * Using this method should always ensure locks are cleared in the appropriate scope.
     * <p/>
-    * Note that currently (as of 3.0.0) this lock is weakly typed.  This is to allow support for both MVCC (which uses {@link Fqn}s as locks)
-    * as well as legacy Optimistic and Pessimistic Locking schemes (which use {@link org.jboss.cache.lock.NodeLock} as locks).  Once support for
-    * legacy node locking schemes are dropped, this method will be more strongly typed to accept {@link Fqn}.
     */
    public void clearKeysLocked()
    {
@@ -408,7 +415,7 @@
    }
 
    /**
-    * @return true if options exist to suppress locking - false otherwise.  Note that this is only used by the {@link org.jboss.cache.interceptors.PessimisticLockInterceptor}.
+    * @return true if options exist to suppress locking - false otherwise.
     */
    public boolean isLockingSuppressed()
    {

Modified: core/branches/flat/src/main/java/org/jboss/starobrno/context/TransactionContextImpl.java
===================================================================
--- core/branches/flat/src/main/java/org/jboss/starobrno/context/TransactionContextImpl.java	2008-12-10 11:04:37 UTC (rev 7273)
+++ core/branches/flat/src/main/java/org/jboss/starobrno/context/TransactionContextImpl.java	2008-12-10 12:51:42 UTC (rev 7274)
@@ -26,6 +26,7 @@
 import org.jboss.starobrno.config.Option;
 import org.jboss.starobrno.container.MVCCEntry;
 import org.jboss.starobrno.transaction.GlobalTransaction;
+import org.jboss.starobrno.util.FastCopyHashMap;
 
 import javax.transaction.RollbackException;
 import javax.transaction.SystemException;
@@ -77,7 +78,7 @@
     */
    private List<Object> removedNodes = null;
 
-   private final Map<Object, MVCCEntry> lookedUpEntries = new HashMap<Object, MVCCEntry>(8);
+   private final FastCopyHashMap<Object, MVCCEntry> lookedUpEntries = new FastCopyHashMap<Object, MVCCEntry>(8);
    private GlobalTransaction gtx;
 
    public TransactionContextImpl(Transaction tx) throws SystemException, RollbackException
@@ -100,6 +101,11 @@
       return lookedUpEntries.get(key);
    }
 
+   public void removeLookedUpEntry(Object key)
+   {
+      lookedUpEntries.remove(key);
+   }
+
    /**
     * Puts an entry in the registry of looked up nodes in the current scope.
     * <p/>
@@ -107,7 +113,7 @@
     * would delegate to this method if a transaction is in scope.
     * <p/>
     *
-    * @param key
+    * @param key key to put
     */
    public void putLookedUpEntry(Object key, MVCCEntry entry)
    {
@@ -290,7 +296,7 @@
    public String toString()
    {
       StringBuilder sb = new StringBuilder();
-      sb.append("TransactionContext (" + System.identityHashCode(this) + ") nmodificationList: ").append(modificationList);
+      sb.append("TransactionContext (").append(System.identityHashCode(this)).append(") nmodificationList: ").append(modificationList);
       return sb.toString();
    }
 

Modified: core/branches/flat/src/main/java/org/jboss/starobrno/interceptors/LockingInterceptor.java
===================================================================
--- core/branches/flat/src/main/java/org/jboss/starobrno/interceptors/LockingInterceptor.java	2008-12-10 11:04:37 UTC (rev 7273)
+++ core/branches/flat/src/main/java/org/jboss/starobrno/interceptors/LockingInterceptor.java	2008-12-10 12:51:42 UTC (rev 7274)
@@ -22,6 +22,7 @@
 package org.jboss.starobrno.interceptors;
 
 import org.jboss.cache.lock.IsolationLevel;
+import org.jboss.starobrno.commands.TransactionBoundaryCommand;
 import org.jboss.starobrno.commands.VisitableCommand;
 import org.jboss.starobrno.commands.read.GetKeyValueCommand;
 import org.jboss.starobrno.commands.read.GravitateDataCommand;
@@ -39,10 +40,10 @@
 import org.jboss.starobrno.interceptors.base.PrePostProcessingCommandInterceptor;
 import org.jboss.starobrno.lock.LockManager;
 
-import java.util.HashSet;
+import java.util.HashMap;
 import java.util.List;
 import java.util.ListIterator;
-import java.util.Set;
+import java.util.Map;
 
 /**
  * Interceptor to implement <a href="http://wiki.jboss.org/wiki/JBossCacheMVCC">MVCC</a> functionality.
@@ -213,23 +214,18 @@
       }
       else
       {
-         if (useReadCommitted)
+         if (useReadCommitted && !(command instanceof TransactionBoundaryCommand))
          {
-            if (trace) log.trace("Wiping unnecessary entries from context");
-            // wipe all unchanged entries from context, to force subsequent reads to go to the container,
-            // hence providing R_C semantics.
-            Set<Object> keysToRemove = new HashSet<Object>();
-            for (MVCCEntry e : ctx.getLookedUpEntries().values())
+            Map<Object, MVCCEntry> original = ctx.getLookedUpEntries();
+            if (!original.isEmpty())
             {
-               if (!e.isChanged()) keysToRemove.add(e.getKey());
+               Map<Object, MVCCEntry> defensiveCopy = new HashMap<Object, MVCCEntry>(original);
+               for (Map.Entry<Object, MVCCEntry> e : defensiveCopy.entrySet())
+               {
+                  if (!e.getValue().isChanged()) ctx.removeLookedUpEntry(e.getKey());
+               }
             }
-            for (Object k : keysToRemove) ctx.removeKeyLocked(k);
          }
-         else
-         {
-            if (trace) log.trace("Nothing to do since there is a transaction in scope.");
-         }
-
       }
    }
 

Modified: core/branches/flat/src/test/java/org/jboss/starobrno/api/mvcc/read_committed/ReadCommittedLockTest.java
===================================================================
--- core/branches/flat/src/test/java/org/jboss/starobrno/api/mvcc/read_committed/ReadCommittedLockTest.java	2008-12-10 11:04:37 UTC (rev 7273)
+++ core/branches/flat/src/test/java/org/jboss/starobrno/api/mvcc/read_committed/ReadCommittedLockTest.java	2008-12-10 12:51:42 UTC (rev 7274)
@@ -1,8 +1,11 @@
 package org.jboss.starobrno.api.mvcc.read_committed;
 
+import org.jboss.starobrno.Cache;
 import org.jboss.starobrno.api.mvcc.LockTestBase;
 import org.testng.annotations.Test;
 
+import javax.transaction.Transaction;
+
 @Test(groups = {"functional", "mvcc"})
 public class ReadCommittedLockTest extends LockTestBase
 {
@@ -10,4 +13,33 @@
    {
       repeatableRead = false;
    }
+
+   public void testVisibilityOfCommittedData() throws Exception
+   {
+      Cache c = threadLocal.get().cache;
+      c.put("k", "v");
+
+      assert "v".equals(c.get("k"));
+
+      // start a tx and read K
+      threadLocal.get().tm.begin();
+      assert "v".equals(c.get("k"));
+      assert "v".equals(c.get("k"));
+      Transaction reader = threadLocal.get().tm.suspend();
+
+      threadLocal.get().tm.begin();
+      c.put("k", "v2");
+      Transaction writer = threadLocal.get().tm.suspend();
+
+      threadLocal.get().tm.resume(reader);
+      assert "v".equals(c.get("k")) : "Should not read uncommitted data";
+      reader = threadLocal.get().tm.suspend();
+
+      threadLocal.get().tm.resume(writer);
+      threadLocal.get().tm.commit();
+
+      threadLocal.get().tm.resume(reader);
+      assert "v2".equals(c.get("k")) : "Should read committed data";
+      threadLocal.get().tm.commit();
+   }
 }

Modified: core/branches/flat/src/test/java/org/jboss/starobrno/profiling/TreeProfileTest.java
===================================================================
--- core/branches/flat/src/test/java/org/jboss/starobrno/profiling/TreeProfileTest.java	2008-12-10 11:04:37 UTC (rev 7273)
+++ core/branches/flat/src/test/java/org/jboss/starobrno/profiling/TreeProfileTest.java	2008-12-10 12:51:42 UTC (rev 7274)
@@ -42,10 +42,10 @@
       Test configuration options
     */
    protected static final long NUM_OPERATIONS = 1000000; // DURATION is replaced with a fixed number of operations instead.
-   protected static final int NUM_THREADS = 50;
+   protected static final int NUM_THREADS = 25;
    protected static final int MAX_RANDOM_SLEEP_MILLIS = 1;
-   protected static final int MAX_OVERALL_FQNS = 2000;
-   protected static final int TREE_DEPTH = 2;
+   protected static final int MAX_DEPTH = 3;
+   protected static final int MAX_OVERALL_NODES = 2000;
    protected static final int WARMUP_LOOPS = 20000;
    protected static final boolean USE_SLEEP = false; // throttle generation a bit
 
@@ -60,7 +60,7 @@
       cfg.setConcurrencyLevel(2000);
       cfg.setLockAcquisitionTimeout(120000);
       cfg.setLockParentForChildInsertRemove(true);
-      cfg.setIsolationLevel(IsolationLevel.REPEATABLE_READ);
+      cfg.setIsolationLevel(IsolationLevel.READ_COMMITTED);
       Cache c = new UnitTestCacheFactory().createCache(cfg);
       cache = new TreeCacheImpl(c);
    }
@@ -72,7 +72,7 @@
       cache = null;
    }
 
-   private List<Fqn> fqns = new ArrayList<Fqn>(MAX_OVERALL_FQNS);
+   private List<Fqn> fqns = new ArrayList<Fqn>(MAX_OVERALL_NODES);
 
    Log log = LogFactory.getLog(TreeProfileTest.class);
 
@@ -102,12 +102,12 @@
       long startTime = System.currentTimeMillis();
       log.warn("Starting init() phase");
       fqns.clear();
-      for (int i = 0; i < MAX_OVERALL_FQNS; i++)
+      for (int i = 0; i < MAX_OVERALL_NODES; i++)
       {
          Fqn fqn;
          do
          {
-            fqn = Generator.createRandomFqn(TREE_DEPTH);
+            fqn = Generator.createRandomFqn(MAX_DEPTH);
          }
          while (fqns.contains(fqn));
 




More information about the jbosscache-commits mailing list