jackrabbit-oak-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From mreut...@apache.org
Subject 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/
Date Thu, 28 Feb 2013 16:26:52 GMT
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());
+        }
+    }
+
 }



Mime
View raw message