Return-Path: Delivered-To: apmail-jackrabbit-commits-archive@www.apache.org Received: (qmail 77551 invoked from network); 28 Sep 2010 14:13:40 -0000 Received: from unknown (HELO mail.apache.org) (140.211.11.3) by 140.211.11.9 with SMTP; 28 Sep 2010 14:13:40 -0000 Received: (qmail 14827 invoked by uid 500); 28 Sep 2010 14:13:40 -0000 Delivered-To: apmail-jackrabbit-commits-archive@jackrabbit.apache.org Received: (qmail 14730 invoked by uid 500); 28 Sep 2010 14:13:37 -0000 Mailing-List: contact commits-help@jackrabbit.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: dev@jackrabbit.apache.org Delivered-To: mailing list commits@jackrabbit.apache.org Received: (qmail 14721 invoked by uid 99); 28 Sep 2010 14:13:36 -0000 Received: from nike.apache.org (HELO nike.apache.org) (192.87.106.230) by apache.org (qpsmtpd/0.29) with ESMTP; Tue, 28 Sep 2010 14:13:36 +0000 X-ASF-Spam-Status: No, hits=-2000.0 required=10.0 tests=ALL_TRUSTED,T_FILL_THIS_FORM_SHORT X-Spam-Check-By: apache.org Received: from [140.211.11.4] (HELO eris.apache.org) (140.211.11.4) by apache.org (qpsmtpd/0.29) with ESMTP; Tue, 28 Sep 2010 14:13:32 +0000 Received: by eris.apache.org (Postfix, from userid 65534) id 69BF223888CF; Tue, 28 Sep 2010 14:13:10 +0000 (UTC) Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Subject: svn commit: r1002168 - in /jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core: ./ state/ virtual/ Date: Tue, 28 Sep 2010 14:13:10 -0000 To: commits@jackrabbit.apache.org From: jukka@apache.org X-Mailer: svnmailer-1.0.8 Message-Id: <20100928141310.69BF223888CF@eris.apache.org> X-Virus-Checked: Checked by ClamAV on apache.org 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; /** * ItemStateReferenceCache internally consists of 2 components: @@ -50,7 +52,10 @@ public class ItemStateReferenceCache imp * primary cache storing weak references to ItemState * instances. */ - private final ItemStateReferenceMap refs; + @SuppressWarnings("unchecked") + private final Map 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 atticStore = + new HashMap(); /** * map of new or modified transient states */ - private final ItemStateStore transientStore; + private final Map transientStore = + new HashMap(); /** * 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 getDescendantItemStates( - ItemId id, ItemStateStore store, HierarchyManager hierarchyManager) - throws RepositoryException { + ItemId id, Map store, + HierarchyManager hierarchyManager) throws RepositoryException { if (id.denotesNode() && !store.isEmpty()) { // Group the descendants by reverse relative depth SortedMap> 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 * false 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 nodes = + new ReferenceMap(ReferenceMap.HARD, ReferenceMap.SOFT); /** * Listeners (weak references) */ + @SuppressWarnings("unchecked") private final transient Collection 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;