commit a9100add682b190bb8e27447a6b6f95a0e4cca91
author Googler <noreply@google.com> Thu Jun 06 13:34:36 2019 -0700
committer Copybara-Service <copybara-worker@google.com> Thu Jun 06 13:35:41 2019 -0700
tree 40b9b18842ef78cedf522fb3afcd6245a03f79f6
parent 13ad6f495c61a6dc9c18141058e2cefffeb44367

Cache results of PlatformMappingValue.map.

Although the return value was already being interned, it needs to go through BuildOptions.diffForReconstruction with a new (uncached) instance prior to getting a hit on the interner.

PiperOrigin-RevId: 251916165

src/main/java/com/google/devtools/build/lib/skyframe/PlatformMappingValue.java

diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/PlatformMappingValue.java b/src/main/java/com/google/devtools/build/lib/skyframe/PlatformMappingValue.java
index 92b2acb..a3c6e0c 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/PlatformMappingValue.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/PlatformMappingValue.java
@@ -16,12 +16,14 @@
 
 import com.google.common.base.MoreObjects;
 import com.google.common.base.Preconditions;
+import com.google.common.base.Throwables;
 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.Interner;
 import com.google.common.collect.Iterables;
+import com.google.common.util.concurrent.UncheckedExecutionException;
 import com.google.devtools.build.lib.analysis.PlatformOptions;
 import com.google.devtools.build.lib.analysis.config.BuildOptions;
 import com.google.devtools.build.lib.cmdline.Label;
@@ -50,6 +52,7 @@
  * the design</a> for more details on how the mapping can be defined and the desired logic on how it
  * is applied to configuration keys.
  */
+@AutoCodec
 public final class PlatformMappingValue implements SkyValue {
 
   public static final PlatformMappingValue EMPTY =
@@ -136,7 +139,8 @@
 
   private final Map<Label, Collection<String>> platformsToFlags;
   private final Map<Collection<String>, Label> flagsToPlatforms;
-  private final transient Cache<Collection<String>, OptionsParsingResult> parserCache;
+  private final Cache<Collection<String>, OptionsParsingResult> parserCache;
+  private final Cache<BuildConfigurationValue.Key, BuildConfigurationValue.Key> mappingCache;
 
   /**
    * Creates a new mapping value which will match on the given platforms (if a target platform is
@@ -156,6 +160,7 @@
         CacheBuilder.newBuilder()
             .initialCapacity(platformsToFlags.size() + flagsToPlatforms.size())
             .build();
+    this.mappingCache = CacheBuilder.newBuilder().weakKeys().build();
   }
 
   /**
@@ -182,6 +187,17 @@
   public BuildConfigurationValue.Key map(
       BuildConfigurationValue.Key original, BuildOptions defaultBuildOptions)
       throws OptionsParsingException {
+    try {
+      return mappingCache.get(original, () -> computeMapping(original, defaultBuildOptions));
+    } catch (ExecutionException | UncheckedExecutionException e) {
+      Throwables.propagateIfPossible(e.getCause(), OptionsParsingException.class);
+      throw new IllegalStateException(e);
+    }
+  }
+
+  private BuildConfigurationValue.Key computeMapping(
+      BuildConfigurationValue.Key original, BuildOptions defaultBuildOptions)
+      throws OptionsParsingException {
     BuildOptions.OptionsDiffForReconstruction originalDiff = original.getOptionsDiff();
     BuildOptions originalOptions = defaultBuildOptions.applyDiff(originalDiff);