Index: oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/MemoryDiffCache.java =================================================================== --- oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/MemoryDiffCache.java (revision 1707509) +++ oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/MemoryDiffCache.java (working copy) @@ -30,6 +30,9 @@ import com.google.common.cache.Cache; import static com.google.common.base.Preconditions.checkNotNull; +import static org.apache.jackrabbit.oak.commons.PathUtils.denotesRoot; +import static org.apache.jackrabbit.oak.commons.PathUtils.getName; +import static org.apache.jackrabbit.oak.commons.PathUtils.getParentPath; /** * An in-memory diff cache implementation. @@ -53,20 +56,27 @@ @CheckForNull @Override - public String getChanges(@Nonnull Revision from, - @Nonnull Revision to, - @Nonnull String path, - final @Nullable Loader loader) { + public String getChanges(@Nonnull final Revision from, + @Nonnull final Revision to, + @Nonnull final String path, + @Nullable final Loader loader) { PathRev key = diffCacheKey(path, from, to); StringValue diff; if (loader == null) { diff = diffCache.getIfPresent(key); + if (diff == null && isUnchanged(from, to, path)) { + diff = new StringValue(""); + } } else { try { diff = diffCache.get(key, new Callable() { @Override public StringValue call() throws Exception { - return new StringValue(loader.call()); + if (isUnchanged(from, to, path)) { + return new StringValue(""); + } else { + return new StringValue(loader.call()); + } } }); } catch (ExecutionException e) { @@ -119,4 +129,61 @@ return new PathRev(from + path, to); } + /** + * Returns {@code true} if it can be inferred from cache entries on + * ancestors of the given {@code path} that the node was not changed between + * the two revisions. This method returns {@code false} if there are no + * matching cache entries for the given revision range or one of them + * indicates that the node at the given path may have been modified. + * + * @param from the from revision. + * @param to the to revision. + * @param path the path of the node to check. + * @return {@code true} if there are cache entries that indicate the node + * at the given path was modified between the two revisions. + */ + private boolean isUnchanged(@Nonnull final Revision from, + @Nonnull final Revision to, + @Nonnull final String path) { + return !denotesRoot(path) + && isChildUnchanged(from, to, getParentPath(path), getName(path)); + } + + private boolean isChildUnchanged(@Nonnull final Revision from, + @Nonnull final Revision to, + @Nonnull final String parent, + @Nonnull final String name) { + PathRev parentKey = diffCacheKey(parent, from, to); + StringValue parentCachedEntry = diffCache.getIfPresent(parentKey); + boolean unchanged; + if (parentCachedEntry == null) { + if (denotesRoot(parent)) { + // reached root and we don't know whether name + // changed between from and to + unchanged = false; + } else { + // recurse down + unchanged = isChildUnchanged(from, to, + getParentPath(parent), getName(parent)); + } + } else { + unchanged = parseJsopDiff(parentCachedEntry.asString(), new Diff() { + @Override + public boolean childNodeAdded(String n) { + return !name.equals(n); + } + + @Override + public boolean childNodeChanged(String n) { + return !name.equals(n); + } + + @Override + public boolean childNodeDeleted(String n) { + return !name.equals(n); + } + }); + } + return unchanged; + } } Index: oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/JournalEntryTest.java =================================================================== --- oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/JournalEntryTest.java (revision 1707509) +++ oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/JournalEntryTest.java (working copy) @@ -21,6 +21,7 @@ import java.util.List; import java.util.Random; import java.util.Set; +import java.util.concurrent.atomic.AtomicBoolean; import com.google.common.collect.Lists; import com.google.common.collect.Sets; @@ -34,7 +35,9 @@ import static org.apache.jackrabbit.oak.plugins.document.Collection.JOURNAL; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; @@ -65,7 +68,27 @@ sort.close(); } + //OAK-3494 @Test + public void useParentDiff() throws Exception { + DiffCache cache = new MemoryDiffCache(new DocumentMK.Builder()); + List paths = Lists.newArrayList("/content/changed", "/content/changed1/child1"); + StringSort sort = JournalEntry.newSorter(); + add(sort, paths); + Revision from = new Revision(1, 0, 1); + Revision to = new Revision(2, 0, 1); + Revision unjournalled = new Revision(3, 0, 1); + sort.sort(); + JournalEntry.applyTo(sort, cache, from, to); + + validateCacheUsage(cache, from, to, "/topUnchanged", true); + validateCacheUsage(cache, from, to, "/content/changed/unchangedLeaf", true); + validateCacheUsage(cache, from, to, "/content/changed1/child2", true); + + validateCacheUsage(cache, from, unjournalled, "/unjournalledPath", false); + } + + @Test public void fillExternalChanges() throws Exception { DocumentStore store = new MemoryDocumentStore(); JournalEntry entry = JOURNAL.newDocument(store); @@ -146,4 +169,24 @@ } } } + + private void validateCacheUsage(DiffCache cache, Revision from, Revision to, String path, boolean cacheExpected) { + String nonLoaderDiff = cache.getChanges(from, to, path, null); + final AtomicBoolean loaderCalled = new AtomicBoolean(false); + cache.getChanges(from, to, path, new DiffCache.Loader() { + @Override + public String call() { + loaderCalled.set(true); + return ""; + } + }); + + if (cacheExpected) { + assertNotNull(nonLoaderDiff); + assertFalse(loaderCalled.get()); + } else { + assertNull(nonLoaderDiff); + assertTrue(loaderCalled.get()); + } + } }