[jbosscache-commits] JBoss Cache SVN: r6289 - in core/trunk/src/main/java/org/jboss/cache: lock and 1 other directory.

jbosscache-commits at lists.jboss.org jbosscache-commits at lists.jboss.org
Wed Jul 16 05:20:10 EDT 2008


Author: manik.surtani at jboss.com
Date: 2008-07-16 05:20:10 -0400 (Wed, 16 Jul 2008)
New Revision: 6289

Added:
   core/trunk/src/main/java/org/jboss/cache/PessimisticUnversionedNode.java
Modified:
   core/trunk/src/main/java/org/jboss/cache/AbstractNode.java
   core/trunk/src/main/java/org/jboss/cache/PessimisticNodeFactory.java
   core/trunk/src/main/java/org/jboss/cache/UnversionedNode.java
   core/trunk/src/main/java/org/jboss/cache/VersionedNode.java
   core/trunk/src/main/java/org/jboss/cache/lock/MVCCLockManager.java
Log:
Reduced synchronization

Modified: core/trunk/src/main/java/org/jboss/cache/AbstractNode.java
===================================================================
--- core/trunk/src/main/java/org/jboss/cache/AbstractNode.java	2008-07-16 08:34:15 UTC (rev 6288)
+++ core/trunk/src/main/java/org/jboss/cache/AbstractNode.java	2008-07-16 09:20:10 UTC (rev 6289)
@@ -7,7 +7,7 @@
 import static org.jboss.cache.AbstractNode.NodeFlags.RESIDENT;
 
 import java.util.EnumSet;
