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
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 b8cbd4a..e8230a6 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 @@
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.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 @@
@Override
public SkyValue compute(SkyKey skyKey, Environment env) throws InterruptedException {
final BuildInfoKeyAndConfig keyAndConfig = (BuildInfoKeyAndConfig) skyKey.argument();
+ 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) env.getValue(WorkspaceStatusValue.BUILD_INFO_KEY);
- if (infoArtifactValue == null) {
- return null;
- }
- WorkspaceNameValue nameValue = (WorkspaceNameValue) env.getValue(WorkspaceNameValue.key());
- if (nameValue == null) {
- return null;
- }
+ (WorkspaceStatusValue) result.get(WorkspaceStatusValue.BUILD_INFO_KEY);
+ WorkspaceNameValue nameValue = (WorkspaceNameValue) result.get(WorkspaceNameValue.key());
RepositoryName repositoryName = RepositoryName.createFromValidStrippedName(
nameValue.getName());
@@ -87,7 +92,8 @@
.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 8695042..09ce2db 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 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 @@
return infoKey;
}
- BuildConfiguration getConfig() {
- return config;
+ BuildConfigurationValue.Key getConfigKey() {
+ return configKey;
}
@Override
@@ -87,7 +89,7 @@
@Override
public int hashCode() {
- return Objects.hash(infoKey, config);
+ return Objects.hash(infoKey, configKey);
}
@Override
@@ -102,7 +104,8 @@
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 8e8baa5..5a56202 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 @@
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 @@
}
}
+ 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 @@
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;
+ }
+ }
}
