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
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); }