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 2014/02/06 09:29:26 UTC

svn commit: r1565111 - in /jackrabbit/oak/trunk/oak-core/src: main/java/org/apache/jackrabbit/oak/plugins/document/ main/java/org/apache/jackrabbit/oak/plugins/document/util/ test/java/org/apache/jackrabbit/oak/plugins/document/util/

Author: mreutegg
Date: Thu Feb  6 08:29:25 2014
New Revision: 1565111

URL: http://svn.apache.org/r1565111
Log:
OAK-1388: Optimize revision lookup

Modified:
    jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/NodeDocument.java
    jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/PropertyHistory.java
    jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/Revision.java
    jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/util/Utils.java
    jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/util/UtilsTest.java

Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/NodeDocument.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/NodeDocument.java?rev=1565111&r1=1565110&r2=1565111&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/NodeDocument.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/NodeDocument.java Thu Feb  6 08:29:25 2014
@@ -33,6 +33,8 @@ import javax.annotation.CheckForNull;
 import javax.annotation.Nonnull;
 import javax.annotation.Nullable;
 
+import com.google.common.base.Function;
+import com.google.common.base.Predicate;
 import com.google.common.collect.ImmutableSet;
 import org.apache.jackrabbit.oak.cache.CacheValue;
 import org.apache.jackrabbit.oak.commons.PathUtils;
@@ -45,6 +47,8 @@ import com.google.common.collect.Iterabl
 import com.google.common.collect.Maps;
 
 import static com.google.common.base.Preconditions.checkNotNull;
+import static com.google.common.collect.Iterables.filter;
+import static com.google.common.collect.Iterables.transform;
 import static org.apache.jackrabbit.oak.plugins.document.UpdateOp.Key;
 import static org.apache.jackrabbit.oak.plugins.document.UpdateOp.Operation;
 
@@ -478,7 +482,7 @@ public class NodeDocument extends Docume
             // first check local map, which contains most recent values
             Value value = getLatestValue(context, getLocalMap(key),
                     min, readRevision, validRevisions);
