Cache project metadata and tokens

Motivation: Currently, only the raw contents of `ProjectMetadata` and `Tokens` are cached when fetched from `MetadataService`. However, deserialization takes a significant amount of time, so caching the deserialized instances would improve performance. Modifications: - Renamed `CacheableCall` to `AbstractCacheableCall`. - Introduced `CacheableCall` interface. - Added `Repository.execute(CacheableCall)` to cache calls. - Cached deserialized instances of `ProjectMetadata` and `Tokens` retrieved by `MetadataService`. Result: - Improved performance by avoiding redundant deserialization of `ProjectMetadata` and `Tokens`.
line · Feb 14, 2025 · b9cebef · b9cebef
1 parent 47723bc
commit b9cebef
Show file tree

Hide file tree

Showing 28 changed files with 386 additions and 78 deletions.
diff --git a/it/server/src/test/java/com/linecorp/centraldogma/it/CacheTest.java b/it/server/src/test/java/com/linecorp/centraldogma/it/CacheTest.java
@@ -71,10 +71,13 @@ void getFile(ClientType clientType, TestInfo testInfo) {
             //     Therefore we should observe five cache miss. (Thrift only)
             assertThat(missCount(meters2)).isEqualTo(
                     missCount(meters1) + 1 +
+                    1 + // (CacheableFetchCall in RepositorySupport for metadata.json)
                     1 + // (CacheableHistoryCall)
                     4); // (CacheableObjectLoaderCall: 2 for revision2 and 2 for revision1)
         } else {
-            assertThat(missCount(meters2)).isEqualTo(missCount(meters1) + 1);
+            assertThat(missCount(meters2)).isEqualTo(
+                    missCount(meters1) + 1 +
+                    1); // (CacheableFetchCall in RepositorySupport for metadata.json)
         }
 
         // First getFile() should miss.
@@ -123,10 +126,13 @@ void history(ClientType clientType, TestInfo testInfo) {
             //     Therefore we should observe five cache miss. (Thrift only)
             assertThat(currentMissCount).isEqualTo(
                     prevMissCount + 1 +
+                    1 + // (CacheableFetchCall in RepositorySupport for metadata.json)
                     1 + // (CacheableHistoryCall)
                     4); // (CacheableObjectLoaderCall: 2 for revision2 and 2 for revision1)
         } else {
-            assertThat(currentMissCount).isEqualTo(prevMissCount + 1);
+            assertThat(currentMissCount).isEqualTo(
+                    prevMissCount + 1 +
+                    1); // (CacheableFetchCall in RepositorySupport for metadata.json)
         }
         prevMissCount = currentMissCount;
 

diff --git a/...in/java/com/linecorp/centraldogma/server/internal/storage/repository/RepositoryCache.java b/...in/java/com/linecorp/centraldogma/server/internal/storage/repository/RepositoryCache.java
@@ -34,6 +34,9 @@
 import com.github.benmanes.caffeine.cache.stats.CacheStats;
 import com.google.common.base.MoreObjects;
 
+import com.linecorp.centraldogma.server.internal.storage.repository.git.AbstractCacheableCall;
+import com.linecorp.centraldogma.server.storage.repository.CacheableCall;
+
 import io.micrometer.core.instrument.MeterRegistry;
 import io.micrometer.core.instrument.binder.cache.CaffeineCacheMetrics;
 
@@ -98,7 +101,7 @@ public <T> void put(CacheableCall<T> call, T value) {
         cache.put(call, CompletableFuture.completedFuture(value));
     }
 
