You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@jackrabbit.apache.org by ju...@apache.org on 2010/09/28 16:13:10 UTC

svn commit: r1002168 - in /jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core: ./ state/ virtual/

Author: jukka
Date: Tue Sep 28 14:13:09 2010
New Revision: 1002168

URL: http://svn.apache.org/viewvc?rev=1002168&view=rev
Log:
JCR-2699: Improve read/write concurrency

Remove wrapper classes that add no extra functionality to the underlying map implementations.
This simplifies the code and helps identify opportunities for improved data structures.

Also removed an obsolete NodeTypeRegistry reference from SessionItemStateManager.

Removed:
    jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/state/ItemStateMap.java
    jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/state/ItemStateReferenceMap.java
    jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/state/ItemStateStore.java
Modified:
    jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/SessionImpl.java
    jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/state/ItemStateReferenceCache.java
    jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/state/SessionItemStateManager.java
    jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/virtual/AbstractVISProvider.java

Modified: jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/SessionImpl.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/SessionImpl.java?rev=1002168&r1=1002167&r2=1002168&view=diff
==============================================================================
--- jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/SessionImpl.java (original)
+++ jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/SessionImpl.java Tue Sep 28 14:13:09 2010
@@ -260,8 +260,7 @@ public class SessionImpl extends Abstrac
     protected SessionItemStateManager createSessionItemStateManager() {
         SessionItemStateManager mgr = new SessionItemStateManager(
                 context.getRootNodeId(),
-                context.getWorkspace().getItemStateManager(),
-                context.getNodeTypeRegistry());
+                context.getWorkspace().getItemStateManager());
         context.getWorkspace().getItemStateManager().addListener(mgr);
         return mgr;
     }

Modified: jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/state/ItemStateReferenceCache.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/state/ItemStateReferenceCache.java?rev=1002168&r1=1002167&r2=1002168&view=diff
==============================================================================
--- jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/state/ItemStateReferenceCache.java (original)
+++ jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/state/ItemStateReferenceCache.java Tue Sep 28 14:13:09 2010
@@ -16,12 +16,14 @@
  */
 package org.apache.jackrabbit.core.state;
 
+import org.apache.commons.collections.map.ReferenceMap;
 import org.apache.jackrabbit.core.id.ItemId;
 import org.apache.jackrabbit.core.util.Dumpable;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
 import java.io.PrintStream;
+import java.util.Map;
 
 /**
  * <code>ItemStateReferenceCache</code> internally consists of 2 components:
@@ -50,7 +52,10 @@ public class ItemStateReferenceCache imp
      * primary cache storing weak references to <code>ItemState</code>
      * instances.
      */
-    private final ItemStateReferenceMap refs;
+    @SuppressWarnings("unchecked")
+    private final Map<ItemId, ItemState> refs =
+        new ReferenceMap(ReferenceMap.HARD, ReferenceMap.WEAK);
+
     /**
      * secondary cache that automatically flushes entries based on some
      * eviction policy; entries flushed from the secondary cache will be
@@ -77,7 +82,6 @@ public class ItemStateReferenceCache imp
      */
     public ItemStateReferenceCache(ItemStateCache cache) {
         this.cache = cache;
-        refs = new ItemStateReferenceMap();
     }
 
     //-------------------------------------------------------< ItemStateCache >
@@ -86,7 +90,7 @@ public class ItemStateReferenceCache imp
      */
     public synchronized boolean isCached(ItemId id) {
         // check primary cache
-        return refs.contains(id);
+        return refs.containsKey(id);
     }
 
     /**
@@ -113,13 +117,13 @@ public class ItemStateReferenceCache imp
      */
     public synchronized void cache(ItemState state) {
         ItemId id = state.getId();
-        if (refs.contains(id)) {
+        if (refs.containsKey(id)) {
             log.warn("overwriting cached entry " + id);
         }
         // fake call to update stats of secondary cache
         cache.cache(state);
         // store weak reference in primary cache
-        refs.put(state);
+        refs.put(id, state);
 
     }
 
@@ -172,8 +176,7 @@ public class ItemStateReferenceCache imp
      */
     public synchronized void dump(PrintStream ps) {
         ps.println("ItemStateReferenceCache (" + this + ")");
+        ps.println("  refs: " + refs.keySet());
         ps.println();
-        ps.print("[refs] ");
-        refs.dump(ps);
     }
 }

