From 44a4b1a26fbc67972630ccb0ed90716f6246f591 Mon Sep 17 00:00:00 2001 From: Vikas Saurabh Date: Wed, 7 Oct 2015 16:46:10 +0530 Subject: [PATCH] OAK-3494 use cached diff entries for parent paths to try to save cache miss --- .../oak/plugins/document/MemoryDiffCache.java | 67 ++++++++++++++++++++-- .../oak/plugins/document/JournalEntryTest.java | 44 ++++++++++++++ 2 files changed, 107 insertions(+), 4 deletions(-) diff --git a/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/MemoryDiffCache.java b/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/MemoryDiffCache.java index 855cdb2..263f873 100644 --- a/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/MemoryDiffCache.java +++ b/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/MemoryDiffCache.java @@ -25,9 +25,14 @@ import javax.annotation.Nonnull; import javax.annotation.Nullable; import org.apache.jackrabbit.oak.cache.CacheStats; +import org.apache.jackrabbit.oak.commons.PathUtils; +import org.apache.jackrabbit.oak.commons.json.JsopReader; +import org.apache.jackrabbit.oak.commons.json.JsopTokenizer; import org.apache.jackrabbit.oak.plugins.document.util.StringValue; import com.google.common.cache.Cache; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import static com.google.common.base.Preconditions.checkNotNull; @@ -35,6 +40,7 @@ import static com.google.common.base.Preconditions.checkNotNull; * An in-memory diff cache implementation. */ public class MemoryDiffCache implements DiffCache { + private static final Logger LOG = LoggerFactory.getLogger(MemoryDiffCache.class); /** * Diff cache. @@ -53,20 +59,27 @@ public class MemoryDiffCache implements DiffCache { @CheckForNull @Override - public String getChanges(@Nonnull Revision from, - @Nonnull Revision to, - @Nonnull String path, + public String getChanges(@Nonnull final Revision from, + @Nonnull final Revision to, + @Nonnull final String path, final @Nullable Loader loader) { PathRev key = diffCacheKey(path, from, to); StringValue diff; if (loader == null) { diff = diffCache.getIfPresent(key); + if (diff == null && doesParentPathImplyNoChange(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 (doesParentPathImplyNoChange(from, to, path)) { + return new StringValue(""); + } else { + return new StringValue(loader.call()); + } } }); } catch (ExecutionException e) { @@ -119,4 +132,50 @@ public class MemoryDiffCache implements DiffCache { return new PathRev(from + path, to); } + /** + * This method would look at cache entries for parent paths upwards (from nearest parent of {@code path} to root). + * @return wheter nearest parent's diff entry in cache list the hierarchy towards {@code path} + */ + private boolean doesParentPathImplyNoChange(@Nonnull Revision from, + @Nonnull Revision to, + @Nonnull String path) { + boolean ret = false; + + if (!PathUtils.denotesRoot(path)) { + String currentPath = path; + while (true) { + String parentPath = PathUtils.getParentPath(currentPath); + PathRev parentKey = diffCacheKey(parentPath, from, to); + StringValue parentCachedEntry = diffCache.getIfPresent(parentKey); + if (parentCachedEntry == null) { + if (PathUtils.denotesRoot(parentPath)) { + break; + } + currentPath = parentPath; + } else { + //A parent's diff is found in cache. We'd return false if the cached entry doesn't point + //to current path + ret = true; + String currentPathName = PathUtils.getName(currentPath); + + JsopReader reader = new JsopTokenizer(parentCachedEntry.asString()); + while (!reader.matches(JsopReader.END)) { + reader.read('^'); + String childName = reader.readString(); + if (childName.equals(currentPathName)) { + ret = false; + break; + } + reader.read(':'); + reader.read('{'); + reader.read('}'); + } + break; + } + } + } + + return ret; + } + } diff --git a/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/JournalEntryTest.java b/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/JournalEntryTest.java index 1be71fa..c3ad0e9 100644 --- a/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/JournalEntryTest.java +++ b/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/JournalEntryTest.java @@ -21,6 +21,7 @@ import java.util.Collections; 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; @@ -30,11 +31,14 @@ import org.apache.jackrabbit.oak.commons.json.JsopReader; import org.apache.jackrabbit.oak.commons.json.JsopTokenizer; import org.apache.jackrabbit.oak.commons.sort.StringSort; import org.apache.jackrabbit.oak.plugins.document.memory.MemoryDocumentStore; +import org.junit.Ignore; import org.junit.Test; 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,6 +69,26 @@ public class JournalEntryTest { 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(); @@ -146,4 +170,24 @@ public class JournalEntryTest { } } } + + 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()); + } + } } -- 2.1.4