Store a SkyKey inside BuildInfoCollectionValue.KeyAndConfig instead of a BuildConfiguration: BuildConfigurations are too heavy to be in SkyKeys.

This adds an extra dependency for BuildInfoCollectionValues, but there are not many in the graph, and the dep request is now batched, which is better than before.

PiperOrigin-RevId: 183106788

3 files changed, 53 insertions, 26 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/BuildInfoCollectionFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/BuildInfoCollectionFunction.java
index b8cbd4a6f7..e8230a6b15 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/BuildInfoCollectionFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/BuildInfoCollectionFunction.java
@@ -16,6 +16,7 @@ package com.google.devtools.build.lib.skyframe;
 import com.google.common.base.Preconditions;
 import com.google.common.base.Supplier;
 import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.ImmutableSet;
 import com.google.devtools.build.lib.actions.ActionKeyContext;
 import com.google.devtools.build.lib.actions.Artifact;
 import com.google.devtools.build.lib.actions.ArtifactFactory;
@@ -30,6 +31,7 @@ import com.google.devtools.build.lib.vfs.PathFragment;
 import com.google.devtools.build.skyframe.SkyFunction;
 import com.google.devtools.build.skyframe.SkyKey;
 import com.google.devtools.build.skyframe.SkyValue;
+import java.util.Map;
 
 /**
  * Creates a {@link BuildInfoCollectionValue}. Only depends on the unique
@@ -57,15 +59,18 @@ public class BuildInfoCollectionFunction implements SkyFunction {
   @Override
   public SkyValue compute(SkyKey skyKey, Environment env) throws InterruptedException {
     final BuildInfoKeyAndConfig keyAndConfig = (BuildInfoKeyAndConfig) skyKey.argument();
-    WorkspaceStatusValue infoArtifactValue =
-        (WorkspaceStatusValue) env.getValue(WorkspaceStatusValue.BUILD_INFO_KEY);
-    if (infoArtifactValue == null) {
-      return null;
-    }
-    WorkspaceNameValue nameValue = (WorkspaceNameValue) env.getValue(WorkspaceNameValue.key());
-    if (nameValue == null) {
+    ImmutableSet<SkyKey> keysToRequest =
+        ImmutableSet.of(
+            WorkspaceStatusValue.BUILD_INFO_KEY,
+            WorkspaceNameValue.key(),
+            keyAndConfig.getConfigKey());
+    Map<SkyKey, SkyValue> result = env.getValues(keysToRequest);
+    if (env.valuesMissing()) {
       return null;
     }
+    WorkspaceStatusValue infoArtifactValue =
+        (WorkspaceStatusValue) result.get(WorkspaceStatusValue.BUILD_INFO_KEY);
+    WorkspaceNameValue nameValue = (WorkspaceNameValue) result.get(WorkspaceNameValue.key());
     RepositoryName repositoryName = RepositoryName.createFromValidStrippedName(
         nameValue.getName());
 
@@ -87,7 +92,8 @@ public class BuildInfoCollectionFunction implements SkyFunction {
             .get(keyAndConfig.getInfoKey())
             .create(
                 context,
-                keyAndConfig.getConfig(),
+                ((BuildConfigurationValue) result.get(keyAndConfig.getConfigKey()))
+                    .getConfiguration(),
                 infoArtifactValue.getStableArtifact(),
                 infoArtifactValue.getVolatileArtifact(),
                 repositoryName),
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/BuildInfoCollectionValue.java b/src/main/java/com/google/devtools/build/lib/skyframe/BuildInfoCollectionValue.java
index 8695042cca..09ce2dbb2e 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/BuildInfoCollectionValue.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/BuildInfoCollectionValue.java
@@ -54,17 +54,19 @@ public class BuildInfoCollectionValue extends ActionLookupValue {
 
   public static BuildInfoKeyAndConfig key(
       BuildInfoFactory.BuildInfoKey key, BuildConfiguration config) {
-    return keyInterner.intern(new BuildInfoKeyAndConfig(key, config));
+    return keyInterner.intern(
+        new BuildInfoKeyAndConfig(key, ConfiguredTargetKey.keyFromConfiguration(config).key));
   }
 
   /** Key for BuildInfoCollectionValues. */
   public static class BuildInfoKeyAndConfig extends ActionLookupKey {
     private final BuildInfoFactory.BuildInfoKey infoKey;
-    private final BuildConfiguration config;
+    private final BuildConfigurationValue.Key configKey;
 
-    private BuildInfoKeyAndConfig(BuildInfoFactory.BuildInfoKey key, BuildConfiguration config) {
-      this.infoKey = Preconditions.checkNotNull(key, config);
-      this.config = Preconditions.checkNotNull(config, key);
+    private BuildInfoKeyAndConfig(
+        BuildInfoFactory.BuildInfoKey key, BuildConfigurationValue.Key configKey) {
+      this.infoKey = Preconditions.checkNotNull(key, configKey);
+      this.configKey = Preconditions.checkNotNull(configKey, key);
     }
 
     @Override
@@ -76,8 +78,8 @@ public class BuildInfoCollectionValue extends ActionLookupValue {
       return infoKey;
     }
 
-    BuildConfiguration getConfig() {
-      return config;
+    BuildConfigurationValue.Key getConfigKey() {
+      return configKey;
     }
 
     @Override
@@ -87,7 +89,7 @@ public class BuildInfoCollectionValue extends ActionLookupValue {
 
     @Override
     public int hashCode() {
-      return Objects.hash(infoKey, config);
+      return Objects.hash(infoKey, configKey);
     }
 
     @Override
@@ -102,7 +104,8 @@ public class BuildInfoCollectionValue extends ActionLookupValue {
         return false;
       }
       BuildInfoKeyAndConfig that = (BuildInfoKeyAndConfig) other;
-      return Objects.equals(this.infoKey, that.infoKey) && Objects.equals(this.config, that.config);
+      return Objects.equals(this.infoKey, that.infoKey)
+          && Objects.equals(this.configKey, that.configKey);
     }
   }
 }
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 8e8baa5226..5a562024b2 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
@@ -57,15 +57,8 @@ public class ConfiguredTargetKey extends ActionLookupKey {
       BlazeInterners.newWeakInterner();
 
   public static ConfiguredTargetKey of(Label label, @Nullable BuildConfiguration configuration) {
-    BuildConfigurationValue.Key configurationKey =
-        configuration == null
-            ? null
-            : BuildConfigurationValue.key(
-                configuration.fragmentClasses(), configuration.getOptions());
-    return of(
-        label,
-        configurationKey,
-        configuration != null && configuration.isHostConfiguration());
+    KeyAndHost keyAndHost = keyFromConfiguration(configuration);
+    return of(label, keyAndHost.key, keyAndHost.isHost);
   }
 
   static ConfiguredTargetKey of(
@@ -79,6 +72,15 @@ public class ConfiguredTargetKey extends ActionLookupKey {
     }
   }
 
+  static KeyAndHost keyFromConfiguration(@Nullable BuildConfiguration configuration) {
+    return configuration == null
+        ? KeyAndHost.NULL_INSTANCE
+        : new KeyAndHost(
+            BuildConfigurationValue.key(
+                configuration.fragmentClasses(), configuration.getOptions()),
+            configuration.isHostConfiguration());
+  }
+
   @Override
   public Label getLabel() {
     return label;
@@ -174,4 +176,20 @@ public class ConfiguredTargetKey extends ActionLookupKey {
       return true;
     }
   }
+
+  /**
+   * Simple wrapper class for turning a {@link BuildConfiguration} into a {@link
+   * BuildConfigurationValue.Key} and boolean isHost.
+   */
+  public static class KeyAndHost {
+    private static final KeyAndHost NULL_INSTANCE = new KeyAndHost(null, false);
+
+    @Nullable public final BuildConfigurationValue.Key key;
+    private final boolean isHost;
+
+    private KeyAndHost(@Nullable BuildConfigurationValue.Key key, boolean isHost) {
+      this.key = key;
+      this.isHost = isHost;
+    }
+  }
 }