Modified: jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/state/SessionItemStateManager.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/state/SessionItemStateManager.java?rev=1002168&r1=1002167&r2=1002168&view=diff
==============================================================================
--- jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/state/SessionItemStateManager.java (original)
+++ jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/state/SessionItemStateManager.java Tue Sep 28 14:13:09 2010
@@ -20,8 +20,10 @@ import java.io.PrintStream;
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Collections;
+import java.util.HashMap;
 import java.util.List;
 import java.util.LinkedList;
+import java.util.Map;
 import java.util.SortedMap;
 import java.util.TreeMap;
 
@@ -36,11 +38,8 @@ import org.apache.jackrabbit.core.Zombie
 import org.apache.jackrabbit.core.id.ItemId;
 import org.apache.jackrabbit.core.id.NodeId;
 import org.apache.jackrabbit.core.id.PropertyId;
-import org.apache.jackrabbit.core.nodetype.NodeTypeRegistry;
-import org.apache.jackrabbit.core.nodetype.EffectiveNodeType;
 import org.apache.jackrabbit.core.util.Dumpable;
 import org.apache.jackrabbit.spi.Name;
-import org.apache.jackrabbit.spi.QNodeDefinition;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -65,12 +64,14 @@ public class SessionItemStateManager
     /**
      * map of those states that have been removed transiently
      */
-    private final ItemStateStore atticStore;
+    private final Map<ItemId, ItemState> atticStore =
+        new HashMap<ItemId, ItemState>();
 
     /**
      * map of new or modified transient states
      */
-    private final ItemStateStore transientStore;
+    private final Map<ItemId, ItemState> transientStore =
+        new HashMap<ItemId, ItemState>();
 
     /**
      * ItemStateManager view of the states in the attic; lazily instantiated
@@ -79,11 +80,6 @@ public class SessionItemStateManager
     private AtticItemStateManager attic;
 
     /**
-     * Node Type Registry
-     */
-    private final NodeTypeRegistry ntReg;
-
-    /**
      * State change dispatcher.
      */
     private final transient StateChangeDispatcher dispatcher = new StateChangeDispatcher();
@@ -93,21 +89,14 @@ public class SessionItemStateManager
      *
      * @param rootNodeId the root node id
      * @param stateMgr the local item state manager
-     * @param ntReg node type registry
      */
