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/10/21 16:57:04 UTC
svn commit: r1026022 - in /jackrabbit/branches/2.0: ./
jackrabbit-core/src/main/java/org/apache/jackrabbit/core/state/
jackrabbit-core/src/main/java/org/apache/jackrabbit/core/virtual/
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;
/**
* <code>ItemStateReferenceCache</code> internally consists of 2 components:
@@ -50,7 +52,15 @@ 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 =
+ // 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<NodeId>;
- 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<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
@@ -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
* <code>false</code> 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<NodeId, NodeState> 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<ItemStateListener> 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;