fixup! Cache MerkleTree creation in RemoteExecutionService.java

bazelbuild · Aug 25, 2021 · 557671e · 557671e
1 parent 23ea382
commit 557671e
Show file tree

Hide file tree

Showing 2 changed files with 9 additions and 10 deletions.
diff --git a/src/main/java/com/google/devtools/build/lib/remote/RemoteExecutionService.java b/src/main/java/com/google/devtools/build/lib/remote/RemoteExecutionService.java
@@ -54,6 +54,8 @@
 import build.bazel.remote.execution.v2.Tree;
 import com.google.common.base.Preconditions;
 import com.google.common.base.Strings;
+import com.google.common.cache.Cache;
+import com.google.common.cache.CacheBuilder;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.ImmutableSet;
@@ -117,7 +119,6 @@
 import java.util.Objects;
 import java.util.SortedMap;
 import java.util.TreeSet;
-import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.ConcurrentLinkedQueue;
 import javax.annotation.Nullable;
 
@@ -136,7 +137,8 @@ public class RemoteExecutionService {
   @Nullable private final RemoteExecutionClient remoteExecutor;
   private final ImmutableSet<PathFragment> filesToDownload;
   @Nullable private final Path captureCorruptedOutputsDir;
-  private final ConcurrentHashMap<Object, MerkleTree> merkleTreeCache = new ConcurrentHashMap<>();
+  private final Cache<Object, MerkleTree> merkleTreeCache =
+      CacheBuilder.newBuilder().softValues().build();
 
   public RemoteExecutionService(
       Path execRoot,
@@ -355,7 +357,7 @@ private MerkleTree buildInputMerkleTree(SpawnExecutionContext context)
   private MerkleTree buildMerkleTreeVisitor(
       Object nodeKey, SpawnExecutionContext.InputWalker walker, MetadataProvider metadataProvider)
       throws IOException, ForbiddenActionInputException {
-    MerkleTree result = merkleTreeCache.get(nodeKey);
+    MerkleTree result = merkleTreeCache.getIfPresent(nodeKey);
     if (result == null) {
       ConcurrentLinkedQueue<MerkleTree> subMerkleTrees = new ConcurrentLinkedQueue();
       subMerkleTrees.add(MerkleTree.build(
@@ -366,10 +368,7 @@ private MerkleTree buildMerkleTreeVisitor(
                 subNodeKey, subWalker, metadataProvider));
           });
       result = MerkleTree.merge(subMerkleTrees, digestUtil);
-      MerkleTree existingResult = merkleTreeCache.putIfAbsent(nodeKey, result);
-      if (existingResult != null) {
-        result = existingResult;
-      }
+      merkleTreeCache.put(nodeKey, result);
     }
     return result;
   }

diff --git a/src/main/java/com/google/devtools/build/lib/remote/options/RemoteOptions.java b/src/main/java/com/google/devtools/build/lib/remote/options/RemoteOptions.java
@@ -505,13 +505,13 @@ public RemoteOutputsStrategyConverter() {
   public boolean remoteVerifyDownloads;
 
   @Option(
-      name = "remote_cache_merkle_trees",
-      defaultValue = "true",
+      name = "experimental_remote_cache_merkle_trees",
+      defaultValue = "false",
       documentationCategory = OptionDocumentationCategory.REMOTE,
       effectTags = {OptionEffectTag.UNKNOWN},
       help =
           "Let the Merkle tree calculation be cached to improve speed for remote cache hit "
-              + "checking. Disabling this option will decrease the memory foot print.")
+              + "checking. Enabling this option will increase the memory foot print.")
   public boolean remoteCacheMerkleTrees;
 
   @Option(