-    public SessionItemStateManager(NodeId rootNodeId,
-                                   LocalItemStateManager stateMgr,
-                                   NodeTypeRegistry ntReg) {
-        transientStore = new ItemStateMap();
-        atticStore = new ItemStateMap();
-
+    public SessionItemStateManager(
+            NodeId rootNodeId, LocalItemStateManager stateMgr) {
         this.stateMgr = stateMgr;
 
         // create hierarchy manager that uses both transient and persistent state
         hierMgr = new CachingHierarchyManager(rootNodeId, this);
         addListener(hierMgr);
-
-        this.ntReg = ntReg;
     }
 
     /**
@@ -163,7 +152,7 @@ public class SessionItemStateManager
             throws NoSuchItemStateException, ItemStateException {
 
         // first check if the specified item has been transiently removed
-        if (atticStore.contains(id)) {
+        if (atticStore.containsKey(id)) {
             /**
              * check if there's new transient state for the specified item
              * (e.g. if a property with name 'x' has been removed and a new
@@ -175,7 +164,7 @@ public class SessionItemStateManager
         }
 
         // check if there's transient state for the specified item
-        if (transientStore.contains(id)) {
+        if (transientStore.containsKey(id)) {
             return getTransientItemState(id);
         }
 
@@ -187,16 +176,16 @@ public class SessionItemStateManager
      */
     public boolean hasItemState(ItemId id) {
         // first check if the specified item has been transiently removed
-        if (atticStore.contains(id)) {
+        if (atticStore.containsKey(id)) {
             /**
              * check if there's new transient state for the specified item
              * (e.g. if a property with name 'x' has been removed and a new
              * property with same name has been created);
              */
-            return transientStore.contains(id);
+            return transientStore.containsKey(id);
         }
         // check if there's transient state for the specified item
-        if (transientStore.contains(id)) {
+        if (transientStore.containsKey(id)) {
             return true;
         }
         // check if there's persistent state for the specified item
@@ -346,7 +335,7 @@ public class SessionItemStateManager
      * @return
      */
     public boolean hasTransientItemState(ItemId id) {
-        return transientStore.contains(id);
+        return transientStore.containsKey(id);
     }
 
     /**
@@ -355,7 +344,7 @@ public class SessionItemStateManager
      * @return
      */
     public boolean hasTransientItemStateInAttic(ItemId id) {
-        return atticStore.contains(id);
+        return atticStore.containsKey(id);
     }
 
     /**
@@ -429,8 +418,8 @@ public class SessionItemStateManager
      * @throws RepositoryException if the descendants could not be accessed
      */
     private List<ItemState> getDescendantItemStates(
-            ItemId id, ItemStateStore store, HierarchyManager hierarchyManager)
-            throws RepositoryException {
+            ItemId id, Map<ItemId, ItemState> store,
+            HierarchyManager hierarchyManager) throws RepositoryException {
         if (id.denotesNode() && !store.isEmpty()) {
             // Group the descendants by reverse relative depth
             SortedMap<Integer, Collection<ItemState>> statesByReverseDepth =
@@ -476,7 +465,7 @@ public class SessionItemStateManager
         }
 
         // short cut
-        if (transientStore.contains(hierMgr.getRootNodeId())) {
+        if (transientStore.containsKey(hierMgr.getRootNodeId())) {
             return hierMgr.getRootNodeId();
         }
 
@@ -569,7 +558,7 @@ public class SessionItemStateManager
      *         <code>false</code> otherwise
      */
     public boolean isItemStateInAttic(ItemId id) {
-        return atticStore.contains(id);
+        return atticStore.containsKey(id);
     }
 
     //------< methods for creating & discarding transient ItemState instances >
@@ -587,7 +576,7 @@ public class SessionItemStateManager
 
         // check map; synchronized to ensure an entry is not created twice.
         synchronized (transientStore) {
-            if (transientStore.contains(id)) {
+            if (transientStore.containsKey(id)) {
                 String msg = "there's already a node state instance with id " + id;
                 log.debug(msg);
                 throw new ItemStateException(msg);
@@ -596,7 +585,7 @@ public class SessionItemStateManager
             NodeState state = new NodeState(id, nodeTypeName, parentId,
                     initialStatus, true);
             // put transient state in the map
-            transientStore.put(state);
+            transientStore.put(state.getId(), state);
             state.setContainer(this);
             return state;
         }
@@ -615,7 +604,7 @@ public class SessionItemStateManager
 
         // check map; synchronized to ensure an entry is not created twice.
         synchronized (transientStore) {
-            if (transientStore.contains(id)) {
+            if (transientStore.containsKey(id)) {
                 String msg = "there's already a node state instance with id " + id;
                 log.debug(msg);
                 throw new ItemStateException(msg);
@@ -623,7 +612,7 @@ public class SessionItemStateManager
 
             NodeState state = new NodeState(overlayedState, initialStatus, true);
             // put transient state in the map
-            transientStore.put(state);
+            transientStore.put(id, state);
             state.setContainer(this);
             return state;
         }
@@ -643,7 +632,7 @@ public class SessionItemStateManager
 
         // check map; synchronized to ensure an entry is not created twice.
         synchronized (transientStore) {
-            if (transientStore.contains(id)) {
+            if (transientStore.containsKey(id)) {
                 String msg = "there's already a property state instance with id " + id;
                 log.debug(msg);
                 throw new ItemStateException(msg);
@@ -651,7 +640,7 @@ public class SessionItemStateManager
 
             PropertyState state = new PropertyState(id, initialStatus, true);
             // put transient state in the map
-            transientStore.put(state);
+            transientStore.put(id, state);
             state.setContainer(this);
             return state;
         }
@@ -670,7 +659,7 @@ public class SessionItemStateManager
 
         // check map; synchronized to ensure an entry is not created twice.
         synchronized (transientStore) {
-            if (transientStore.contains(id)) {
+            if (transientStore.containsKey(id)) {
                 String msg = "there's already a property state instance with id " + id;
                 log.debug(msg);
                 throw new ItemStateException(msg);
@@ -678,7 +667,7 @@ public class SessionItemStateManager
 
             PropertyState state = new PropertyState(overlayedState, initialStatus, true);
             // put transient state in the map
-            transientStore.put(state);
+            transientStore.put(id, state);
             state.setContainer(this);
             return state;
         }
@@ -724,7 +713,7 @@ public class SessionItemStateManager
         // remove from map
         transientStore.remove(state.getId());
         // add to attic
-        atticStore.put(state);
+        atticStore.put(state.getId(), state);
     }
 
     /**
@@ -896,7 +885,8 @@ public class SessionItemStateManager
      * or if the local state is not overlayed.
      */
     public void nodeAdded(NodeState state, Name name, int index, NodeId id) {
-        if (state.getContainer() == this || !transientStore.contains(state.getId())) {
+        if (state.getContainer() == this
+                || !transientStore.containsKey(state.getId())) {
             dispatcher.notifyNodeAdded(state, name, index, id);
         }
     }
@@ -908,7 +898,8 @@ public class SessionItemStateManager
      * or if the local state is not overlayed.
      */
     public void nodesReplaced(NodeState state) {
-        if (state.getContainer() == this || !transientStore.contains(state.getId())) {
+        if (state.getContainer() == this
+                || !transientStore.containsKey(state.getId())) {
             dispatcher.notifyNodesReplaced(state);
         }
     }
@@ -920,7 +911,8 @@ public class SessionItemStateManager
      * or if the local state is not overlayed.
      */
     public void nodeModified(NodeState state) {
-        if (state.getContainer() == this || !transientStore.contains(state.getId())) {
+        if (state.getContainer() == this
+                || !transientStore.containsKey(state.getId())) {
             dispatcher.notifyNodeModified(state);
         }
     }
@@ -932,7 +924,8 @@ public class SessionItemStateManager
      * or if the local state is not overlayed.
      */
     public void nodeRemoved(NodeState state, Name name, int index, NodeId id) {
-        if (state.getContainer() == this || !transientStore.contains(state.getId())) {
+        if (state.getContainer() == this
+                || !transientStore.containsKey(state.getId())) {
             dispatcher.notifyNodeRemoved(state, name, index, id);
         }
     }
@@ -964,7 +957,7 @@ public class SessionItemStateManager
          * {@inheritDoc}
          */
         public boolean hasItemState(ItemId id) {
-            return atticStore.contains(id);
+            return atticStore.containsKey(id);
         }
 
         /**

Modified: jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/virtual/AbstractVISProvider.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/virtual/AbstractVISProvider.java?rev=1002168&r1=1002167&r2=1002168&view=diff
==============================================================================
--- jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/virtual/AbstractVISProvider.java (original)
+++ jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/virtual/AbstractVISProvider.java Tue Sep 28 14:13:09 2010
@@ -16,6 +16,7 @@
  */
 package org.apache.jackrabbit.core.virtual;
 
+import org.apache.commons.collections.map.ReferenceMap;
 import org.apache.jackrabbit.core.id.ItemId;
 import org.apache.jackrabbit.core.id.NodeId;
 import org.apache.jackrabbit.core.id.PropertyId;
@@ -27,7 +28,6 @@ import org.apache.jackrabbit.core.state.
 import org.apache.jackrabbit.core.state.NoSuchItemStateException;
 import org.apache.jackrabbit.core.state.NodeReferences;
 import org.apache.jackrabbit.core.state.NodeState;
-import org.apache.jackrabbit.core.state.ItemStateReferenceMap;
 import org.apache.jackrabbit.core.state.ItemStateListener;
 import org.apache.jackrabbit.core.state.ChildNodeEntry;
 import org.apache.jackrabbit.spi.Name;
@@ -39,6 +39,7 @@ import org.slf4j.LoggerFactory;
 
 import javax.jcr.RepositoryException;
 import java.util.Collection;
+import java.util.Map;
 
 /**
  * This Class implements a virtual item state provider, in order to expose the
@@ -68,11 +69,14 @@ public abstract class AbstractVISProvide
     /**
      * the cache node states. key=ItemId, value=ItemState
      */
-    private ItemStateReferenceMap nodes = new ItemStateReferenceMap();
+    @SuppressWarnings("unchecked")
+    private final Map<NodeId, NodeState> nodes =
+        new ReferenceMap(ReferenceMap.HARD, ReferenceMap.SOFT);
 
     /**
      * Listeners (weak references)
      */
+    @SuppressWarnings("unchecked")
     private final transient Collection<ItemStateListener> listeners =
         new WeakIdentityCollection(5);
 
@@ -102,7 +106,7 @@ public abstract class AbstractVISProvide
      */
     public boolean hasItemState(ItemId id) {
         if (id instanceof NodeId) {
-            if (nodes.contains(id)) {
+            if (nodes.containsKey(id)) {
                 return true;
             } else if (id.equals(rootNodeId)) {
                 return true;
@@ -122,7 +126,7 @@ public abstract class AbstractVISProvide
 
         if (id instanceof NodeId) {
             ItemState s;
-            if (nodes.contains(id)) {
+            if (nodes.containsKey(id)) {
                 s = nodes.get(id);
             } else if (id.equals(rootNodeId)) {
                 s = getRootState();
@@ -359,7 +363,7 @@ public abstract class AbstractVISProvide
      */
     protected NodeState cache(NodeState state) {
         if (state != null) {
-            nodes.put(state);
+            nodes.put(state.getNodeId(), state);
             log.debug("item added to cache. size=" + nodes.size());
         }
         return state;