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 2015/06/09 11:36:56 UTC
svn commit: r1684361 - in /jackrabbit/oak/branches/1.0/oak-core/src:
main/java/org/apache/jackrabbit/oak/plugins/document/
test/java/org/apache/jackrabbit/oak/plugins/document/
Author: mreutegg
Date: Tue Jun 9 09:36:56 2015
New Revision: 1684361
URL: http://svn.apache.org/r1684361
Log:
OAK-1970: Optimize the diff logic for large number of children case
Commit partial fix from OAK-2685 (support for DocumentNodeState.getRootRevision())
Modified:
jackrabbit/oak/branches/1.0/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeState.java
jackrabbit/oak/branches/1.0/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStore.java
jackrabbit/oak/branches/1.0/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStoreTest.java
Modified: jackrabbit/oak/branches/1.0/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeState.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/branches/1.0/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeState.java?rev=1684361&r1=1684360&r2=1684361&view=diff
==============================================================================
--- jackrabbit/oak/branches/1.0/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeState.java (original)
+++ jackrabbit/oak/branches/1.0/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeState.java Tue Jun 9 09:36:56 2015
@@ -54,7 +54,6 @@ import org.slf4j.LoggerFactory;
import com.google.common.base.Function;
import com.google.common.collect.Iterables;
import com.google.common.collect.Iterators;
-import com.google.common.collect.Maps;
import static com.google.common.base.Preconditions.checkNotNull;
import static org.apache.jackrabbit.oak.plugins.memory.EmptyNodeState.EMPTY_NODE;
@@ -82,29 +81,80 @@ public class DocumentNodeState extends A
final String path;
final Revision rev;
- final Map<String, PropertyState> properties = Maps.newHashMap();
Revision lastRevision;
+ final Revision rootRevision;
+ final Map<String, PropertyState> properties;
final boolean hasChildren;
private final DocumentNodeStore store;
- DocumentNodeState(@Nonnull DocumentNodeStore store, @Nonnull String path,
+ DocumentNodeState(@Nonnull DocumentNodeStore store,
+ @Nonnull String path,
@Nonnull Revision rev) {
this(store, path, rev, false);
}
DocumentNodeState(@Nonnull DocumentNodeStore store, @Nonnull String path,
@Nonnull Revision rev, boolean hasChildren) {
+ this(store, path, rev, new HashMap<String, PropertyState>(),
+ hasChildren, null, null);
+ }
+
+ private DocumentNodeState(@Nonnull DocumentNodeStore store,
+ @Nonnull String path,
+ @Nonnull Revision rev,
+ @Nonnull Map<String, PropertyState> properties,
+ boolean hasChildren,
+ @Nullable Revision lastRevision,
+ @Nullable Revision rootRevision) {
this.store = checkNotNull(store);
this.path = checkNotNull(path);
this.rev = checkNotNull(rev);
+ this.lastRevision = lastRevision;
+ this.rootRevision = rootRevision != null ? rootRevision : rev;
this.hasChildren = hasChildren;
+ this.properties = checkNotNull(properties);
}
+ /**
+ * Creates a copy of this {@code DocumentNodeState} with the
+ * {@link #rootRevision} set to the given {@code root} revision. This method
+ * returns {@code this} instance if the given {@code root} revision is
+ * the same as the one in this instance.
+ *
+ * @param root the root revision for the copy of this node state.
+ * @return a copy of this node state with the given root revision.
+ */
+ DocumentNodeState withRootRevision(@Nonnull Revision root) {
+ if (rootRevision.equals(root)) {
+ return this;
+ } else {
+ return new DocumentNodeState(store, path, rev, properties,
+ hasChildren, lastRevision, root);
+ }
+ }
+
+ @Nonnull
Revision getRevision() {
return rev;
}
+ /**
+ * Returns the root revision for this node state. This is the read revision
+ * passed from the parent node state. This revision therefore reflects the
+ * revision of the root node state where the traversal down the tree
+ * started. The returned revision is only maintained on a best effort basis
+ * and may be the same as {@link #getRevision()} if this node state is
+ * retrieved directly from the {@code DocumentNodeStore}.
+ *
+ * @return the revision of the root node state is available, otherwise the
+ * same value as returned by {@link #getRevision()}.
+ */
+ @Nonnull
+ Revision getRootRevision() {
+ return rootRevision;
+ }
+
//--------------------------< NodeState >-----------------------------------
@Override
@@ -284,7 +334,7 @@ public class DocumentNodeState extends A
checkValidName(name);
return EmptyNodeState.MISSING_NODE;
} else {
- return child;
+ return child.withRootRevision(rootRevision);
}
}
@@ -379,7 +429,7 @@ public class DocumentNodeState extends A
@Override
public int getMemory() {
- int size = 180 + path.length() * 2;
+ int size = 212 + path.length() * 2;
// rough approximation for properties
for (Map.Entry<String, PropertyState> entry : properties.entrySet()) {
// name
@@ -442,7 +492,7 @@ public class DocumentNodeState extends A
@Nonnull
@Override
public NodeState getNodeState() {
- return input;
+ return input.withRootRevision(rootRevision);
}
};
}
@@ -529,9 +579,12 @@ public class DocumentNodeState extends A
@Override
public int getMemory() {
if (cachedMemory == 0) {
- int size = 114;
- for (String c : children) {
- size += c.length() * 2 + 56;
+ int size = 48;
+ if (!children.isEmpty()) {
+ size = 114;
+ for (String c : children) {
+ size += c.length() * 2 + 56;
+ }
}
cachedMemory = size;
}
Modified: jackrabbit/oak/branches/1.0/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStore.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/branches/1.0/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStore.java?rev=1684361&r1=1684360&r2=1684361&view=diff
==============================================================================
--- jackrabbit/oak/branches/1.0/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStore.java (original)
+++ jackrabbit/oak/branches/1.0/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStore.java Tue Jun 9 09:36:56 2015
@@ -2087,7 +2087,7 @@ public final class DocumentNodeStore
if (FAST_DIFF) {
diffAlgo = "diffManyChildren";
diffManyChildren(w, from.getPath(),
- from.getLastRevision(), to.getLastRevision());
+ from.getRootRevision(), to.getRootRevision());
} else {
diffAlgo = "diffAllChildren";
max = Integer.MAX_VALUE;
Modified: jackrabbit/oak/branches/1.0/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStoreTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/branches/1.0/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStoreTest.java?rev=1684361&r1=1684360&r2=1684361&view=diff
==============================================================================
--- jackrabbit/oak/branches/1.0/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStoreTest.java (original)
+++ jackrabbit/oak/branches/1.0/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStoreTest.java Tue Jun 9 09:36:56 2015
@@ -24,6 +24,7 @@ import static org.apache.jackrabbit.oak.
import static org.apache.jackrabbit.oak.plugins.document.NodeDocument.MODIFIED_IN_SECS_RESOLUTION;
import static org.apache.jackrabbit.oak.plugins.document.NodeDocument.NUM_REVS_THRESHOLD;
import static org.apache.jackrabbit.oak.plugins.document.NodeDocument.PREV_SPLIT_FACTOR;
+import static org.apache.jackrabbit.oak.plugins.document.NodeDocument.getModifiedInSecs;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertNotNull;
@@ -85,10 +86,6 @@ import org.apache.jackrabbit.oak.stats.C
import org.junit.After;
import org.junit.Test;
-import com.google.common.collect.Iterables;
-import com.google.common.collect.Lists;
-import com.google.common.collect.Sets;
-
public class DocumentNodeStoreTest {
@After
@@ -1685,6 +1682,78 @@ public class DocumentNodeStoreTest {
ns.dispose();
}
+ // OAK-1970
+ @Test
+ public void diffMany() throws Exception {
+ Clock clock = new Clock.Virtual();
+ clock.waitUntil(System.currentTimeMillis());
+ Revision.setClock(clock);
+ final List<Long> startValues = Lists.newArrayList();
+ MemoryDocumentStore ds = new MemoryDocumentStore() {
+ @Nonnull
+ @Override
+ public <T extends Document> List<T> query(Collection<T> collection,
+ String fromKey,
+ String toKey,
+ String indexedProperty,
+ long startValue,
+ int limit) {
+ if (indexedProperty != null) {
+ startValues.add(startValue);
+ }
+ return super.query(collection, fromKey, toKey, indexedProperty, startValue, limit);
+ }
+ };
+ DocumentNodeStore ns = new DocumentMK.Builder().clock(clock)
+ .setDocumentStore(ds).setAsyncDelay(0).getNodeStore();
+
+ NodeBuilder builder = ns.getRoot().builder();
+ NodeBuilder test = builder.child("test");
+ for (int i = 0; i < DocumentMK.MANY_CHILDREN_THRESHOLD * 2; i++) {
+ test.child("node-" + i);
+ }
+ merge(ns, builder);
+
+ // 'wait one hour'
+ clock.waitUntil(clock.getTime() + TimeUnit.HOURS.toMillis(1));
+
+ // perform a change and use the resulting root as before state
+ builder = ns.getRoot().builder();
+ builder.child("foo");
+ DocumentNodeState before = asDocumentNodeState(merge(ns, builder));
+ NodeState beforeTest = before.getChildNode("test");
+
+ // perform another change to span the diff across multiple revisions
+ // this will prevent diff calls served from the local cache
+ builder = ns.getRoot().builder();
+ builder.child("bar");
+ merge(ns, builder);
+
+ // add a child node
+ builder = ns.getRoot().builder();
+ builder.child("test").child("bar");
+ NodeState after = merge(ns, builder);
+ NodeState afterTest = after.getChildNode("test");
+
+ startValues.clear();
+ afterTest.compareAgainstBaseState(beforeTest, new DefaultNodeStateDiff());
+
+ assertEquals(1, startValues.size());
+ long beforeModified = getModifiedInSecs(before.getRevision().getTimestamp());
+ // startValue must be based on the revision of the before state
+ // and not when '/test' was last modified
+ assertEquals(beforeModified, (long) startValues.get(0));
+
+ ns.dispose();
+ }
+
+ private static DocumentNodeState asDocumentNodeState(NodeState state) {
+ if (!(state instanceof DocumentNodeState)) {
+ throw new IllegalArgumentException("Not a DocumentNodeState");
+ }
+ return (DocumentNodeState) state;
+ }
+
private static void assertNoPreviousDocs(Set<String> ids) {
for (String id : ids) {
assertFalse("must not read previous document: " +
@@ -1693,9 +1762,9 @@ public class DocumentNodeStoreTest {
}
}
- private static void merge(NodeStore store, NodeBuilder root)
+ private static NodeState merge(NodeStore store, NodeBuilder root)
throws CommitFailedException {
- store.merge(root, EmptyHook.INSTANCE, CommitInfo.EMPTY);
+ return store.merge(root, EmptyHook.INSTANCE, CommitInfo.EMPTY);
}
private static class TestHook extends EditorHook {