Merge pull request #874 from mreutegg/OAK-10149

OAK-10149: Rebase may be expensive with many siblings

oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/AbstractDocumentNodeState.java

@@ -19,6 +19,7 @@
 
 package org.apache.jackrabbit.oak.plugins.document;
 
+import org.apache.jackrabbit.oak.plugins.document.util.ReverseNodeStateDiff;
 import org.apache.jackrabbit.oak.plugins.memory.EmptyNodeState;
 import org.apache.jackrabbit.oak.plugins.memory.ModifiedNodeState;
 import org.apache.jackrabbit.oak.spi.state.AbstractNodeState;
@@ -127,6 +128,13 @@ public boolean compareAgainstBaseState(NodeState base, NodeStateDiff diff) {
                     }
                 }
             }
+        } else if (base instanceof ModifiedNodeState) {
+            ModifiedNodeState mBase = (ModifiedNodeState) base;
+            if (mBase.getBaseState() == this) {
+                // this is the base state of the ModifiedNodeState
+                // do a reverse comparison and report the inverse back to NodeStateDiff
+                return mBase.compareAgainstBaseState(this, new ReverseNodeStateDiff(diff));
+            }
         }
         // fall back to the generic node state diff algorithm
         return super.compareAgainstBaseState(base, diff);

oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/util/ReverseNodeStateDiff.java

@@ -0,0 +1,71 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.jackrabbit.oak.plugins.document.util;
+
+import org.apache.jackrabbit.oak.api.PropertyState;
+import org.apache.jackrabbit.oak.spi.state.NodeState;
+import org.apache.jackrabbit.oak.spi.state.NodeStateDiff;
+
+import static java.util.Objects.requireNonNull;
+
+/**
+ * Implementation of a {@code NodeStateDiff} that reports the inverse operation
+ * to the wrapped {@code NodeStateDiff}.
+ */
+public class ReverseNodeStateDiff implements NodeStateDiff {
+
+    private final NodeStateDiff diff;
+
+    public ReverseNodeStateDiff(NodeStateDiff diff) {
+        this.diff = requireNonNull(diff);
+    }
+
+    @Override
+    public boolean propertyAdded(PropertyState after) {
+        return diff.propertyDeleted(after);
+    }
+
+    @Override
+    public boolean propertyChanged(PropertyState before,
+                                   PropertyState after) {
+        return diff.propertyChanged(after, before);
+    }
+
+    @Override
+    public boolean propertyDeleted(PropertyState before) {
+        return diff.propertyAdded(before);
+    }
+
+    @Override
+    public boolean childNodeAdded(String name,
+                                  NodeState after) {
+        return diff.childNodeDeleted(name, after);
+    }
+
+    @Override
+    public boolean childNodeChanged(String name,
+                                    NodeState before,
+                                    NodeState after) {
+        return diff.childNodeChanged(name, after, before);
+    }
+
+    @Override
+    public boolean childNodeDeleted(String name,
+                                    NodeState before) {
+        return diff.childNodeAdded(name, before);
+    }
+}

oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStateTest.java

@@ -16,6 +16,9 @@
  */
 package org.apache.jackrabbit.oak.plugins.document;
 
+import org.apache.jackrabbit.oak.json.JsopDiff;
+import org.apache.jackrabbit.oak.plugins.memory.MemoryNodeBuilder;
+import org.apache.jackrabbit.oak.plugins.memory.ModifiedNodeState;
 import org.apache.jackrabbit.oak.spi.commit.CommitInfo;
 import org.apache.jackrabbit.oak.spi.commit.EmptyHook;
 import org.apache.jackrabbit.oak.spi.state.NodeBuilder;
@@ -79,4 +82,42 @@ public void asBranchRootStateNonRootState() throws Exception {
         DocumentNodeStoreBranch branch = store.createBranch(store.getRoot());
         asDocumentState(store.getRoot().getChildNode("a")).asBranchRootState(branch);
     }
+
+    @Test // OAK-10149
+    public void compare() throws Exception {
+        DocumentNodeStore store = builderProvider.newBuilder().getNodeStore();
+        NodeBuilder builder = store.getRoot().builder();
+        builder.child("a");
+        builder.setProperty("p", "v");
+        store.merge(builder, EmptyHook.INSTANCE, CommitInfo.EMPTY);
+
+        DocumentNodeState state = store.getRoot();
+
+        // diff reports the inverse operation because the comparison is done
+        // with the modified state as the base state
+        compareAgainstModified(state, b -> b.setProperty("q", "v"), "^\"/q\":null");
+        compareAgainstModified(state, b -> b.removeProperty("p"), "^\"/p\":\"v\"");
+        compareAgainstModified(state, b -> b.setProperty("p", "w"), "^\"/p\":\"v\"");
+        compareAgainstModified(state, b -> b.child("a").setProperty("p", "v"), "^\"/a/p\":null");
+        compareAgainstModified(state, b -> b.child("a").remove(), "+\"/a\":{}");
+        compareAgainstModified(state, b -> b.child("a").child("b"), "-\"/a/b\"");
+    }
+
+    private void compareAgainstModified(DocumentNodeState root,
+                                        Modification modification,
+                                        String jsopDiff) {
+        NodeBuilder builder = new MemoryNodeBuilder(root);
+        modification.apply(builder);
+        NodeState modified = builder.getNodeState();
+        assertEquals(ModifiedNodeState.class, modified.getClass());
+
+        JsopDiff diff = new JsopDiff();
+        root.compareAgainstBaseState(modified, diff);
+        assertEquals(jsopDiff, diff.toString());
+    }
+
+    private interface Modification {
+
+        void apply(NodeBuilder builder);
+    }
 }

oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/RebaseTest.java

@@ -0,0 +1,77 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.jackrabbit.oak.plugins.document;
+
+import org.apache.jackrabbit.oak.plugins.document.memory.MemoryDocumentStore;
+import org.apache.jackrabbit.oak.spi.state.NodeBuilder;
+import org.junit.Before;
+import org.junit.Rule;
+import org.junit.Test;
+
+import static org.apache.jackrabbit.oak.plugins.document.TestUtils.merge;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.lessThan;
+
+public class RebaseTest {
+
+    @Rule
+    public DocumentMKBuilderProvider builderProvider = new DocumentMKBuilderProvider();
+
+    private CountingDocumentStore store;
+
+    private DocumentNodeStore ns;
+
+    @Before
+    public void setup() {
+        store = new CountingDocumentStore(new MemoryDocumentStore());
+        ns = builderProvider.newBuilder()
+                .setDocumentStore(store)
+                .setLeaseCheckMode(LeaseCheckMode.DISABLED)
+                .setAsyncDelay(0)
+                .build();
+    }
+
+    @Test
+    public void rebase() throws Exception {
+        NodeBuilder builder = ns.getRoot().builder();
+        addNodes(builder, 10, 3);
+        merge(ns, builder);
+
+        DocumentNodeState root = ns.getRoot();
+        DocumentNodeStoreBranch b = ns.createBranch(root);
+        builder = root.builder();
+        builder.child("node-4").child("node-2").child("x").child("y");
+        b.setRoot(builder.getNodeState());
+
+        builder.child("node-4").child("node-2").child("x").child("y").setProperty("foo", "bar");
+
+        ns.invalidateNodeChildrenCache();
+        ns.getNodeCache().invalidateAll();
+
+        store.resetCounters();
+        ns.rebase(builder);
+        assertThat(store.getNumFindCalls(Collection.NODES), lessThan(3));
+    }
+
+    private void addNodes(NodeBuilder builder, int num, int level) {
+        if (level > 0) {
+            for (int i = 0; i < num; i++) {
+                addNodes(builder.child("node-" + i), num, level - 1);
+            }
+        }
+    }
+}

oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/util/ReverseNodeStateDiffTest.java

@@ -0,0 +1,67 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.jackrabbit.oak.plugins.document.util;
+
+import org.apache.jackrabbit.oak.json.JsopDiff;
+import org.apache.jackrabbit.oak.plugins.memory.EmptyNodeState;
+import org.apache.jackrabbit.oak.plugins.memory.MemoryNodeBuilder;
+import org.apache.jackrabbit.oak.spi.state.NodeBuilder;
+import org.apache.jackrabbit.oak.spi.state.NodeState;
+import org.junit.Before;
+import org.junit.Test;
+
+import static org.junit.Assert.assertEquals;
+
+public class ReverseNodeStateDiffTest {
+
+    private NodeState base = EmptyNodeState.EMPTY_NODE;
+
+    @Before
+    public void initializeBase() {
+        NodeBuilder builder = new MemoryNodeBuilder(base);
+        builder.child("a");
+        builder.setProperty("p", "v");
+        base = builder.getNodeState();
+    }
+
+    @Test
+    public void compare() {
+        // diff must report the reverse
+        compareAgainstBase(b -> b.setProperty("q", "v"), "^\"/q\":null");
+        compareAgainstBase(b -> b.removeProperty("p"), "^\"/p\":\"v\"");
+        compareAgainstBase(b -> b.setProperty("p", "w"), "^\"/p\":\"v\"");
+        compareAgainstBase(b -> b.child("a").setProperty("p", "v"), "^\"/a/p\":null");
+        compareAgainstBase(b -> b.child("a").remove(), "+\"/a\":{}");
+        compareAgainstBase(b -> b.child("a").child("b"), "-\"/a/b\"");
+    }
+
+    private void compareAgainstBase(Modification modification,
+                                    String jsopDiff) {
+        NodeBuilder builder = new MemoryNodeBuilder(base);
+        modification.apply(builder);
+        NodeState modified = builder.getNodeState();
+
+        JsopDiff diff = new JsopDiff();
+        modified.compareAgainstBaseState(base, new ReverseNodeStateDiff(diff));
+        assertEquals(jsopDiff, diff.toString());
+    }
+
+    private interface Modification {
+
+        void apply(NodeBuilder builder);
+    }
+}