commit 4c8c152da4a5dee33c26154379486689df6e3035
author nharmata <nharmata@google.com> Mon Feb 25 14:25:44 2019 -0800
committer Copybara-Service <copybara-worker@google.com> Mon Feb 25 14:27:44 2019 -0800
tree af8d5f0cd7277b0a06449fcd35178b858c8eb58d
parent 3dd18be5096aae257d80acfef6fcce4bb48c2417

Use two layers of LoadingCaches, rather than two layers of ConcurrentHashMaps,
to implement Aspect#forNative.

See the class javadoc in FastHotKeyAtomicLongMap.java for a discussion of the
contention issues with ConcurrentHashMap#compute.

RELNOTES: None
PiperOrigin-RevId: 235602802

src/main/java/com/google/devtools/build/lib/packages/Aspect.java

diff --git a/src/main/java/com/google/devtools/build/lib/packages/Aspect.java b/src/main/java/com/google/devtools/build/lib/packages/Aspect.java
index b0ccfe6..3ea8682 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/Aspect.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/Aspect.java
@@ -14,6 +14,9 @@
 package com.google.devtools.build.lib.packages;
 
 import com.google.common.base.Preconditions;
+import com.google.common.cache.CacheBuilder;
+import com.google.common.cache.CacheLoader;
+import com.google.common.cache.LoadingCache;
 import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable;
 import com.google.devtools.build.lib.skyframe.serialization.DeserializationContext;
 import com.google.devtools.build.lib.skyframe.serialization.ObjectCodec;
@@ -22,8 +25,6 @@
 import com.google.protobuf.CodedInputStream;
 import com.google.protobuf.CodedOutputStream;
 import java.io.IOException;
-import java.util.Map;
-import java.util.concurrent.ConcurrentHashMap;
 
 /**
  * An instance of a given {@code AspectClass} with loaded definition and parameters.
@@ -44,8 +45,18 @@
    *
    * <p>Caching of Skylark aspects is not yet implemented.
    */
-  private static final Map<NativeAspectClass, Map<AspectParameters, AspectDefinition>>
-      definitionCache = new ConcurrentHashMap<>();
+  private static final LoadingCache<
+          NativeAspectClass, LoadingCache<AspectParameters, AspectDefinition>>
+      definitionCache =
+          CacheBuilder.newBuilder()
+              .build(
+                  CacheLoader.from(
+                      nativeAspectClass ->
+                          CacheBuilder.newBuilder()
+                              .build(
+                                  CacheLoader.from(
+                                      aspectParameters ->
+                                          nativeAspectClass.getDefinition(aspectParameters)))));
 
   private final AspectDescriptor aspectDescriptor;
   private final AspectDefinition aspectDefinition;
@@ -63,9 +74,7 @@
   public static Aspect forNative(
       NativeAspectClass nativeAspectClass, AspectParameters parameters) {
     AspectDefinition definition =
-        definitionCache
-            .computeIfAbsent(nativeAspectClass, key -> new ConcurrentHashMap<>())
-            .computeIfAbsent(parameters, nativeAspectClass::getDefinition);
+        definitionCache.getUnchecked(nativeAspectClass).getUnchecked(parameters);
     return new Aspect(nativeAspectClass, definition, parameters);
   }