From 40d0077016dee31865af5246cbd6b2953d6386b3 Mon Sep 17 00:00:00 2001
From: janakr <janakr@google.com>
Date: Wed, 14 Feb 2018 14:08:45 -0800
Subject: Start slimming down ToolchainResolutionKey: get rid of
 BuildConfiguration. The real blocker is PlatformInfo, which is coming.

PiperOrigin-RevId: 185742130
---
 .../build/lib/skyframe/AspectFunction.java         |  3 +-
 .../lib/skyframe/ConfiguredTargetFunction.java     |  6 ++-
 .../lib/skyframe/RegisteredToolchainsFunction.java |  8 +++-
 .../lib/skyframe/RegisteredToolchainsValue.java    | 50 ++++++++++++++++++++--
 .../build/lib/skyframe/SkyframeExecutor.java       |  9 +++-
 .../lib/skyframe/ToolchainResolutionFunction.java  | 10 ++++-
 .../lib/skyframe/ToolchainResolutionValue.java     | 13 +++---
 .../devtools/build/lib/skyframe/ToolchainUtil.java | 16 ++++---
 8 files changed, 92 insertions(+), 23 deletions(-)

(limited to 'src/main')

diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/AspectFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/AspectFunction.java
index a5e12144f9..9d3f49cb26 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/AspectFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/AspectFunction.java
@@ -357,7 +357,8 @@ public final class AspectFunction implements SkyFunction {
                     aspect.getDescriptor().getDescription(),
                     associatedConfiguredTargetAndTarget.getTarget().toString()),
                 requiredToolchains,