-import java.util.Map;
+import java.util.concurrent.ConcurrentMap;
 
 /**
  * Base class for {@link UnversionedNode}.
@@ -16,7 +16,7 @@
  */
 public abstract class AbstractNode<K, V>
 {
-   protected Map<Object, Node<K, V>> children;
+   protected volatile ConcurrentMap<Object, Node<K, V>> children;
    protected Fqn fqn;
    /**
     * Flags placed on the node.  Replaces older 'boolean' flags.

Modified: core/trunk/src/main/java/org/jboss/cache/PessimisticNodeFactory.java
===================================================================
--- core/trunk/src/main/java/org/jboss/cache/PessimisticNodeFactory.java	2008-07-16 08:34:15 UTC (rev 6288)
+++ core/trunk/src/main/java/org/jboss/cache/PessimisticNodeFactory.java	2008-07-16 09:20:10 UTC (rev 6289)
@@ -13,7 +13,7 @@
    @Override
    protected UnversionedNode<K, V> createInternalNode(Object childName, Fqn fqn, NodeSPI<K, V> parent, Map<K, V> data, boolean mapSafe)
    {
-      UnversionedNode<K, V> internal = new UnversionedNode<K, V>(childName, fqn, data, cache);
+      PessimisticUnversionedNode<K, V> internal = new PessimisticUnversionedNode<K, V>(childName, fqn, data, cache);
       internal.injectDependencies(cache, commandsFactory, lockStrategyFactory, this);
       return internal;
    }

Added: core/trunk/src/main/java/org/jboss/cache/PessimisticUnversionedNode.java
===================================================================
--- core/trunk/src/main/java/org/jboss/cache/PessimisticUnversionedNode.java	                        (rev 0)
+++ core/trunk/src/main/java/org/jboss/cache/PessimisticUnversionedNode.java	2008-07-16 09:20:10 UTC (rev 6289)
@@ -0,0 +1,131 @@
+package org.jboss.cache;
+
+import org.jboss.cache.commands.write.CreateNodeCommand;
+import org.jboss.cache.invocation.InvocationContext;
+import org.jboss.cache.lock.IdentityLock;
+import org.jboss.cache.transaction.GlobalTransaction;
+
+import java.util.Map;
+
+/**
+ * UnversionedNode specific to pessimistic locking, with legacy code.
+ *
+ * @author Manik Surtani (<a href="mailto:manik at jboss.org">manik at jboss.org</a>)
+ * @since 3.0
+ */
+ at SuppressWarnings("deprecation")
+public class PessimisticUnversionedNode<K, V> extends UnversionedNode<K, V>
+{
+   /**
+    * Lock manager that manages locks to be acquired when accessing the node inside a transaction. Lazy set just in case
+    * locking is not needed.
+    */
+   protected transient IdentityLock lock = null;
+
+   public PessimisticUnversionedNode(Object name, Fqn fqn, Map data, CacheSPI<K, V> cache)
+   {
+      super(name, fqn, data, cache);
+   }
+
+   // ------ lock-per-node paradigm
+
+   protected synchronized void initLock()
+   {
+      if (lock == null)
+      {
+         lock = new IdentityLock(lockStrategyFactory, delegate);
+      }
+   }
+
+   @Override
+   public IdentityLock getLock()
+   {
+      initLock();
+      return lock;
+   }
+
+   @Override
+   public String toString()
+   {
+      StringBuilder sb = new StringBuilder(super.toString());
+      if (lock != null)
+      {
+         if (lock.isReadLocked())
+         {
+            sb.append(" RL");
+         }
+         if (lock.isWriteLocked())
+         {
+            sb.append(" WL");
+         }
+      }
+      return sb.toString();
+   }
+
+   // ------ legacy addChild methods that used a lot of implicit locks.
+
+   @Override
+   public void addChildDirect(NodeSPI<K, V> child)
+   {
+      if (child.getFqn().getParent().equals(getFqn()))
+      {
+         synchronized (this)
+         {
+            children().put(child.getFqn().getLastElement(), child);
+         }
+      }
+      else
+         throw new CacheException("Attempting to add a child [" + child.getFqn() + "] to [" + getFqn() + "].  Can only add direct children.");
+   }
+
+   @Override
+   protected NodeSPI<K, V> getOrCreateChild(Object childName, GlobalTransaction gtx, boolean createIfNotExists, boolean notify)
+   {
+      NodeSPI<K, V> child;
+      if (childName == null)
+      {
+         throw new IllegalArgumentException("null child name");
+      }
+
+      child = (NodeSPI<K, V>) children().get(childName);
+      InvocationContext ctx = cache.getInvocationContext();
+      if (createIfNotExists && child == null)
+      {
+         // construct the new child outside the synchronized block to avoid
+         // spending any more time than necessary in the synchronized section
+         Fqn childFqn = Fqn.fromRelativeElements(fqn, childName);
+         NodeSPI<K, V> newChild = nodeFactory.createNode(childName, childFqn, delegate, null, true);
+         if (newChild == null)
+         {
+            throw new IllegalStateException();
+         }
+         synchronized (this)
+         {
+            // check again to see if the child exists
+            // after acquiring exclusive lock
+            child = (NodeSPI<K, V>) children().get(childName);
+            if (child == null)
+            {
+               if (notify) cache.getNotifier().notifyNodeCreated(childFqn, true, ctx);
+               child = newChild;
+               children.put(childName, child);
+            }
+         }
+
+         // notify if we actually created a new child
+         if (newChild == child)
+         {
+            if (trace) log.trace("created child: fqn=" + childFqn);
+
+            if (gtx != null)
+            {
+               CreateNodeCommand createNodeCommand = commandsFactory.buildCreateNodeCommand(childFqn);
+               ctx.getTransactionContext().addModification(createNodeCommand);
+            }
+            if (notify) cache.getNotifier().notifyNodeCreated(childFqn, false, ctx);
+         }
+      }
+      return child;
+
+   }
+}

Modified: core/trunk/src/main/java/org/jboss/cache/UnversionedNode.java
===================================================================
--- core/trunk/src/main/java/org/jboss/cache/UnversionedNode.java	2008-07-16 08:34:15 UTC (rev 6288)
+++ core/trunk/src/main/java/org/jboss/cache/UnversionedNode.java	2008-07-16 09:20:10 UTC (rev 6289)
@@ -25,6 +25,7 @@
 import java.util.Map;
 import java.util.Set;
 import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.ConcurrentMap;
 
 /**
  * Basic data node class.  Throws {@link UnsupportedOperationException} for version-specific methods like {@link #getVersion()} and
@@ -33,7 +34,6 @@
  * @author Manik Surtani (<a href="mailto:manik at jboss.org">manik at jboss.org</a>)
  * @since 2.0.0
  */
- at SuppressWarnings("deprecation")
 public class UnversionedNode<K, V> extends AbstractNode<K, V> implements InternalNode<K, V>
 {
    /**
@@ -43,12 +43,6 @@
    protected static final boolean trace = log.isTraceEnabled();
 
    /**
-    * Lock manager that manages locks to be acquired when accessing the node inside a transaction. Lazy set just in case
-    * locking is not needed.
-    */
-   protected transient IdentityLock lock = null;
-
-   /**
     * A reference of the CacheImpl instance.
     */
    transient CacheSPI<K, V> cache;
@@ -61,7 +55,7 @@
    protected NodeSPI<K, V> delegate;
    CommandsFactory commandsFactory;
    protected LockStrategyFactory lockStrategyFactory;
-   private NodeFactory<K, V> nodeFactory;
+   protected NodeFactory<K, V> nodeFactory;
 
    /**
     * Constructs a new node with an FQN of Root.
@@ -91,6 +85,7 @@
     * @param data  data to add to node
     * @param cache cache reference
     */
+   @SuppressWarnings("unchecked")
    public UnversionedNode(Object name, Fqn fqn, Map data, CacheSPI<K, V> cache)
    {
       if (cache == null)
@@ -108,6 +103,12 @@
 
       init();
       if (data != null && !data.isEmpty()) setInternalState(data);
+
+      // if this is a root node, create the child map.
+      if (fqn.isRoot())
+      {
+         children = new ConcurrentHashMap<Object, Node<K, V>>(64, .5f, 16);
+      }
    }
 
    /**
@@ -160,29 +161,19 @@
       return cache.peek(fqn.getParent(), true);
    }
 
-   protected synchronized void initLock()
+   protected final ConcurrentMap<Object, Node<K, V>> children()
    {
-      if (lock == null)
-      {
-         lock = new IdentityLock(lockStrategyFactory, delegate);
-      }
+      if (children == null) initChildMap();
+      return children;
    }
 
-   private synchronized Map<Object, Node<K, V>> children()
+   private synchronized void initChildMap()
    {
       if (children == null)
       {
-         if (getFqn().isRoot())
-         {
-            children = new ConcurrentHashMap<Object, Node<K, V>>(64, .5f, 16);
-         }
-         else
-         {
-            // Less segments to save memory
-            children = new ConcurrentHashMap<Object, Node<K, V>>(4, .75f, 4);
-         }
+         // Fewer segments to save memory
+         children = new ConcurrentHashMap<Object, Node<K, V>>(4, .75f, 4);
       }
-      return children;
    }
 
    public CacheSPI<K, V> getCache()
@@ -217,21 +208,10 @@
       return data.get(key);
    }
 
-
-   private boolean isReadLocked()
-   {
-      return lock != null && lock.isReadLocked();
-   }
-
-   private boolean isWriteLocked()
-   {
-      return lock != null && lock.isWriteLocked();
-   }
-
+   @SuppressWarnings("deprecation")
    public IdentityLock getLock()
    {
-      initLock();
-      return lock;
+      throw new UnsupportedOperationException("Not supported in this implementation!");
    }
 
    public Map<K, V> getDataDirect()
@@ -251,12 +231,12 @@
       return data.put(key, value);
    }
 
-   public NodeSPI<K, V> getOrCreateChild(Object child_name, GlobalTransaction gtx, boolean notify)
+   public NodeSPI<K, V> getOrCreateChild(Object childName, GlobalTransaction gtx, boolean notify)
    {
-      return getOrCreateChild(child_name, gtx, true, notify);
+      return getOrCreateChild(childName, gtx, true, notify);
    }
 
-   private NodeSPI<K, V> getOrCreateChild(Object childName, GlobalTransaction gtx, boolean createIfNotExists, boolean notify)
+   protected NodeSPI<K, V> getOrCreateChild(Object childName, GlobalTransaction gtx, boolean createIfNotExists, boolean notify)
    {
       NodeSPI<K, V> child;
       if (childName == null)
@@ -268,30 +248,16 @@
       InvocationContext ctx = cache.getInvocationContext();
       if (createIfNotExists && child == null)
       {
-         // construct the new child outside the synchronized block to avoid
-         // spending any more time than necessary in the synchronized section
          Fqn childFqn = Fqn.fromRelativeElements(fqn, childName);
          NodeSPI<K, V> newChild = nodeFactory.createNode(childName, childFqn, delegate, null, true);
-         if (newChild == null)
-         {
-            throw new IllegalStateException();
-         }
-         synchronized (this)
-         {
-            // check again to see if the child exists
-            // after acquiring exclusive lock
-            child = (NodeSPI<K, V>) children().get(childName);
-            if (child == null)
-            {
-               if (notify) cache.getNotifier().notifyNodeCreated(childFqn, true, ctx);
-               child = newChild;
-               children.put(childName, child);
-            }
-         }
 
-         // notify if we actually created a new child
-         if (newChild == child)
+         child = (NodeSPI<K, V>) children().putIfAbsent(childName, newChild);
+
+         if (child == null)
          {
+            // addChild actually succeeded!
+            child = newChild;
+
             if (trace) log.trace("created child: fqn=" + childFqn);
 
             if (gtx != null)
@@ -299,11 +265,16 @@
                CreateNodeCommand createNodeCommand = commandsFactory.buildCreateNodeCommand(childFqn);
                ctx.getTransactionContext().addModification(createNodeCommand);
             }
-            if (notify) cache.getNotifier().notifyNodeCreated(childFqn, false, ctx);
+
+            // notify if we actually created a new child
+            if (notify)
+            {
+               cache.getNotifier().notifyNodeCreated(childFqn, true, ctx);
+               cache.getNotifier().notifyNodeCreated(childFqn, false, ctx);
+            }
          }
       }
       return child;
-
    }
 
    public V remove(K key)
@@ -417,32 +388,22 @@
             sb.append("]");
          }
       }
-      if (lock != null)
-      {
-         if (isReadLocked())
-         {
-            sb.append(" RL");
-         }
-         if (isWriteLocked())
-         {
-            sb.append(" WL");
-         }
-      }
       sb.append("]");
       return sb.toString();
    }
 
    public void addChildDirect(NodeSPI<K, V> child)
    {
-      if (child.getFqn().getParent().equals(getFqn()))
+      Fqn childFqn = child.getFqn();
+      Fqn parentFqn = childFqn.getParent();
+      if (parentFqn.equals(fqn))
       {
-         synchronized (this)
-         {
-            children().put(child.getFqn().getLastElement(), child);
-         }
+         children().put(childFqn.getLastElement(), child);
       }
       else
-         throw new CacheException("Attempting to add a child [" + child.getFqn() + "] to [" + getFqn() + "].  Can only add direct children.");
+      {
+         throw new CacheException("Attempting to add a child [" + childFqn + "] to [" + fqn + "].  Can only add direct children.");
+      }
    }
 
    public NodeSPI<K, V> addChildDirect(Fqn f)

Modified: core/trunk/src/main/java/org/jboss/cache/VersionedNode.java
===================================================================
--- core/trunk/src/main/java/org/jboss/cache/VersionedNode.java	2008-07-16 08:34:15 UTC (rev 6288)
+++ core/trunk/src/main/java/org/jboss/cache/VersionedNode.java	2008-07-16 09:20:10 UTC (rev 6289)
@@ -24,7 +24,7 @@
  * @author <a href="mailto:manik at jboss.org">Manik Surtani (manik at jboss.org)</a>
  * @since 2.0.0
  */
-public class VersionedNode<K, V> extends UnversionedNode<K, V>
+public class VersionedNode<K, V> extends PessimisticUnversionedNode<K, V>
 {
    private static final String DATA_VERSION_INTERNAL_KEY = "_JBOSS_INTERNAL_OPTIMISTIC_DATA_VERSION";
    private DataVersion version; // make sure this is NOT initialized to anything, even a null!  Since the UnversionedNode constructor may set this value based on a data version passed along in the data map.

Modified: core/trunk/src/main/java/org/jboss/cache/lock/MVCCLockManager.java
===================================================================
--- core/trunk/src/main/java/org/jboss/cache/lock/MVCCLockManager.java	2008-07-16 08:34:15 UTC (rev 6288)
+++ core/trunk/src/main/java/org/jboss/cache/lock/MVCCLockManager.java	2008-07-16 09:20:10 UTC (rev 6289)
@@ -320,7 +320,8 @@
        */
       final int hash(Fqn fqn)
       {
-         int h = fqn.hashCode();
+         // TODO: 3.0.0: Find an efficient algorithm to ensure a proper spread of locks across Fqns.
+         int h = fqn.toString().hashCode();
          h += ~(h << 9);
          h ^= (h >>> 14);
          h += (h << 4);




More information about the jbosscache-commits mailing list