-    public <T> T load(CacheableCall<T> key, Supplier<T> supplier, boolean logIfMiss) {
+    public <T> T load(AbstractCacheableCall<T> key, Supplier<T> supplier, boolean logIfMiss) {
         CompletableFuture<T> existingFuture = getIfPresent(key);
         if (existingFuture != null) {
             final T existingValue = existingFuture.getNow(null);

diff --git a/.../java/com/linecorp/centraldogma/server/internal/storage/repository/RepositoryWrapper.java b/.../java/com/linecorp/centraldogma/server/internal/storage/repository/RepositoryWrapper.java
@@ -36,6 +36,7 @@
 import com.linecorp.centraldogma.server.command.CommitResult;
 import com.linecorp.centraldogma.server.command.ContentTransformer;
 import com.linecorp.centraldogma.server.storage.project.Project;
+import com.linecorp.centraldogma.server.storage.repository.CacheableCall;
 import com.linecorp.centraldogma.server.storage.repository.DiffResultType;
 import com.linecorp.centraldogma.server.storage.repository.FindOption;
 import com.linecorp.centraldogma.server.storage.repository.Repository;
@@ -215,6 +216,11 @@ public <T> CompletableFuture<MergedEntry<T>> mergeFiles(Revision revision, Merge
         return unwrap().mergeFiles(revision, query);
     }
 
+    @Override
+    public <T> CompletableFuture<T> execute(CacheableCall<T> cacheableCall) {
+        return unwrap().execute(cacheableCall);
+    }
+
     @Override
     public void addListener(RepositoryListener listener) {
         unwrap().addListener(listener);

diff --git a/...com/linecorp/centraldogma/server/internal/storage/repository/cache/CacheableFindCall.java b/...com/linecorp/centraldogma/server/internal/storage/repository/cache/CacheableFindCall.java
@@ -26,11 +26,11 @@
 
 import com.linecorp.centraldogma.common.Entry;
 import com.linecorp.centraldogma.common.Revision;
-import com.linecorp.centraldogma.server.internal.storage.repository.CacheableCall;
+import com.linecorp.centraldogma.server.internal.storage.repository.git.AbstractCacheableCall;
 import com.linecorp.centraldogma.server.storage.repository.FindOption;
 import com.linecorp.centraldogma.server.storage.repository.Repository;
 
-final class CacheableFindCall extends CacheableCall<Map<String, Entry<?>>> {
+final class CacheableFindCall extends AbstractCacheableCall<Map<String, Entry<?>>> {
 
     final Revision revision;
     final String pathPattern;
@@ -50,7 +50,7 @@ final class CacheableFindCall extends CacheableCall<Map<String, Entry<?>>> {
     }
 
     @Override
-    protected int weigh(Map<String, Entry<?>> value) {
+    public int weigh(Map<String, Entry<?>> value) {
         int weight = 0;
         weight += pathPattern.length();
         weight += options.size();

diff --git a/...orp/centraldogma/server/internal/storage/repository/cache/CacheableFindLatestRevCall.java b/...orp/centraldogma/server/internal/storage/repository/cache/CacheableFindLatestRevCall.java
@@ -27,10 +27,10 @@
 import com.linecorp.armeria.common.util.Exceptions;
 import com.linecorp.centraldogma.common.EntryNotFoundException;
 import com.linecorp.centraldogma.common.Revision;
-import com.linecorp.centraldogma.server.internal.storage.repository.CacheableCall;
+import com.linecorp.centraldogma.server.internal.storage.repository.git.AbstractCacheableCall;
 import com.linecorp.centraldogma.server.storage.repository.Repository;
 
-final class CacheableFindLatestRevCall extends CacheableCall<Revision> {
+final class CacheableFindLatestRevCall extends AbstractCacheableCall<Revision> {
 
     static final Revision EMPTY = new Revision(Integer.MIN_VALUE);
     static final Revision ENTRY_NOT_FOUND = new Revision(Integer.MIN_VALUE);
@@ -57,7 +57,7 @@ final class CacheableFindLatestRevCall extends CacheableCall<Revision> {
     }
 
     @Override
-    protected int weigh(Revision value) {
+    public int weigh(Revision value) {
         return pathPattern.length();
     }
 

diff --git a/.../linecorp/centraldogma/server/internal/storage/repository/cache/CacheableHistoryCall.java b/.../linecorp/centraldogma/server/internal/storage/repository/cache/CacheableHistoryCall.java
@@ -26,10 +26,10 @@
 
 import com.linecorp.centraldogma.common.Commit;
 import com.linecorp.centraldogma.common.Revision;
-import com.linecorp.centraldogma.server.internal.storage.repository.CacheableCall;
+import com.linecorp.centraldogma.server.internal.storage.repository.git.AbstractCacheableCall;
 import com.linecorp.centraldogma.server.storage.repository.Repository;
 
-final class CacheableHistoryCall extends CacheableCall<List<Commit>> {
+final class CacheableHistoryCall extends AbstractCacheableCall<List<Commit>> {
 
     final Revision from;
     final Revision to;
@@ -52,7 +52,7 @@ final class CacheableHistoryCall extends CacheableCall<List<Commit>> {
     }
 
     @Override
-    protected int weigh(List<Commit> value) {
+    public int weigh(List<Commit> value) {
         int weight = 0;
         weight += pathPattern.length();
         for (Commit c : value) {

diff --git a/...necorp/centraldogma/server/internal/storage/repository/cache/CacheableMergeQueryCall.java b/...necorp/centraldogma/server/internal/storage/repository/cache/CacheableMergeQueryCall.java
@@ -32,10 +32,10 @@
 import com.linecorp.centraldogma.common.MergeSource;
 import com.linecorp.centraldogma.common.MergedEntry;
 import com.linecorp.centraldogma.common.Revision;
-import com.linecorp.centraldogma.server.internal.storage.repository.CacheableCall;
+import com.linecorp.centraldogma.server.internal.storage.repository.git.AbstractCacheableCall;
 import com.linecorp.centraldogma.server.storage.repository.Repository;
 
-final class CacheableMergeQueryCall extends CacheableCall<MergedEntry<?>> {
+final class CacheableMergeQueryCall extends AbstractCacheableCall<MergedEntry<?>> {
 
     private final Revision revision;
     private final MergeQuery<?> query;
@@ -57,7 +57,7 @@ final class CacheableMergeQueryCall extends CacheableCall<MergedEntry<?>> {
     }
 
     @Override
-    protected int weigh(MergedEntry<?> value) {
+    public int weigh(MergedEntry<?> value) {
         int weight = 0;
         final List<MergeSource> mergeSources = query.mergeSources();
         weight += mergeSources.size();

diff --git a/...inecorp/centraldogma/server/internal/storage/repository/cache/CacheableMultiDiffCall.java b/...inecorp/centraldogma/server/internal/storage/repository/cache/CacheableMultiDiffCall.java
@@ -26,11 +26,11 @@
 
 import com.linecorp.centraldogma.common.Change;
 import com.linecorp.centraldogma.common.Revision;
-import com.linecorp.centraldogma.server.internal.storage.repository.CacheableCall;
+import com.linecorp.centraldogma.server.internal.storage.repository.git.AbstractCacheableCall;
 import com.linecorp.centraldogma.server.storage.repository.DiffResultType;
 import com.linecorp.centraldogma.server.storage.repository.Repository;
 
-final class CacheableMultiDiffCall extends CacheableCall<Map<String, Change<?>>> {
+final class CacheableMultiDiffCall extends AbstractCacheableCall<Map<String, Change<?>>> {
 
     private final Revision from;
     private final Revision to;
@@ -54,7 +54,7 @@ final class CacheableMultiDiffCall extends CacheableCall<Map<String, Change<?>>>
     }
 
     @Override
-    protected int weigh(Map<String, Change<?>> value) {
+    public int weigh(Map<String, Change<?>> value) {
         int weight = 0;
         weight += pathPattern.length();
         for (Change<?> e : value.values()) {

diff --git a/...om/linecorp/centraldogma/server/internal/storage/repository/cache/CacheableQueryCall.java b/...om/linecorp/centraldogma/server/internal/storage/repository/cache/CacheableQueryCall.java
@@ -27,10 +27,10 @@
 import com.linecorp.centraldogma.common.Entry;
 import com.linecorp.centraldogma.common.Query;
 import com.linecorp.centraldogma.common.Revision;
-import com.linecorp.centraldogma.server.internal.storage.repository.CacheableCall;
+import com.linecorp.centraldogma.server.internal.storage.repository.git.AbstractCacheableCall;
 import com.linecorp.centraldogma.server.storage.repository.Repository;
 
-final class CacheableQueryCall extends CacheableCall<Entry<?>> {
+final class CacheableQueryCall extends AbstractCacheableCall<Entry<?>> {
 
     static final Entry<?> EMPTY = Entry.ofDirectory(new Revision(Integer.MAX_VALUE), "/");
 
@@ -49,7 +49,7 @@ final class CacheableQueryCall extends CacheableCall<Entry<?>> {
     }
 
     @Override
-    protected int weigh(Entry<?> value) {
+    public int weigh(Entry<?> value) {
         int weight = 0;
         weight += query.path().length();
         for (String e : query.expressions()) {

diff --git a/...necorp/centraldogma/server/internal/storage/repository/cache/CacheableSingleDiffCall.java b/...necorp/centraldogma/server/internal/storage/repository/cache/CacheableSingleDiffCall.java
@@ -26,10 +26,10 @@
 import com.linecorp.centraldogma.common.Change;
 import com.linecorp.centraldogma.common.Query;
 import com.linecorp.centraldogma.common.Revision;
-import com.linecorp.centraldogma.server.internal.storage.repository.CacheableCall;
+import com.linecorp.centraldogma.server.internal.storage.repository.git.AbstractCacheableCall;
 import com.linecorp.centraldogma.server.storage.repository.Repository;
 
-final class CacheableSingleDiffCall extends CacheableCall<Change<?>> {
+final class CacheableSingleDiffCall extends AbstractCacheableCall<Change<?>> {
 
     final Revision from;
     final Revision to;
@@ -50,7 +50,7 @@ final class CacheableSingleDiffCall extends CacheableCall<Change<?>> {
     }
 
     @Override
-    protected int weigh(Change<?> value) {
+    public int weigh(Change<?> value) {
         int weight = 0;
         weight += query.path().length();
         for (String e : query.expressions()) {

diff --git a/...com/linecorp/centraldogma/server/internal/storage/repository/cache/CachingRepository.java b/...com/linecorp/centraldogma/server/internal/storage/repository/cache/CachingRepository.java
@@ -27,6 +27,8 @@
 import java.util.concurrent.CancellationException;
 import java.util.concurrent.CompletableFuture;
 import java.util.concurrent.Executor;
+import java.util.concurrent.locks.Lock;
+import java.util.concurrent.locks.ReentrantLock;
 
 import com.linecorp.armeria.common.CommonPools;
 import com.linecorp.armeria.common.RequestContext;
@@ -47,6 +49,7 @@
 import com.linecorp.centraldogma.server.command.ContentTransformer;
 import com.linecorp.centraldogma.server.internal.storage.repository.RepositoryCache;
 import com.linecorp.centraldogma.server.storage.project.Project;
+import com.linecorp.centraldogma.server.storage.repository.CacheableCall;
 import com.linecorp.centraldogma.server.storage.repository.DiffResultType;
 import com.linecorp.centraldogma.server.storage.repository.FindOption;
 import com.linecorp.centraldogma.server.storage.repository.Repository;
@@ -57,6 +60,15 @@ final class CachingRepository implements Repository {
     private static final CancellationException CANCELLATION_EXCEPTION =
             Exceptions.clearTrace(new CancellationException("watch cancelled by caller"));
 
+    private static final Lock[] locks;
+
+    static {
+        locks = new Lock[8192];
+        for (int i = 0; i < locks.length; i++) {
+            locks[i] = new ReentrantLock();
+        }
+    }
+
     private final Repository repo;
     private final RepositoryCache cache;
 
@@ -269,13 +281,37 @@ public <T> CompletableFuture<MergedEntry<T>> mergeFiles(Revision revision, Merge
             }, executor()));
         }
 
+        // Do not call key.execute() because it may block the current thread.
         return Repository.super.mergeFiles(normalizedRevision, query).thenApply(mergedEntry -> {
             key.computedValue(mergedEntry);
             cache.get(key);
             return mergedEntry;
         });
     }
 
+    @Override
+    public <T> CompletableFuture<T> execute(CacheableCall<T> cacheableCall) {
+        final CompletableFuture<T> value = cache.getIfPresent(cacheableCall);
+        if (value != null) {
+            return unsafeCast(value.handleAsync((unused, cause) -> {
+                throwUnsafelyIfNonNull(cause);
+                return unused;
+            }, executor()));
+        }
+
+        final Lock lock = locks[Math.abs(cacheableCall.hashCode() % locks.length)];
+        lock.lock();
+        try {
+            return cacheableCall.execute().handleAsync((result, cause) -> {
+                throwUnsafelyIfNonNull(cause);
+                cache.put(cacheableCall, result);
+                return result;
+            }, executor());
+        } finally {
+            lock.unlock();
+        }
+    }
+
     @Override
     public void addListener(RepositoryListener listener) {
         repo.addListener(listener);

diff --git a/...nal/storage/repository/CacheableCall.java → ...repository/git/AbstractCacheableCall.java b/...nal/storage/repository/CacheableCall.java → ...repository/git/AbstractCacheableCall.java
@@ -14,20 +14,22 @@
  * under the License.
  */
 
-package com.linecorp.centraldogma.server.internal.storage.repository;
+package com.linecorp.centraldogma.server.internal.storage.repository.git;
 
 import static java.util.Objects.requireNonNull;
 
-import java.util.concurrent.CompletableFuture;
 import java.util.concurrent.locks.Lock;
 import java.util.concurrent.locks.ReentrantLock;
 
 import com.google.common.base.MoreObjects;
 
+import com.linecorp.centraldogma.server.storage.repository.CacheableCall;
 import com.linecorp.centraldogma.server.storage.repository.Repository;
 
-// XXX(trustin): Consider using reflection or AOP so that it takes less effort to add more call types.
-public abstract class CacheableCall<T> {
+/**
+ * A cacheable call which is used to retrieve a value from a repository.
+ */
+public abstract class AbstractCacheableCall<T> implements CacheableCall<T> {
 
     private static final Lock[] locks;
 
@@ -40,22 +42,27 @@ public abstract class CacheableCall<T> {
 
     final Repository repo;
 
-    protected CacheableCall(Repository repo) {
+    /**
+     * Creates a new instance.
+     */
+    protected AbstractCacheableCall(Repository repo) {
         this.repo = requireNonNull(repo, "repo");
     }
 
+    /**
+     * Returns the repository which this call is associated with.
+     */
     public final Repository repo() {
         return repo;
     }
 
-    public final Lock coarseGrainedLock() {
+    /**
+     * Returns the lock which is associated with this call.
+     */
+    public Lock coarseGrainedLock() {
         return locks[Math.abs(hashCode() % locks.length)];
     }
 
-    protected abstract int weigh(T value);
-
-    public abstract CompletableFuture<T> execute();
-
     @Override
     public int hashCode() {
         return System.identityHashCode(repo);
@@ -75,7 +82,7 @@ public boolean equals(Object obj) {
             return false;
         }
 
-        final CacheableCall<?> that = (CacheableCall<?>) obj;
+        final AbstractCacheableCall<?> that = (AbstractCacheableCall<?>) obj;
         return repo == that.repo;
     }
 
@@ -88,5 +95,8 @@ public final String toString() {
         return helper.toString();
     }
 
+    /**
+     * Overrides this method to add more information to the {@link #toString()} result.
+     */
     protected abstract void toString(MoreObjects.ToStringHelper helper);
 }