-                aspectConfiguration);
+                aspectConfiguration,
+                key.getAspectConfigurationKey());
       } catch (ToolchainContextException e) {
         // TODO(katre): better error handling
         throw new AspectCreationException(e.getMessage());
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ConfiguredTargetFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/ConfiguredTargetFunction.java
index ca24fdf829..963a8bedb5 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/ConfiguredTargetFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/ConfiguredTargetFunction.java
@@ -246,7 +246,11 @@ public final class ConfiguredTargetFunction implements SkyFunction {
               rule.getRuleClassObject().getRequiredToolchains();
           toolchainContext =
               ToolchainUtil.createToolchainContext(
-                  env, rule.toString(), requiredToolchains, configuration);
+                  env,
+                  rule.toString(),
+                  requiredToolchains,
+                  configuration,
+                  configuredTargetKey.getConfigurationKey());
           if (env.valuesMissing()) {
             return null;
           }
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/RegisteredToolchainsFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/RegisteredToolchainsFunction.java
index 7a2c948efe..d6a8bfe1dd 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/RegisteredToolchainsFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/RegisteredToolchainsFunction.java
@@ -43,7 +43,13 @@ public class RegisteredToolchainsFunction implements SkyFunction {
   public SkyValue compute(SkyKey skyKey, Environment env)
       throws SkyFunctionException, InterruptedException {
 
-    BuildConfiguration configuration = (BuildConfiguration) skyKey.argument();
+    BuildConfigurationValue buildConfigurationValue =
+        (BuildConfigurationValue)
+            env.getValue(((RegisteredToolchainsValue.Key) skyKey).getConfigurationKey());
+    if (env.valuesMissing()) {
+      return null;
+    }
+    BuildConfiguration configuration = buildConfigurationValue.getConfiguration();
 
     ImmutableList.Builder<Label> registeredToolchainLabels = new ImmutableList.Builder<>();
 
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/RegisteredToolchainsValue.java b/src/main/java/com/google/devtools/build/lib/skyframe/RegisteredToolchainsValue.java
index 23d30a07a8..1fb6d11c5f 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/RegisteredToolchainsValue.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/RegisteredToolchainsValue.java
@@ -16,11 +16,14 @@ package com.google.devtools.build.lib.skyframe;
 
 import com.google.auto.value.AutoValue;
 import com.google.common.collect.ImmutableList;
-import com.google.devtools.build.lib.analysis.config.BuildConfiguration;
+import com.google.common.collect.Interner;
 import com.google.devtools.build.lib.analysis.platform.DeclaredToolchainInfo;
-import com.google.devtools.build.skyframe.LegacySkyKey;
+import com.google.devtools.build.lib.concurrent.BlazeInterners;
+import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec;
+import com.google.devtools.build.skyframe.SkyFunctionName;
 import com.google.devtools.build.skyframe.SkyKey;
 import com.google.devtools.build.skyframe.SkyValue;
+import java.util.Objects;
 
 /**
  * A value which represents every toolchain known to Bazel and available for toolchain resolution.
@@ -29,8 +32,47 @@ import com.google.devtools.build.skyframe.SkyValue;
 public abstract class RegisteredToolchainsValue implements SkyValue {
 
   /** Returns the {@link SkyKey} for {@link RegisteredToolchainsValue}s. */
-  public static SkyKey key(BuildConfiguration configuration) {
-    return LegacySkyKey.create(SkyFunctions.REGISTERED_TOOLCHAINS, configuration);
+  public static Key key(BuildConfigurationValue.Key configurationKey) {
+    return Key.of(configurationKey);
+  }
+
+  @AutoCodec
+  static class Key implements SkyKey {
+    private static final Interner<Key> interners = BlazeInterners.newWeakInterner();
+    private final BuildConfigurationValue.Key configurationKey;
+
+    private Key(BuildConfigurationValue.Key configurationKey) {
+      this.configurationKey = configurationKey;
+    }
+
+    @AutoCodec.Instantiator
+    @AutoCodec.VisibleForSerialization
+    static Key of(BuildConfigurationValue.Key configurationKey) {
+      return interners.intern(new Key(configurationKey));
+    }
+
+    @Override
+    public SkyFunctionName functionName() {
+      return SkyFunctions.REGISTERED_TOOLCHAINS;
+    }
+
+    BuildConfigurationValue.Key getConfigurationKey() {
+      return configurationKey;
+    }
+
+    @Override
+    public boolean equals(Object obj) {
+      if (!(obj instanceof Key)) {
+        return false;
+      }
+      Key that = (Key) obj;
+      return Objects.equals(this.configurationKey, that.configurationKey);
+    }
+
+    @Override
+    public int hashCode() {
+      return configurationKey.hashCode();
+    }
   }
 
   public static RegisteredToolchainsValue create(
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java
index a0245b7d4b..df71799ec8 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java
@@ -895,7 +895,14 @@ public abstract class SkyframeExecutor implements WalkableGraphFactory {
       throws ToolchainContextException, InterruptedException {
     SkyFunctionEnvironmentForTesting env =
         new SkyFunctionEnvironmentForTesting(buildDriver, eventHandler, this);
-    return ToolchainUtil.createToolchainContext(env, "", requiredToolchains, config);
+    return ToolchainUtil.createToolchainContext(
+        env,
+        "",
+        requiredToolchains,
+        config,
+        config == null
+            ? null
+            : BuildConfigurationValue.key(config.fragmentClasses(), config.getOptions()));
   }
 
   /**
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ToolchainResolutionFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/ToolchainResolutionFunction.java
index d415ef0078..6a8dcca3d6 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/ToolchainResolutionFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/ToolchainResolutionFunction.java
@@ -49,7 +49,13 @@ public class ToolchainResolutionFunction implements SkyFunction {
       throws SkyFunctionException, InterruptedException {
     ToolchainResolutionKey key = (ToolchainResolutionKey) skyKey.argument();
 
-    BuildConfiguration configuration = key.configuration();
+    // This call could be combined with the call below, but this SkyFunction is evaluated so rarely
+    // it's not worth optimizing.
+    BuildConfigurationValue value = (BuildConfigurationValue) env.getValue(key.configurationKey());
+    if (env.valuesMissing()) {
+      return null;
+    }
+    BuildConfiguration configuration = value.getConfiguration();
 
     // Get all toolchains.
     RegisteredToolchainsValue toolchains;
@@ -57,7 +63,7 @@ public class ToolchainResolutionFunction implements SkyFunction {
       toolchains =
           (RegisteredToolchainsValue)
               env.getValueOrThrow(
-                  RegisteredToolchainsValue.key(configuration),
+                  RegisteredToolchainsValue.key(key.configurationKey()),
                   InvalidToolchainLabelException.class,
                   EvalException.class);
       if (toolchains == null) {
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ToolchainResolutionValue.java b/src/main/java/com/google/devtools/build/lib/skyframe/ToolchainResolutionValue.java
index 1f39059adf..f71c3bdf07 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/ToolchainResolutionValue.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/ToolchainResolutionValue.java
@@ -17,7 +17,6 @@ package com.google.devtools.build.lib.skyframe;
 import com.google.auto.value.AutoValue;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
-import com.google.devtools.build.lib.analysis.config.BuildConfiguration;
 import com.google.devtools.build.lib.analysis.platform.PlatformInfo;
 import com.google.devtools.build.lib.cmdline.Label;
 import com.google.devtools.build.skyframe.SkyFunctionName;
@@ -36,12 +35,12 @@ public abstract class ToolchainResolutionValue implements SkyValue {
 
   // A key representing the input data.
   public static SkyKey key(
-      BuildConfiguration configuration,
+      BuildConfigurationValue.Key configurationKey,
       Label toolchainType,
       PlatformInfo targetPlatform,
       List<PlatformInfo> availableExecutionPlatforms) {
     return ToolchainResolutionKey.create(
-        configuration, toolchainType, targetPlatform, availableExecutionPlatforms);
+        configurationKey, toolchainType, targetPlatform, availableExecutionPlatforms);
   }
 
   /** {@link SkyKey} implementation used for {@link ToolchainResolutionFunction}. */
@@ -52,16 +51,16 @@ public abstract class ToolchainResolutionValue implements SkyValue {
       return SkyFunctions.TOOLCHAIN_RESOLUTION;
     }
 
-    public abstract BuildConfiguration configuration();
+    abstract BuildConfigurationValue.Key configurationKey();
 
     public abstract Label toolchainType();
 
     public abstract PlatformInfo targetPlatform();
 
-    public abstract ImmutableList<PlatformInfo> availableExecutionPlatforms();
+    abstract ImmutableList<PlatformInfo> availableExecutionPlatforms();
 
-    public static ToolchainResolutionKey create(
-        BuildConfiguration configuration,
+    static ToolchainResolutionKey create(
+        BuildConfigurationValue.Key configuration,
         Label toolchainType,
         PlatformInfo targetPlatform,
         List<PlatformInfo> availableExecutionPlatforms) {
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ToolchainUtil.java b/src/main/java/com/google/devtools/build/lib/skyframe/ToolchainUtil.java
index de14ac60fe..28d5d9a666 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/ToolchainUtil.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/ToolchainUtil.java
@@ -55,11 +55,12 @@ public class ToolchainUtil {
       Environment env,
       String targetDescription,
       Set<Label> requiredToolchains,
-      @Nullable BuildConfiguration configuration)
+      @Nullable BuildConfiguration configuration,
+      BuildConfigurationValue.Key configurationKey)
       throws ToolchainContextException, InterruptedException {
 
     // In some cases this is called with a missing configuration, so we skip toolchain context.
-    if (configuration == null) {
+    if (configurationKey == null) {
       return null;
     }
 
@@ -78,7 +79,7 @@ public class ToolchainUtil {
 
     ImmutableBiMap<Label, Label> resolvedLabels =
         resolveToolchainLabels(
-            env, requiredToolchains, configuration, executionPlatform, targetPlatform);
+            env, requiredToolchains, executionPlatform, targetPlatform, configurationKey);
     if (resolvedLabels == null) {
       return null;
     }
@@ -173,9 +174,9 @@ public class ToolchainUtil {
   private static ImmutableBiMap<Label, Label> resolveToolchainLabels(
       Environment env,
       Set<Label> requiredToolchains,
-      BuildConfiguration configuration,
       PlatformInfo executionPlatform,
-      PlatformInfo targetPlatform)
+      PlatformInfo targetPlatform,
+      BuildConfigurationValue.Key configurationKey)
       throws InterruptedException, ToolchainContextException {
 
     // If there are no required toolchains, bail out early.
@@ -188,7 +189,10 @@ public class ToolchainUtil {
     for (Label toolchainType : requiredToolchains) {
       registeredToolchainKeys.add(
           ToolchainResolutionValue.key(
-              configuration, toolchainType, targetPlatform, ImmutableList.of(executionPlatform)));
+              configurationKey,
+              toolchainType,
+              targetPlatform,
+              ImmutableList.of(executionPlatform)));
     }
 
     Map<
-- 
cgit v1.2.3