Stop including BuildConfiguration inside ConfiguredTargetKey: it's too heavyweight. For now, put a BuildConfigurationValue.Key in there. In the future, we may want to put some kind of "delta" key in.

PiperOrigin-RevId: 181673805

3 files changed, 76 insertions, 27 deletions

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 258223b3f0..4a80c7ab24 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
@@ -139,16 +139,29 @@ public final class ConfiguredTargetFunction implements SkyFunction {
     NestedSetBuilder<Package> transitivePackagesForPackageRootResolution =
         storeTransitivePackagesForPackageRootResolution ? NestedSetBuilder.stableOrder() : null;
     NestedSetBuilder<Label> transitiveLoadingRootCauses = NestedSetBuilder.stableOrder();
+
     ConfiguredTargetKey configuredTargetKey = (ConfiguredTargetKey) key.argument();
     Label label = configuredTargetKey.getLabel();
-
-    BuildConfiguration configuration = configuredTargetKey.getConfiguration();
-
-    PackageValue packageValue =
-        (PackageValue) env.getValue(PackageValue.key(label.getPackageIdentifier()));
-    if (packageValue == null) {
+    BuildConfiguration configuration = null;
+    ImmutableSet<SkyKey> packageAndMaybeConfiguration;
+    SkyKey packageKey = PackageValue.key(label.getPackageIdentifier());
+    SkyKey configurationKeyMaybe = configuredTargetKey.getConfigurationKey();
+    if (configurationKeyMaybe == null) {
+      packageAndMaybeConfiguration = ImmutableSet.of(packageKey);
+    } else {
+      packageAndMaybeConfiguration = ImmutableSet.of(packageKey, configurationKeyMaybe);
+    }
+    Map<SkyKey, SkyValue> packageAndMaybeConfigurationValues =
+        env.getValues(packageAndMaybeConfiguration);
+    if (env.valuesMissing()) {
       return null;
     }
+    PackageValue packageValue = (PackageValue) packageAndMaybeConfigurationValues.get(packageKey);
+    if (configurationKeyMaybe != null) {
+      configuration =
+          ((BuildConfigurationValue) packageAndMaybeConfigurationValues.get(configurationKeyMaybe))
+              .getConfiguration();
+    }
 
     // TODO(ulfjack): This tries to match the logic in TransitiveTargetFunction /
     // TargetMarkerFunction. Maybe we can merge the two?
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ConfiguredTargetKey.java b/src/main/java/com/google/devtools/build/lib/skyframe/ConfiguredTargetKey.java
index 6cdd9599c6..b69193fca1 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/ConfiguredTargetKey.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/ConfiguredTargetKey.java
@@ -23,25 +23,23 @@ import com.google.devtools.build.lib.analysis.config.BuildConfiguration;
 import com.google.devtools.build.lib.cmdline.Label;
 import com.google.devtools.build.lib.concurrent.BlazeInterners;
 import com.google.devtools.build.skyframe.SkyFunctionName;
+import com.google.devtools.build.skyframe.SkyKey;
 import java.util.Objects;
 import javax.annotation.Nullable;
 
 /**
- * A (Label, Configuration) pair. Note that this pair may be used to look up the generating action
- * of an artifact. Callers may want to ensure that they have the correct configuration for this
- * purpose by passing in {@link BuildConfiguration#getArtifactOwnerTransition} in preference to
- * the raw configuration.
+ * A (Label, Configuration key) pair. Note that this pair may be used to look up the generating
+ * action of an artifact.
  */
 public class ConfiguredTargetKey extends ActionLookupKey {
   private final Label label;
-  @Nullable
-  private final BuildConfiguration configuration;
+  @Nullable private final SkyKey configurationKey;
 
   private transient int hashCode;
 
-  private ConfiguredTargetKey(Label label, @Nullable BuildConfiguration configuration) {
+  private ConfiguredTargetKey(Label label, @Nullable SkyKey configurationKey) {
     this.label = Preconditions.checkNotNull(label);
-    this.configuration = configuration;
+    this.configurationKey = configurationKey;
   }
 
   public static ConfiguredTargetKey of(ConfiguredTarget configuredTarget) {
@@ -52,13 +50,32 @@ public class ConfiguredTargetKey extends ActionLookupKey {
   }
 
   /**
-   * Cache so that the number of ConfiguredTargetKey instances is {@code O(configured targets)} and
+   * Caches so that the number of ConfiguredTargetKey instances is {@code O(configured targets)} and
    * not {@code O(edges between configured targets)}.
    */
   private static final Interner<ConfiguredTargetKey> interner = BlazeInterners.newWeakInterner();
+  private static final Interner<HostConfiguredTargetKey> hostInterner =
+      BlazeInterners.newWeakInterner();
 
   public static ConfiguredTargetKey of(Label label, @Nullable BuildConfiguration configuration) {
-    return interner.intern(new ConfiguredTargetKey(label, configuration));
+    SkyKey configurationKey =
+        configuration == null
+            ? null
+            : BuildConfigurationValue.key(
+            configuration.fragmentClasses(), configuration.getOptions());
+    return of(
+        label,
+        configurationKey,
+        configuration != null && configuration.isHostConfiguration());
+  }
+
+  static ConfiguredTargetKey of(
+      Label label, @Nullable SkyKey configurationKey, boolean isHostConfiguration) {
+    if (isHostConfiguration) {
+      return hostInterner.intern(new HostConfiguredTargetKey(label, configurationKey));
+    } else {
+      return interner.intern(new ConfiguredTargetKey(label, configurationKey));
+    }
   }
 
   @Override
@@ -72,8 +89,8 @@ public class ConfiguredTargetKey extends ActionLookupKey {
   }
 
   @Nullable
-  public BuildConfiguration getConfiguration() {
-    return configuration;
+  SkyKey getConfigurationKey() {
+    return configurationKey;
   }
 
   @Override
@@ -103,8 +120,8 @@ public class ConfiguredTargetKey extends ActionLookupKey {
   }
 
   private int computeHashCode() {
-    int configVal = configuration == null ? 79 : configuration.hashCode();
-    return 31 * label.hashCode() + configVal;
+    int configVal = configurationKey == null ? 79 : configurationKey.hashCode();
+    return 31 * label.hashCode() + configVal + (isHostConfiguration() ? 41 : 0);
   }
 
   @Override
@@ -119,22 +136,40 @@ public class ConfiguredTargetKey extends ActionLookupKey {
       return false;
     }
     ConfiguredTargetKey other = (ConfiguredTargetKey) obj;
-    return Objects.equals(label, other.label) && Objects.equals(configuration, other.configuration);
+    return this.isHostConfiguration() == other.isHostConfiguration()
+        && Objects.equals(label, other.label)
+        && Objects.equals(configurationKey, other.configurationKey);
+  }
+
+  public boolean isHostConfiguration() {
+    return false;
   }
 
   public String prettyPrint() {
     if (label == null) {
       return "null";
     }
-    return (configuration != null && configuration.isHostConfiguration())
-        ? (label + " (host)") : label.toString();
+    return isHostConfiguration() ? (label + " (host)") : label.toString();
   }
 
   @Override
   public String toString() {
     return String.format(
-        "%s %s (%s %s)", label, (configuration == null ? "null" : configuration),
-        System.identityHashCode(this),
-        (configuration == null ? "null" : System.identityHashCode(configuration)));
+        "%s %s %s (%s)",
+        label,
+        configurationKey,
+        isHostConfiguration(),
+        System.identityHashCode(this));
+  }
+
+  private static class HostConfiguredTargetKey extends ConfiguredTargetKey {
+    private HostConfiguredTargetKey(Label label, @Nullable SkyKey configurationKey) {
+      super(label, configurationKey);
+    }
+
+    @Override
+    public boolean isHostConfiguration() {
+      return true;
+    }
   }
 }
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeBuildView.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeBuildView.java
index a137435872..81159c3ce8 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeBuildView.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeBuildView.java
@@ -347,7 +347,8 @@ public final class SkyframeBuildView {
       if (analysisRootCause != null) {
         eventBus.post(
             new AnalysisFailureEvent(
-                ConfiguredTargetKey.of(topLevelLabel, label.getConfiguration()),
+                ConfiguredTargetKey.of(
+                    topLevelLabel, label.getConfigurationKey(), label.isHostConfiguration()),
                 analysisRootCause));
       }
     }