-            if (value == null) {
+            if (value == null && !getPreviousRanges().isEmpty()) {
                 // check complete revision history
                 value = getLatestValue(context, getValueMap(key),
                         min, readRevision, validRevisions);
@@ -575,7 +579,7 @@ public class NodeDocument extends Docume
         // check local deleted map first
         Value value = getLatestValue(context, getLocalDeleted(),
                 null, maxRev, validRevisions);
-        if (value == null) {
+        if (value == null && !getPreviousRanges().isEmpty()) {
             // need to check complete map
             value = getLatestValue(context, getDeleted(),
                     null, maxRev, validRevisions);
@@ -761,19 +765,45 @@ public class NodeDocument extends Docume
      * Returns previous {@link NodeDocument}, which include entries for the
      * property in the given revision.
      * If the <code>revision</code> is <code>null</code>, then all previous
-     * documents are returned. The returned documents are returned in descending
-     * revision order (newest first).
+     * documents with changes for the given property are returned. The returned
+     * documents are returned in descending revision order (newest first).
      *
      * @param property the name of a property.
      * @param revision the revision to match or <code>null</code>.
      * @return previous documents.
      */
+    @Nonnull
     Iterable<NodeDocument> getPreviousDocs(@Nonnull final String property,
                                            @Nullable final Revision revision) {
         if (getPreviousRanges().isEmpty()) {
             return Collections.emptyList();
         }
-        return new PropertyHistory(store, this, property, revision);
+        if (revision == null) {
+            return new PropertyHistory(store, this, property);
+        } else {
+            return filter(transform(getPreviousRanges().entrySet(),
+                    new Function<Map.Entry<Revision, Range>, NodeDocument>() {
+                @Override
+                public NodeDocument apply(Map.Entry<Revision, Range> input) {
+                    if (input.getValue().includes(revision)) {
+                        Revision r = input.getKey();
+                        String prevId = Utils.getPreviousIdFor(getId(), r);
+                        NodeDocument prev = store.find(Collection.NODES, prevId);
+                        if (prev != null) {
+                            return prev;
+                        } else {
+                            LOG.warn("Document with previous revisions not found: " + prevId);
+                        }
+                    }
+                    return null;
+                }
+            }), new Predicate<NodeDocument>() {
+                @Override
+                public boolean apply(@Nullable NodeDocument input) {
+                    return input != null && input.getValueMap(property).containsKey(revision);
+                }
+            });
+        }
     }
 
     /**
@@ -988,7 +1018,7 @@ public class NodeDocument extends Docume
         if (value == null) {
             // check previous
             for (NodeDocument prev : getPreviousDocs(REVISIONS, revision)) {
-                value = prev.getLocalRevisions().get(revision);
+                value = prev.getCommitValue(revision);
                 if (value != null) {
                     break;
                 }

Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/PropertyHistory.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/PropertyHistory.java?rev=1565111&r1=1565110&r2=1565111&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/PropertyHistory.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/PropertyHistory.java Thu Feb  6 08:29:25 2014
@@ -17,6 +17,8 @@
 package org.apache.jackrabbit.oak.plugins.document;
 
 import static com.google.common.base.Preconditions.checkNotNull;
+import static com.google.common.collect.Iterables.filter;
+import static com.google.common.collect.Iterables.transform;
 import static java.util.AbstractMap.SimpleImmutableEntry;
 
 import java.util.Iterator;
@@ -28,9 +30,7 @@ import javax.annotation.Nonnull;
 import javax.annotation.Nullable;
 
 import com.google.common.base.Function;
-import com.google.common.base.Predicate;
 import com.google.common.base.Predicates;
-import com.google.common.collect.Iterables;
 import com.google.common.collect.Iterators;
 import com.google.common.collect.PeekingIterator;
 import org.apache.jackrabbit.oak.plugins.document.util.Utils;
@@ -48,38 +48,22 @@ class PropertyHistory implements Iterabl
     private final DocumentStore store;
     private final NodeDocument main;
     private final String property;
-    private final Revision revision;
     private final String id;
 
     public PropertyHistory(@Nonnull DocumentStore store,
                            @Nonnull NodeDocument main,
-                           @Nonnull String property,
-                           @Nullable Revision revision) {
+                           @Nonnull String property) {
         this.store = checkNotNull(store);
         this.main = checkNotNull(main);
         this.property = checkNotNull(property);
-        this.revision = revision;
         this.id = main.getId();
     }
 
     @Override
     public Iterator<NodeDocument> iterator() {
-        Iterable<Map.Entry<Revision, Range>> ranges;
-        if (revision != null) {
-            ranges = Iterables.filter(
-                    main.getPreviousRanges().entrySet(),
-                    new Predicate<Map.Entry<Revision, Range>>() {
-                        @Override
-                        public boolean apply(Map.Entry<Revision, Range> input) {
-                            return input.getValue().includes(revision);
-                        }
-                    });
-        } else {
-            ranges = main.getPreviousRanges().entrySet();
-        }
-
-        Iterable<Map.Entry<Revision, NodeDocument>> docs = Iterables.transform(ranges,
-                new Function<Map.Entry<Revision, Range>, Map.Entry<Revision, NodeDocument>>() {
+        return ensureOrder(filter(
+                transform(main.getPreviousRanges().entrySet(),
+                        new Function<Map.Entry<Revision, Range>, Map.Entry<Revision, NodeDocument>>() {
             @Nullable
             @Override
             public Map.Entry<Revision, NodeDocument> apply(Map.Entry<Revision, Range> input) {
@@ -92,10 +76,7 @@ class PropertyHistory implements Iterabl
                 }
                 return new SimpleImmutableEntry<Revision, NodeDocument>(r, prev);
             }
-        });
-
-        // filter out null docs and ensure order
-        return ensureOrder(Iterables.filter(docs, Predicates.notNull()));
+        }), Predicates.notNull()));
     }
 
     /**
@@ -174,7 +155,7 @@ class PropertyHistory implements Iterabl
                     // check if the revision is actually in there
                     if (doc != null) {
                         Map<Revision, String> values = doc.getValueMap(property);
-                        if (!values.isEmpty() && (revision == null || values.containsKey(revision))) {
+                        if (!values.isEmpty()) {
                             // put into queue with first (highest) revision
                             // from value map
                             queue.put(values.keySet().iterator().next(), doc);

Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/Revision.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/Revision.java?rev=1565111&r1=1565110&r2=1565111&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/Revision.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/Revision.java Thu Feb  6 08:29:25 2014
@@ -200,10 +200,36 @@ public class Revision {
     
     @Override
     public String toString() {
-        return (branch ? "b" : "") + 'r' + Long.toHexString(timestamp) + '-' +
-                Integer.toHexString(counter) + '-' + Integer.toHexString(clusterId);
+        return toStringBuilder(new StringBuilder()).toString();
     }
-    
+
+    /**
+     * Appends the string representation of this revision to the given
+     * StringBuilder.
+     *
+     * @param sb a StringBuilder.
+     * @return the StringBuilder instance passed to this method.
+     */
+    public StringBuilder toStringBuilder(StringBuilder sb) {
+        if (branch) {
+            sb.append('b');
+        }
+        sb.append('r');
+        sb.append(Long.toHexString(timestamp)).append('-');
+        if (counter < 10) {
+            sb.append(counter);
+        } else {
+            sb.append(Integer.toHexString(counter));
+        }
+        sb.append('-');
+        if (clusterId < 10) {
+            sb.append(clusterId);
+        } else {
+            sb.append(Integer.toHexString(clusterId));
+        }
+        return sb;
+    }
+
     @SuppressWarnings("UnusedDeclaration")
     public String toReadableString() {
         StringBuilder buff = new StringBuilder();
@@ -305,7 +331,7 @@ public class Revision {
     public int getClusterId() {
         return clusterId;
     }
-    
+
     /**
      * Revision ranges allow to compare revisions ids of different cluster instances. A
      * range tells when a list of revisions from a certain cluster instance was seen by

Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/util/Utils.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/util/Utils.java?rev=1565111&r1=1565110&r2=1565111&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/util/Utils.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/util/Utils.java Thu Feb  6 08:29:25 2014
@@ -39,7 +39,13 @@ import static com.google.common.base.Pre
  * Utility methods.
  */
 public class Utils {
-    
+
+    /**
+     * Approximate length of a Revision string.
+     */
+    private static final int REVISION_LENGTH =
+            new Revision(System.currentTimeMillis(), 0, 0).toString().length();
+
     /**
      * Make sure the name string does not contain unnecessary baggage (shared
      * strings).
@@ -214,7 +220,20 @@ public class Utils {
     }
 
     public static String getPreviousIdFor(String id, Revision r) {
-        return getIdFromPath("p" + PathUtils.concat(getPathFromId(id), r.toString()));
+        StringBuilder sb = new StringBuilder(id.length() + REVISION_LENGTH + 3);
+        int index = id.indexOf(':');
+        int depth = 0;
+        for (int i = 0; i < index; i++) {
+            depth *= 10;
+            depth += Character.digit(id.charAt(i), 10);
+        }
+        sb.append(depth + 1).append(":p");
+        sb.append(id, index + 1, id.length());
+        if (sb.charAt(sb.length() - 1) != '/') {
+            sb.append('/');
+        }
+        r.toStringBuilder(sb);
+        return sb.toString();
     }
 
     /**

Modified: jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/util/UtilsTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/util/UtilsTest.java?rev=1565111&r1=1565110&r2=1565111&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/util/UtilsTest.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/util/UtilsTest.java Thu Feb  6 08:29:25 2014
@@ -17,6 +17,7 @@
 package org.apache.jackrabbit.oak.plugins.document.util;
 
 import org.apache.jackrabbit.oak.plugins.document.Revision;
+import org.junit.Ignore;
 import org.junit.Test;
 
 import static org.junit.Assert.assertEquals;
@@ -31,5 +32,39 @@ public class UtilsTest {
         Revision r = new Revision(System.currentTimeMillis(), 0, 0);
         assertEquals("1:p/" + r.toString(), Utils.getPreviousIdFor("0:/", r));
         assertEquals("2:p/test/" + r.toString(), Utils.getPreviousIdFor("1:/test", r));
+        assertEquals("14:p/a/b/c/d/e/f/g/h/i/j/k/l/m/" + r.toString(), Utils.getPreviousIdFor("13:/a/b/c/d/e/f/g/h/i/j/k/l/m", r));
+    }
+
+    @Ignore("Performance test")
+    @Test
+    public void performance_getPreviousIdFor() {
+        Revision r = new Revision(System.currentTimeMillis(), 0, 0);
+        String id = Utils.getIdFromPath("/some/test/path/foo");
+        // warm up
+        for (int i = 0; i < 1 * 1000 * 1000; i++) {
+            Utils.getPreviousIdFor(id, r);
+        }
+        long time = System.currentTimeMillis();
+        for (int i = 0; i < 10 * 1000 * 1000; i++) {
+            Utils.getPreviousIdFor(id, r);
+        }
+        time = System.currentTimeMillis() - time;
+        System.out.println(time);
+    }
+
+    @Ignore("Performance test")
+    @Test
+    public void performance_revisionToString() {
+        Revision r = new Revision(System.currentTimeMillis(), 0, 0);
+        // warm up
+        for (int i = 0; i < 1 * 1000 * 1000; i++) {
+            r.toString();
+        }
+        long time = System.currentTimeMillis();
+        for (int i = 0; i < 30 * 1000 * 1000; i++) {
+            r.toString();
+        }
+        time = System.currentTimeMillis() - time;
+        System.out.println(time);
     }
 }