You are viewing a plain text version of this content. The canonical link for it is here.
Posted to oak-commits@jackrabbit.apache.org by mr...@apache.org on 2013/02/28 17:26:52 UTC
svn commit: r1451247 - in /jackrabbit/oak/trunk/oak-core/src:
main/java/org/apache/jackrabbit/oak/kernel/
main/java/org/apache/jackrabbit/oak/spi/state/
test/java/org/apache/jackrabbit/oak/kernel/
Author: mreutegg
Date: Thu Feb 28 16:26:52 2013
New Revision: 1451247
URL: http://svn.apache.org/r1451247
Log:
OAK-643: Very high memory usage with 6000 child nodes
Modified:
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/kernel/KernelNodeState.java
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/state/AbstractNodeState.java
jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/kernel/KernelNodeStoreTest.java
Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/kernel/KernelNodeState.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/kernel/KernelNodeState.java?rev=1451247&r1=1451246&r2=1451247&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/kernel/KernelNodeState.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/kernel/KernelNodeState.java Thu Feb 28 16:26:52 2013
@@ -65,7 +65,7 @@ public final class KernelNodeState exten
/**
* Maximum number of child nodes kept in memory.
*/
- static final int MAX_CHILD_NODE_NAMES = 10000;
+ static final int MAX_CHILD_NODE_NAMES = 1000;
private final MicroKernel kernel;
@@ -278,11 +278,12 @@ public final class KernelNodeState exten
return; // no differences
} else if (id != null && id.equals(kbase.id)) {
return; // no differences
- } else if (path.equals(kbase.path) && !path.equals("/")) {
+ } else if (path.equals(kbase.path)
+ && childNodeCount > MAX_CHILD_NODE_NAMES) {
+ // use MK.diff() when there are 'many' child nodes
String jsonDiff = kernel.diff(kbase.getRevision(), revision, path, 0);
- if (!hasChanges(jsonDiff)) {
- return; // no differences
- }
+ processJsonDiff(jsonDiff, kbase, diff);
+ return;
}
}
}
@@ -349,6 +350,81 @@ public final class KernelNodeState exten
return !journal.trim().isEmpty();
}
+ /**
+ * Process the given JSON diff, which is the diff of of the <code>base</code>
+ * node state to this node state.
+ *
+ * @param jsonDiff the JSON diff.
+ * @param base the base node state.
+ * @param diff where diffs are reported to.
+ */
+ private void processJsonDiff(String jsonDiff,
+ KernelNodeState base,
+ NodeStateDiff diff) {
+ if (!hasChanges(jsonDiff)) {
+ return;
+ }
+ comparePropertiesAgainstBaseState(base, diff);
+ JsopTokenizer t = new JsopTokenizer(jsonDiff);
+ while (true) {
+ int r = t.read();
+ if (r == JsopReader.END) {
+ break;
+ }
+ switch (r) {
+ case '+': {
+ String path = t.readString();
+ t.read(':');
+ t.read('{');
+ while (t.read() != '}') {
+ // skip properties
+ }
+ String name = PathUtils.getName(path);
+ diff.childNodeAdded(name, getChildNode(name));
+ break;
+ }
+ case '-': {
+ String path = t.readString();
+ String name = PathUtils.getName(path);
+ diff.childNodeDeleted(name, base.getChildNode(name));
+ break;
+ }
+ case '^': {
+ String path = t.readString();
+ t.read(':');
+ if (t.matches('{')) {
+ t.read('}');
+ String name = PathUtils.getName(path);
+ diff.childNodeChanged(name,
+ base.getChildNode(name), getChildNode(name));
+ } else if (t.matches('[')) {
+ // ignore multi valued property
+ while (t.read() != ']') {
+ // skip values
+ }
+ } else {
+ // ignore single valued property
+ t.read();
+ }
+ break;
+ }
+ case '>': {
+ String from = t.readString();
+ t.read(':');
+ String to = t.readString();
+ String fromName = PathUtils.getName(from);
+ diff.childNodeDeleted(fromName, base.getChildNode(fromName));
+ String toName = PathUtils.getName(to);
+ diff.childNodeAdded(toName, getChildNode(toName));
+ break;
+ }
+ default:
+ throw new IllegalArgumentException("jsonDiff: illegal token '"
+ + t.getToken() + "' at pos: " + t.getLastPos() + " " + jsonDiff);
+ }
+ }
+ }
+
private Iterable<ChildNodeEntry> getChildNodeEntries(
final long offset, final int count) {
return new Iterable<ChildNodeEntry>() {
Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/state/AbstractNodeState.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/state/AbstractNodeState.java?rev=1451247&r1=1451246&r2=1451247&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/state/AbstractNodeState.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/state/AbstractNodeState.java Thu Feb 28 16:26:52 2013
@@ -98,25 +98,7 @@ public abstract class AbstractNodeState
*/
@Override
public void compareAgainstBaseState(NodeState base, NodeStateDiff diff) {
- Set<String> baseProperties = new HashSet<String>();
- for (PropertyState beforeProperty : base.getProperties()) {
- String name = beforeProperty.getName();
- PropertyState afterProperty = getProperty(name);
- if (afterProperty == null) {
- diff.propertyDeleted(beforeProperty);
- } else {
- baseProperties.add(name);
- if (!beforeProperty.equals(afterProperty)) {
- diff.propertyChanged(beforeProperty, afterProperty);
- }
- }
- }
- for (PropertyState afterProperty : getProperties()) {
- if (!baseProperties.contains(afterProperty.getName())) {
- diff.propertyAdded(afterProperty);
- }
- }
-
+ comparePropertiesAgainstBaseState(base, diff);
Set<String> baseChildNodes = new HashSet<String>();
for (ChildNodeEntry beforeCNE : base.getChildNodeEntries()) {
String name = beforeCNE.getName();
@@ -219,6 +201,35 @@ public abstract class AbstractNodeState
return 0;
}
+ /**
+ * Compares the properties of <code>base</code> state with <code>this</code>
+ * state.
+ *
+ * @param base the base node state.
+ * @param diff the node state diff.
+ */
+ protected void comparePropertiesAgainstBaseState(NodeState base,
+ NodeStateDiff diff) {
+ Set<String> baseProperties = new HashSet<String>();
+ for (PropertyState beforeProperty : base.getProperties()) {
+ String name = beforeProperty.getName();
+ PropertyState afterProperty = getProperty(name);
+ if (afterProperty == null) {
+ diff.propertyDeleted(beforeProperty);
+ } else {
+ baseProperties.add(name);
+ if (!beforeProperty.equals(afterProperty)) {
+ diff.propertyChanged(beforeProperty, afterProperty);
+ }
+ }
+ }
+ for (PropertyState afterProperty : getProperties()) {
+ if (!baseProperties.contains(afterProperty.getName())) {
+ diff.propertyAdded(afterProperty);
+ }
+ }
+ }
+
//-----------------------------------------------------------< private >--
private static long count(Iterable<?> iterable) {
Modified: jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/kernel/KernelNodeStoreTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/kernel/KernelNodeStoreTest.java?rev=1451247&r1=1451246&r2=1451247&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/kernel/KernelNodeStoreTest.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/kernel/KernelNodeStoreTest.java Thu Feb 28 16:26:52 2013
@@ -18,12 +18,19 @@
*/
package org.apache.jackrabbit.oak.kernel;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.List;
+
import org.apache.jackrabbit.mk.api.MicroKernel;
import org.apache.jackrabbit.mk.core.MicroKernelImpl;
import org.apache.jackrabbit.oak.api.CommitFailedException;
+import org.apache.jackrabbit.oak.api.PropertyState;
+import org.apache.jackrabbit.oak.plugins.memory.MultiStringPropertyState;
import org.apache.jackrabbit.oak.spi.commit.CommitHook;
import org.apache.jackrabbit.oak.spi.commit.EmptyHook;
import org.apache.jackrabbit.oak.spi.commit.Observer;
+import org.apache.jackrabbit.oak.spi.state.EmptyNodeStateDiff;
import org.apache.jackrabbit.oak.spi.state.NodeBuilder;
import org.apache.jackrabbit.oak.spi.state.NodeState;
import org.apache.jackrabbit.oak.spi.state.NodeStoreBranch;
@@ -34,6 +41,7 @@ import static org.apache.jackrabbit.oak.
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertNull;
+import static org.junit.Assert.assertTrue;
public class KernelNodeStoreTest {
@@ -183,4 +191,127 @@ public class KernelNodeStoreTest {
assertEquals(test, store.getRoot().getChildNode("test"));
}
+ @Test
+ public void manyChildNodes() throws CommitFailedException {
+ NodeStoreBranch branch = store.branch();
+ NodeBuilder root = branch.getHead().builder();
+ NodeBuilder parent = root.child("parent");
+ for (int i = 0; i <= KernelNodeState.MAX_CHILD_NODE_NAMES; i++) {
+ parent.child("child-" + i);
+ }
+ branch.setRoot(root.getNodeState());
+ branch.merge(EmptyHook.INSTANCE);
+
+ NodeState base = store.getRoot();
+ branch = store.branch();
+ root = branch.getHead().builder();
+ parent = root.child("parent");
+ parent.child("child-new");
+ branch.setRoot(root.getNodeState());
+ branch.merge(EmptyHook.INSTANCE);
+
+ Diff diff = new Diff();
+ store.getRoot().compareAgainstBaseState(base, diff);
+
+ assertEquals(0, diff.removed.size());
+ assertEquals(1, diff.added.size());
+ assertEquals("child-new", diff.added.get(0));
+
+ base = store.getRoot();
+ branch = store.branch();
+ branch.move("/parent/child-new", "/parent/child-moved");
+ branch.merge(EmptyHook.INSTANCE);
+
+ diff = new Diff();
+ store.getRoot().compareAgainstBaseState(base, diff);
+
+ assertEquals(1, diff.removed.size());
+ assertEquals("child-new", diff.removed.get(0));
+ assertEquals(1, diff.added.size());
+ assertEquals("child-moved", diff.added.get(0));
+
+ base = store.getRoot();
+ branch = store.branch();
+ root = branch.getHead().builder();
+ parent = root.child("parent");
+ parent.child("child-moved").setProperty("foo", "value");
+ parent.child("child-moved").setProperty(
+ new MultiStringPropertyState("bar", Arrays.asList("value")));
+ branch.setRoot(root.getNodeState());
+ branch.merge(EmptyHook.INSTANCE);
+
+ diff = new Diff();
+ store.getRoot().compareAgainstBaseState(base, diff);
+
+ assertEquals(0, diff.removed.size());
+ assertEquals(0, diff.added.size());
+ assertEquals(2, diff.addedProperties.size());
+ assertTrue(diff.addedProperties.contains("foo"));
+ assertTrue(diff.addedProperties.contains("bar"));
+
+ base = store.getRoot();
+ branch = store.branch();
+ root = branch.getHead().builder();
+ parent = root.child("parent");
+ parent.setProperty("foo", "value");
+ parent.setProperty(new MultiStringPropertyState(
+ "bar", Arrays.asList("value")));
+ branch.setRoot(root.getNodeState());
+ branch.merge(EmptyHook.INSTANCE);
+
+ diff = new Diff();
+ store.getRoot().compareAgainstBaseState(base, diff);
+
+ assertEquals(0, diff.removed.size());
+ assertEquals(0, diff.added.size());
+ assertEquals(2, diff.addedProperties.size());
+ assertTrue(diff.addedProperties.contains("foo"));
+ assertTrue(diff.addedProperties.contains("bar"));
+
+ base = store.getRoot();
+ branch = store.branch();
+ root = branch.getHead().builder();
+ parent = root.child("parent");
+ parent.removeNode("child-moved");
+ branch.setRoot(root.getNodeState());
+ branch.merge(EmptyHook.INSTANCE);
+
+ diff = new Diff();
+ store.getRoot().compareAgainstBaseState(base, diff);
+
+ assertEquals(1, diff.removed.size());
+ assertEquals(0, diff.added.size());
+ assertEquals("child-moved", diff.removed.get(0));
+
+ }
+
+ private static class Diff extends EmptyNodeStateDiff {
+
+ List<String> addedProperties = new ArrayList<String>();
+ List<String> added = new ArrayList<String>();
+ List<String> removed = new ArrayList<String>();
+
+ @Override
+ public void childNodeAdded(String name, NodeState after) {
+ added.add(name);
+ }
+
+ @Override
+ public void childNodeDeleted(String name, NodeState before) {
+ removed.add(name);
+ }
+
+ @Override
+ public void childNodeChanged(String name,
+ NodeState before,
+ NodeState after) {
+ after.compareAgainstBaseState(before, this);
+ }
+
+ @Override
+ public void propertyAdded(PropertyState after) {
+ addedProperties.add(after.getName());
+ }
+ }
+
}