Author: jukka Date: Thu Oct 21 14:57:04 2010 New Revision: 1026022 URL: http://svn.apache.org/viewvc?rev=1026022&view=rev Log: 2.0: Merged revisions 1025962 and 1025964 (JCR-2699) Removed: jackrabbit/branches/2.0/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/state/ItemStateMap.java jackrabbit/branches/2.0/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/state/ItemStateReferenceMap.java jackrabbit/branches/2.0/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/state/ItemStateStore.java Modified: jackrabbit/branches/2.0/ (props changed) jackrabbit/branches/2.0/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/state/ItemStateReferenceCache.java jackrabbit/branches/2.0/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/state/NodeState.java jackrabbit/branches/2.0/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/state/PropertyState.java jackrabbit/branches/2.0/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/state/SessionItemStateManager.java jackrabbit/branches/2.0/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/virtual/AbstractVISProvider.java Propchange: jackrabbit/branches/2.0/ ------------------------------------------------------------------------------ --- svn:mergeinfo (original) +++ svn:mergeinfo Thu Oct 21 14:57:04 2010 @@ -1,6 +1,6 @@ /jackrabbit/branches/1.5:794012,794100,794102 -/jackrabbit/branches/2.1:955309,955314,982266,982277,982505,998310,1025933,1025957 +/jackrabbit/branches/2.1:955309,955314,982266,982277,982505,998310,1025933,1025957,1025962,1025964 /jackrabbit/sandbox/JCR-1456:774917-886178 /jackrabbit/sandbox/JCR-2170:812417-816332 /jackrabbit/sandbox/tripod-JCR-2209:795441-795863 -/jackrabbit/trunk:891595,891629,892253,892263,894150-894151,896408,896513,896532,896857,896870,896876,896908,896940,896942-896943,896969,896977,897071,897836,897842,897858,897935,897983,897992-897993,897996,898002,898042,898267,898325,898540,898677,898699,898701,898715,898872,899102,899181,899391,899393-899394,899583,899594,899643,900305,900310,900314,900453,900702,900736,900762-900763,900767,900782,901095,901122,901139,901144,901170,901176,901191,901193,901196,901216,901228,901285,902058,902062,926324,928888,936668,955222,955229,955307,955852,965539,996810,1001707,1002065-1002066,1002084,1002101-1002102 +/jackrabbit/trunk:891595,891629,892253,892263,894150-894151,896408,896513,896532,896857,896870,896876,896908,896940,896942-896943,896969,896977,897071,897836,897842,897858,897935,897983,897992-897993,897996,898002,898042,898267,898325,898540,898677,898699,898701,898715,898872,899102,899181,899391,899393-899394,899583,899594,899643,900305,900310,900314,900453,900702,900736,900762-900763,900767,900782,901095,901122,901139,901144,901170,901176,901191,901193,901196,901216,901228,901285,902058,902062,926324,928888,936668,955222,955229,955307,955852,965539,996810,1001707,1002065-1002066,1002084,1002101-1002102,1002168,1002170,1002589,1002608,1002657 Modified: jackrabbit/branches/2.0/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/state/ItemStateReferenceCache.java URL: http://svn.apache.org/viewvc/jackrabbit/branches/2.0/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/state/ItemStateReferenceCache.java?rev=1026022&r1=1026021&r2=1026022&view=diff ============================================================================== --- jackrabbit/branches/2.0/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/state/ItemStateReferenceCache.java (original) +++ jackrabbit/branches/2.0/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/state/ItemStateReferenceCache.java Thu Oct 21 14:57:04 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,15 @@ public class ItemStateReferenceCache imp * primary cache storing weak references to ItemState * instances. */ - private final ItemStateReferenceMap refs; + @SuppressWarnings("unchecked") + private final Map refs = + // I tried using soft instead of weak references here, but that + // seems to have some unexpected performance consequences (notable + // increase in the JCR TCK run time). So even though soft references + // are generally recommended over weak references for caching + // purposes, it seems that using weak references is safer here. + 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 +87,6 @@ public class ItemStateReferenceCache imp */ public ItemStateReferenceCache(ItemStateCache cache) { this.cache = cache; - refs = new ItemStateReferenceMap(); } //-------------------------------------------------------< ItemStateCache > @@ -86,7 +95,7 @@ public class ItemStateReferenceCache imp */ public synchronized boolean isCached(ItemId id) { // check primary cache - return refs.contains(id); + return refs.containsKey(id); } /** @@ -113,13 +122,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 +181,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/branches/2.0/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/state/NodeState.java URL: http://svn.apache.org/viewvc/jackrabbit/branches/2.0/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/state/NodeState.java?rev=1026022&r1=1026021&r2=1026022&view=diff ============================================================================== --- jackrabbit/branches/2.0/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/state/NodeState.java (original) +++ jackrabbit/branches/2.0/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/state/NodeState.java Thu Oct 21 14:57:04 2010 @@ -800,28 +800,20 @@ public class NodeState extends ItemState //-------------------------------------------------< misc. helper methods > /** - * {@inheritDoc} + * Returns an estimate of the memory size of this node state. The return + * value actually highly overestimates the amount of required memory, but + * changing the estimates would likely cause OOMs in many downstream + * deployments that have set their cache sizes based on experience with + * these erroneous size estimates. So we don't change the formula used + * by this method. */ @Override public long calculateMemoryFootprint() { - /* - private Name nodeTypeName; - private Set mixinTypeNames = Collections.EMPTY_SET; - private NodeId id; - private NodeId parentId; - private ChildNodeEntries childNodeEntries = new ChildNodeEntries(); - private HashSet propertyNames = new HashSet(); - private boolean sharedSet = Set; - private boolean sharedSetRW = false; - private NodeStateListener listener = ...; - - We assume only 16 bytes per name or node id, - as they are shared between states - ChildNodeEntries = 8 + n * (name(16) + index(4) + id(16) + hashentry(16)) ~ n*52 - MixinTypeNames/PropNames = 8 + n * (name(16) + hashentry(16)) - */ - return 100 + mixinTypeNames.size() * 32 + childNodeEntries.size() * 52 - + propertyNames.size() * 32; + // Don't change this formula! See javadoc above. + return 350 + + mixinTypeNames.size() * 250 + + childNodeEntries.size() * 300 + + propertyNames.size() * 250; } /** Modified: jackrabbit/branches/2.0/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/state/PropertyState.java URL: http://svn.apache.org/viewvc/jackrabbit/branches/2.0/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/state/PropertyState.java?rev=1026022&r1=1026021&r2=1026022&view=diff ============================================================================== --- jackrabbit/branches/2.0/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/state/PropertyState.java (original) +++ jackrabbit/branches/2.0/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/state/PropertyState.java Thu Oct 21 14:57:04 2010 @@ -194,23 +194,16 @@ public class PropertyState extends ItemS } /** - * {@inheritDoc} + * Returns an estimate of the memory size of this property state. The + * return value actually highly overestimates the amount of required + * memory, but changing the estimates would likely cause OOMs in many + * downstream deployments that have set their cache sizes based on + * experience with these erroneous size estimates. So we don't change + * the formula used by this method. */ @Override public long calculateMemoryFootprint() { - /* - private PropertyId id; - private InternalValue[] values; - private int type; - private boolean multiValued; - - We assume only 16 bytes per name or node id, - as they are shared between states - PropertyId = 8 + nodeId(16) + name(16) + hash(4) ~ 44; - InternalValue = 8 + n * (values) ~ 8 + n*100; - value=approx 100 bytes. - */ - return 64 + values.length * 100; + return 350 + values.length * 100; } } Modified: jackrabbit/branches/2.0/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/state/SessionItemStateManager.java URL: http://svn.apache.org/viewvc/jackrabbit/branches/2.0/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/state/SessionItemStateManager.java?rev=1026022&r1=1026021&r2=1026022&view=diff ============================================================================== --- jackrabbit/branches/2.0/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/state/SessionItemStateManager.java (original) +++ jackrabbit/branches/2.0/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/state/SessionItemStateManager.java Thu Oct 21 14:57:04 2010 @@ -21,8 +21,12 @@ import java.util.ArrayList; import java.util.Collection; import java.util.Collections; import java.util.Iterator; +import java.util.HashMap; import java.util.List; import java.util.LinkedList; +import java.util.Map; +import java.util.SortedMap; +import java.util.TreeMap; import javax.jcr.InvalidItemStateException; import javax.jcr.ItemNotFoundException; @@ -65,12 +69,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 @@ -93,21 +99,16 @@ public class SessionItemStateManager * * @param rootNodeId the root node id * @param stateMgr the local item state manager - * @param ntReg node type registry */ - protected SessionItemStateManager(NodeId rootNodeId, - LocalItemStateManager stateMgr, - NodeTypeRegistry ntReg) { - transientStore = new ItemStateMap(); - atticStore = new ItemStateMap(); - + public SessionItemStateManager( + NodeId rootNodeId, LocalItemStateManager stateMgr, + NodeTypeRegistry ntReg) { this.stateMgr = stateMgr; + this.ntReg = ntReg; // create hierarchy manager that uses both transient and persistent state hierMgr = new CachingHierarchyManager(rootNodeId, this); addListener(hierMgr); - - this.ntReg = ntReg; } /** @@ -119,11 +120,10 @@ public class SessionItemStateManager * @return the session item state manager. */ public static SessionItemStateManager createInstance( - NodeId rootNodeId, - LocalItemStateManager stateMgr, + NodeId rootNodeId, LocalItemStateManager stateMgr, NodeTypeRegistry ntReg) { - SessionItemStateManager mgr = new SessionItemStateManager( - rootNodeId, stateMgr, ntReg); + SessionItemStateManager mgr = + new SessionItemStateManager(rootNodeId, stateMgr, ntReg); stateMgr.addListener(mgr); return mgr; } @@ -181,7 +181,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 @@ -193,7 +193,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); } @@ -205,16 +205,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 @@ -366,7 +366,7 @@ public class SessionItemStateManager * @return */ public boolean hasTransientItemState(ItemId id) { - return transientStore.contains(id); + return transientStore.containsKey(id); } /** @@ -375,7 +375,7 @@ public class SessionItemStateManager * @return */ public boolean hasTransientItemStateInAttic(ItemId id) { - return atticStore.contains(id); + return atticStore.containsKey(id); } /** @@ -572,7 +572,7 @@ public class SessionItemStateManager } // short cut - if (transientStore.contains(hierMgr.getRootNodeId())) { + if (transientStore.containsKey(hierMgr.getRootNodeId())) { return hierMgr.getRootNodeId(); } @@ -665,7 +665,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 > @@ -683,7 +683,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); @@ -692,7 +692,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; } @@ -711,7 +711,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); @@ -719,7 +719,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; } @@ -739,7 +739,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); @@ -747,7 +747,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; } @@ -766,7 +766,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); @@ -774,7 +774,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; } @@ -820,7 +820,7 @@ public class SessionItemStateManager // remove from map transientStore.remove(state.getId()); // add to attic - atticStore.put(state); + atticStore.put(state.getId(), state); } /** @@ -949,7 +949,7 @@ public class SessionItemStateManager } public boolean isDeleted(ItemId id) { - return atticStore.contains(id); + return atticStore.containsKey(id); } public boolean isModified(ItemId id) { @@ -1046,7 +1046,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); } } @@ -1058,7 +1059,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); } } @@ -1070,7 +1072,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); } } @@ -1082,7 +1085,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); } } @@ -1114,7 +1118,7 @@ public class SessionItemStateManager * {@inheritDoc} */ public boolean hasItemState(ItemId id) { - return atticStore.contains(id); + return atticStore.containsKey(id); } /** Modified: jackrabbit/branches/2.0/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/virtual/AbstractVISProvider.java URL: http://svn.apache.org/viewvc/jackrabbit/branches/2.0/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/virtual/AbstractVISProvider.java?rev=1026022&r1=1026021&r2=1026022&view=diff ============================================================================== --- jackrabbit/branches/2.0/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/virtual/AbstractVISProvider.java (original) +++ jackrabbit/branches/2.0/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/virtual/AbstractVISProvider.java Thu Oct 21 14:57:04 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,17 @@ public abstract class AbstractVISProvide /** * the cache node states. key=ItemId, value=ItemState */ - private ItemStateReferenceMap nodes = new ItemStateReferenceMap(); + @SuppressWarnings("unchecked") + private final Map nodes = + // Using soft references instead of weak ones seems to have + // some unexpected performance consequences, so for now it's + // better to stick with weak references. + new ReferenceMap(ReferenceMap.HARD, ReferenceMap.WEAK); /** * Listeners (weak references) */ + @SuppressWarnings("unchecked") private final transient Collection listeners = new WeakIdentityCollection(5); @@ -102,7 +109,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 +129,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(); @@ -321,7 +328,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;