Convert ActionLookupKey implementations to directly implement SkyKey, removing the layer of indirection of getting SkyKey out of ActionLookupKey, which uses more memory for no reason.
PiperOrigin-RevId: 181658615
diff --git a/src/main/java/com/google/devtools/build/lib/actions/ActionLookupValue.java b/src/main/java/com/google/devtools/build/lib/actions/ActionLookupValue.java
index ffe650a..f1d91ba 100644
--- a/src/main/java/com/google/devtools/build/lib/actions/ActionLookupValue.java
+++ b/src/main/java/com/google/devtools/build/lib/actions/ActionLookupValue.java
@@ -23,8 +23,6 @@
import com.google.devtools.build.lib.actions.MutableActionGraph.ActionConflictException;
import com.google.devtools.build.lib.cmdline.Label;
import com.google.devtools.build.lib.concurrent.ThreadSafety.ThreadSafe;
-import com.google.devtools.build.skyframe.LegacySkyKey;
-import com.google.devtools.build.skyframe.SkyFunctionName;
import com.google.devtools.build.skyframe.SkyKey;
import com.google.devtools.build.skyframe.SkyValue;
import java.util.ArrayList;
@@ -151,11 +149,6 @@
return getStringHelper().toString();
}
- @VisibleForTesting
- public static SkyKey key(ActionLookupKey ownerKey) {
- return ownerKey.getSkyKey();
- }
-
public int getNumActions() {
return actions.size();
}
@@ -190,43 +183,14 @@
}
/**
- * ArtifactOwner is not a SkyKey, but we wish to convert any ArtifactOwner into a SkyKey as simply
- * as possible. To that end, all subclasses of ActionLookupValue "own" artifacts with
- * ArtifactOwners that are subclasses of ActionLookupKey. This allows callers to easily find the
- * value key, while remaining agnostic to what ActionLookupValues actually exist.
- *
- * <p>The methods of this class should only be called by {@link ActionLookupValue#key}.
+ * All subclasses of ActionLookupValue "own" artifacts with {@link ArtifactOwner}s that are
+ * subclasses of ActionLookupKey. This allows callers to easily find the value key, while
+ * remaining agnostic to what ActionLookupValues actually exist.
*/
- public abstract static class ActionLookupKey implements ArtifactOwner {
+ public abstract static class ActionLookupKey implements ArtifactOwner, SkyKey {
@Override
public Label getLabel() {
return null;
}
-
- /**
- * Subclasses must override this to specify their specific value type, unless they override
- * {@link #getSkyKey}, in which case they are free not to implement this method.
- */
- protected abstract SkyFunctionName getType();
-
- protected SkyKey getSkyKeyInternal() {
- return LegacySkyKey.create(getType(), this);
- }
-
- /**
- * Prefer {@link ActionLookupValue#key} to calling this method directly.
- *
- * <p>Subclasses may override {@link #getSkyKeyInternal} if the {@link SkyKey} argument should
- * not be this {@link ActionLookupKey} itself.
- */
- public final SkyKey getSkyKey() {
- SkyKey result = getSkyKeyInternal();
- Preconditions.checkState(
- result.argument() instanceof ActionLookupKey,
- "Not ActionLookupKey for %s: %s",
- this,
- result);
- return result;
- }
}
}
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/AspectResolver.java b/src/main/java/com/google/devtools/build/lib/analysis/AspectResolver.java
index d5e1f00..35eeae8 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/AspectResolver.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/AspectResolver.java
@@ -156,7 +156,7 @@
dependentAspects.build(),
aspectDeps.getAspect(),
dep.getAspectConfiguration(aspectDeps.getAspect()));
- result.put(aspectKey.getAspectDescriptor(), aspectKey.getSkyKey());
+ result.put(aspectKey.getAspectDescriptor(), aspectKey);
return aspectKey;
}
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/BuildView.java b/src/main/java/com/google/devtools/build/lib/analysis/BuildView.java
index 660f69b..4b58054 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/BuildView.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/BuildView.java
@@ -490,13 +490,15 @@
eventBus.post(new TargetConfiguredEvent(target, byLabel.get(target.getLabel())));
}
- List<ConfiguredTargetKey> topLevelCtKeys = Lists.transform(topLevelTargetsWithConfigs,
- new Function<TargetAndConfiguration, ConfiguredTargetKey>() {
- @Override
- public ConfiguredTargetKey apply(TargetAndConfiguration node) {
- return new ConfiguredTargetKey(node.getLabel(), node.getConfiguration());
- }
- });
+ List<ConfiguredTargetKey> topLevelCtKeys =
+ Lists.transform(
+ topLevelTargetsWithConfigs,
+ new Function<TargetAndConfiguration, ConfiguredTargetKey>() {
+ @Override
+ public ConfiguredTargetKey apply(TargetAndConfiguration node) {
+ return ConfiguredTargetKey.of(node.getLabel(), node.getConfiguration());
+ }
+ });
Multimap<Pair<Label, String>, BuildConfiguration> aspectConfigurations =
ArrayListMultimap.create();
@@ -646,13 +648,14 @@
Iterables.addAll(artifactsToBuild, baselineCoverageArtifacts);
if (coverageReportActionFactory != null) {
CoverageReportActionsWrapper actionsWrapper;
- actionsWrapper = coverageReportActionFactory.createCoverageReportActionsWrapper(
- eventHandler,
- directories,
- allTargetsToTest,
- baselineCoverageArtifacts,
- getArtifactFactory(),
- CoverageReportValue.ARTIFACT_OWNER);
+ actionsWrapper =
+ coverageReportActionFactory.createCoverageReportActionsWrapper(
+ eventHandler,
+ directories,
+ allTargetsToTest,
+ baselineCoverageArtifacts,
+ getArtifactFactory(),
+ CoverageReportValue.COVERAGE_REPORT_KEY);
if (actionsWrapper != null) {
ImmutableList<ActionAnalysisMetadata> actions = actionsWrapper.getActions();
skyframeExecutor.injectCoverageReportData(actions);
@@ -673,7 +676,7 @@
public ActionAnalysisMetadata getGeneratingAction(Artifact artifact) {
ArtifactOwner artifactOwner = artifact.getArtifactOwner();
if (artifactOwner instanceof ActionLookupValue.ActionLookupKey) {
- SkyKey key = ActionLookupValue.key((ActionLookupValue.ActionLookupKey) artifactOwner);
+ SkyKey key = (ActionLookupValue.ActionLookupKey) artifactOwner;
ActionLookupValue val;
try {
val = (ActionLookupValue) graph.getValue(key);
@@ -1081,7 +1084,7 @@
new CachingAnalysisEnvironment(
getArtifactFactory(),
skyframeExecutor.getActionKeyContext(),
- new ConfiguredTargetKey(target.getLabel(), targetConfig),
+ ConfiguredTargetKey.of(target.getLabel(), targetConfig),
/*isSystemEnv=*/ false,
targetConfig.extendedSanityChecks(),
eventHandler,
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/CachingAnalysisEnvironment.java b/src/main/java/com/google/devtools/build/lib/analysis/CachingAnalysisEnvironment.java
index 7d17950..81d5d2e 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/CachingAnalysisEnvironment.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/CachingAnalysisEnvironment.java
@@ -297,14 +297,14 @@
@Override
public Artifact getStableWorkspaceStatusArtifact() throws InterruptedException {
- return ((WorkspaceStatusValue) skyframeEnv.getValue(WorkspaceStatusValue.SKY_KEY))
- .getStableArtifact();
+ return ((WorkspaceStatusValue) skyframeEnv.getValue(WorkspaceStatusValue.BUILD_INFO_KEY))
+ .getStableArtifact();
}
@Override
public Artifact getVolatileWorkspaceStatusArtifact() throws InterruptedException {
- return ((WorkspaceStatusValue) skyframeEnv.getValue(WorkspaceStatusValue.SKY_KEY))
- .getVolatileArtifact();
+ return ((WorkspaceStatusValue) skyframeEnv.getValue(WorkspaceStatusValue.BUILD_INFO_KEY))
+ .getVolatileArtifact();
}
// See SkyframeBuildView#getWorkspaceStatusValues for the code that this method is attempting to
@@ -327,8 +327,7 @@
throws InterruptedException {
boolean stamp = AnalysisUtils.isStampingEnabled(ruleContext, config);
BuildInfoCollectionValue collectionValue =
- (BuildInfoCollectionValue) skyframeEnv.getValue(BuildInfoCollectionValue.key(
- new BuildInfoCollectionValue.BuildInfoKeyAndConfig(key, config)));
+ (BuildInfoCollectionValue) skyframeEnv.getValue(BuildInfoCollectionValue.key(key, config));
if (collectionValue == null) {
throw collectDebugInfoAndCrash(key, config);
}
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredTargetFactory.java b/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredTargetFactory.java
index 25a33be..913b6f1 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredTargetFactory.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredTargetFactory.java
@@ -166,8 +166,10 @@
Root root = rule.hasBinaryOutput()
? configuration.getBinDirectory(rule.getRepository())
: configuration.getGenfilesDirectory(rule.getRepository());
- ArtifactOwner owner = new ConfiguredTargetKey(rule.getLabel(),
- getArtifactOwnerConfiguration(analysisEnvironment.getSkyframeEnv(), configuration));
+ ArtifactOwner owner =
+ ConfiguredTargetKey.of(
+ rule.getLabel(),
+ getArtifactOwnerConfiguration(analysisEnvironment.getSkyframeEnv(), configuration));
if (analysisEnvironment.getSkyframeEnv().valuesMissing()) {
return null;
}
@@ -271,11 +273,13 @@
}
} else if (target instanceof InputFile) {
InputFile inputFile = (InputFile) target;
- Artifact artifact = artifactFactory.getSourceArtifact(
- inputFile.getExecPath(),
- Root.asSourceRoot(inputFile.getPackage().getSourceRoot(),
- inputFile.getPackage().getPackageIdentifier().getRepository().isMain()),
- new ConfiguredTargetKey(target.getLabel(), config));
+ Artifact artifact =
+ artifactFactory.getSourceArtifact(
+ inputFile.getExecPath(),
+ Root.asSourceRoot(
+ inputFile.getPackage().getSourceRoot(),
+ inputFile.getPackage().getPackageIdentifier().getRepository().isMain()),
+ ConfiguredTargetKey.of(target.getLabel(), config));
return new InputFileConfiguredTarget(targetContext, inputFile, artifact);
} else if (target instanceof PackageGroup) {
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/TargetAndConfiguration.java b/src/main/java/com/google/devtools/build/lib/analysis/TargetAndConfiguration.java
index e64098d..21ed49e 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/TargetAndConfiguration.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/TargetAndConfiguration.java
@@ -13,13 +13,11 @@
// limitations under the License.
package com.google.devtools.build.lib.analysis;
-import com.google.common.base.Function;
import com.google.common.base.Preconditions;
import com.google.devtools.build.lib.analysis.config.BuildConfiguration;
import com.google.devtools.build.lib.cmdline.Label;
import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable;
import com.google.devtools.build.lib.packages.Target;
-import com.google.devtools.build.lib.skyframe.ConfiguredTargetKey;
import java.util.Objects;
import javax.annotation.Nullable;
@@ -49,22 +47,6 @@
+ (configuration == null ? "null" : configuration.checksum());
}
- public static final Function<TargetAndConfiguration, String> NAME_FUNCTION =
- new Function<TargetAndConfiguration, String>() {
- @Override
- public String apply(TargetAndConfiguration node) {
- return node.getName();
- }
- };
-
- public static final Function<TargetAndConfiguration, ConfiguredTargetKey>
- TO_LABEL_AND_CONFIGURATION = new Function<TargetAndConfiguration, ConfiguredTargetKey>() {
- @Override
- public ConfiguredTargetKey apply(TargetAndConfiguration input) {
- return new ConfiguredTargetKey(input.getLabel(), input.getConfiguration());
- }
- };
-
@Override
public boolean equals(Object that) {
if (this == that) {
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileAction.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileAction.java
index 4f7bce0..549341a 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileAction.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileAction.java
@@ -474,7 +474,7 @@
}
Map<Artifact, SkyKey> skyKeys = new HashMap<>();
for (Artifact artifact : this.usedModules) {
- skyKeys.put(artifact, ActionLookupValue.key((ActionLookupKey) artifact.getArtifactOwner()));
+ skyKeys.put(artifact, (ActionLookupKey) artifact.getArtifactOwner());
}
Map<SkyKey, SkyValue> skyValues = env.getValues(skyKeys.values());
Set<Artifact> additionalModules = Sets.newLinkedHashSet();
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ActionTemplateExpansionValue.java b/src/main/java/com/google/devtools/build/lib/skyframe/ActionTemplateExpansionValue.java
index 7771e08..0897116 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/ActionTemplateExpansionValue.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/ActionTemplateExpansionValue.java
@@ -15,20 +15,19 @@
import com.google.common.base.Preconditions;
import com.google.common.collect.ImmutableList;
+import com.google.common.collect.Interner;
import com.google.devtools.build.lib.actions.Action;
import com.google.devtools.build.lib.actions.ActionKeyContext;
import com.google.devtools.build.lib.actions.ActionLookupValue;
import com.google.devtools.build.lib.analysis.actions.ActionTemplate;
import com.google.devtools.build.lib.cmdline.Label;
-import com.google.devtools.build.skyframe.LegacySkyKey;
+import com.google.devtools.build.lib.concurrent.BlazeInterners;
import com.google.devtools.build.skyframe.SkyFunctionName;
-import com.google.devtools.build.skyframe.SkyKey;
/**
* Value that stores expanded actions from ActionTemplate.
*/
public final class ActionTemplateExpansionValue extends ActionLookupValue {
-
ActionTemplateExpansionValue(
ActionKeyContext actionKeyContext,
Iterable<Action> expandedActions,
@@ -36,21 +35,17 @@
super(actionKeyContext, ImmutableList.copyOf(expandedActions), removeActionsAfterEvaluation);
}
- static SkyKey key(ActionTemplate<?> actionTemplate) {
- return LegacySkyKey.create(
- SkyFunctions.ACTION_TEMPLATE_EXPANSION, createActionTemplateExpansionKey(actionTemplate));
- }
+ private static final Interner<ActionTemplateExpansionKey> interner =
+ BlazeInterners.newWeakInterner();
- static ActionTemplateExpansionKey createActionTemplateExpansionKey(
- ActionTemplate<?> actionTemplate) {
- return new ActionTemplateExpansionKey(actionTemplate);
+ static ActionTemplateExpansionKey key(ActionTemplate<?> actionTemplate) {
+ return interner.intern(new ActionTemplateExpansionKey(actionTemplate));
}
-
static final class ActionTemplateExpansionKey extends ActionLookupKey {
private final ActionTemplate<?> actionTemplate;
- ActionTemplateExpansionKey(ActionTemplate<?> actionTemplate) {
+ private ActionTemplateExpansionKey(ActionTemplate<?> actionTemplate) {
Preconditions.checkNotNull(
actionTemplate,
"Passed in action template cannot be null: %s",
@@ -59,11 +54,10 @@
}
@Override
- protected SkyFunctionName getType() {
+ public SkyFunctionName functionName() {
return SkyFunctions.ACTION_TEMPLATE_EXPANSION;
}
-
@Override
public Label getLabel() {
return actionTemplate.getOwner().getLabel();
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ArtifactFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/ArtifactFunction.java
index bb0ddba..10016a1 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/ArtifactFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/ArtifactFunction.java
@@ -315,7 +315,7 @@
ArtifactOwner artifactOwner = artifact.getArtifactOwner();
Preconditions.checkState(artifactOwner instanceof ActionLookupKey, "", artifact, artifactOwner);
- return ActionLookupValue.key((ActionLookupKey) artifactOwner);
+ return (ActionLookupKey) artifactOwner;
}
@Nullable
@@ -326,7 +326,7 @@
if (value == null) {
ArtifactOwner artifactOwner = artifact.getArtifactOwner();
Preconditions.checkState(
- artifactOwner == CoverageReportValue.ARTIFACT_OWNER,
+ artifactOwner == CoverageReportValue.COVERAGE_REPORT_KEY,
"Not-yet-present artifact owner: %s (%s %s)",
artifactOwner,
artifact,
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 f3d47e3..56599c1 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
@@ -20,7 +20,6 @@
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.ActionLookupValue;
import com.google.devtools.build.lib.analysis.AliasProvider;
import com.google.devtools.build.lib.analysis.AspectResolver;
import com.google.devtools.build.lib.analysis.CachingAnalysisEnvironment;
@@ -408,8 +407,7 @@
throws DuplicateException {
ArrayList<ConfiguredAspect> aspectValues = new ArrayList<>();
for (AspectKey aspectKey : aspectKeys) {
- SkyKey skyAspectKey = aspectKey.getSkyKey();
- AspectValue aspectValue = (AspectValue) values.get(skyAspectKey);
+ AspectValue aspectValue = (AspectValue) values.get(aspectKey);
ConfiguredAspect configuredAspect = aspectValue.getConfiguredAspect();
aspectValues.add(configuredAspect);
}
@@ -438,14 +436,13 @@
return;
}
ImmutableList<AspectKey> baseKeys = key.getBaseKeys();
- SkyKey skyKey = key.getSkyKey();
- result.put(key.getAspectDescriptor(), skyKey);
+ result.put(key.getAspectDescriptor(), key);
for (AspectKey baseKey : baseKeys) {
buildSkyKeys(baseKey, result, aspectPathBuilder);
}
// Post-order list of aspect SkyKeys gives the order of propagating aspects:
// the aspect comes after all aspects it transitively sees.
- aspectPathBuilder.add(skyKey);
+ aspectPathBuilder.add(key);
}
private SkyValue createAliasAspect(
@@ -462,7 +459,7 @@
// the real configured target.
Label aliasLabel = aliasChain.size() > 1 ? aliasChain.get(1) : configuredTarget.getLabel();
- SkyKey depKey = ActionLookupValue.key(originalKey.withLabel(aliasLabel));
+ SkyKey depKey = originalKey.withLabel(aliasLabel);
// Compute the AspectValue of the target the alias refers to (which can itself be either an
// alias or a real target)
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/AspectValue.java b/src/main/java/com/google/devtools/build/lib/skyframe/AspectValue.java
index a6ecc98..b6c6e0d 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/AspectValue.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/AspectValue.java
@@ -17,6 +17,7 @@
import com.google.common.base.Objects;
import com.google.common.base.Preconditions;
import com.google.common.collect.ImmutableList;
+import com.google.common.collect.Interner;
import com.google.devtools.build.lib.actions.ActionAnalysisMetadata;
import com.google.devtools.build.lib.actions.ActionKeyContext;
import com.google.devtools.build.lib.actions.ActionLookupValue;
@@ -24,6 +25,7 @@
import com.google.devtools.build.lib.analysis.config.BuildConfiguration;
import com.google.devtools.build.lib.cmdline.Label;
import com.google.devtools.build.lib.collect.nestedset.NestedSet;
+import com.google.devtools.build.lib.concurrent.BlazeInterners;
import com.google.devtools.build.lib.events.Location;
import com.google.devtools.build.lib.packages.Aspect;
import com.google.devtools.build.lib.packages.AspectClass;
@@ -56,6 +58,7 @@
private final BuildConfiguration aspectConfiguration;
private final BuildConfiguration baseConfiguration;
private final AspectDescriptor aspectDescriptor;
+ private int hashCode;
private AspectKey(
Label label,
@@ -71,7 +74,7 @@
}
@Override
- protected SkyFunctionName getType() {
+ public SkyFunctionName functionName() {
return SkyFunctions.ASPECT;
}
@@ -148,6 +151,31 @@
@Override
public int hashCode() {
+ // We use the hash code caching strategy employed by java.lang.String. There are three subtle
+ // things going on here:
+ //
+ // (1) We use a value of 0 to indicate that the hash code hasn't been computed and cached yet.
+ // Yes, this means that if the hash code is really 0 then we will "recompute" it each time.
+ // But this isn't a problem in practice since a hash code of 0 should be rare.
+ //
+ // (2) Since we have no synchronization, multiple threads can race here thinking there are the
+ // first one to compute and cache the hash code.
+ //
+ // (3) Moreover, since 'hashCode' is non-volatile, the cached hash code value written from one
+ // thread may not be visible by another.
+ //
+ // All three of these issues are benign from a correctness perspective; in the end we have no
+ // overhead from synchronization, at the cost of potentially computing the hash code more than
+ // once.
+ int h = hashCode;
+ if (h == 0) {
+ h = computeHashCode();
+ hashCode = h;
+ }
+ return h;
+ }
+
+ private int computeHashCode() {
return Objects.hashCode(
label,
baseKeys,
@@ -227,6 +255,7 @@
private final BuildConfiguration targetConfiguration;
private final SkylarkImport skylarkImport;
private final String skylarkValueName;
+ private int hashCode;
private SkylarkAspectLoadingKey(
Label targetLabel,
@@ -243,7 +272,7 @@
}
@Override
- protected SkyFunctionName getType() {
+ public SkyFunctionName functionName() {
return SkyFunctions.LOAD_SKYLARK_ASPECT;
}
@@ -282,6 +311,31 @@
@Override
public int hashCode() {
+ // We use the hash code caching strategy employed by java.lang.String. There are three subtle
+ // things going on here:
+ //
+ // (1) We use a value of 0 to indicate that the hash code hasn't been computed and cached yet.
+ // Yes, this means that if the hash code is really 0 then we will "recompute" it each time.
+ // But this isn't a problem in practice since a hash code of 0 should be rare.
+ //
+ // (2) Since we have no synchronization, multiple threads can race here thinking there are the
+ // first one to compute and cache the hash code.
+ //
+ // (3) Moreover, since 'hashCode' is non-volatile, the cached hash code value written from one
+ // thread may not be visible by another.
+ //
+ // All three of these issues are benign from a correctness perspective; in the end we have no
+ // overhead from synchronization, at the cost of potentially computing the hash code more than
+ // once.
+ int h = hashCode;
+ if (h == 0) {
+ h = computeHashCode();
+ hashCode = h;
+ }
+ return h;
+ }
+
+ private int computeHashCode() {
return Objects.hashCode(targetLabel,
aspectConfiguration,
targetConfiguration,
@@ -392,24 +446,27 @@
ImmutableList<AspectKey> baseKeys,
AspectDescriptor aspectDescriptor,
BuildConfiguration aspectConfiguration) {
- return new AspectKey(
+ return aspectKeyInterner.intern(new AspectKey(
label, baseConfiguration,
baseKeys,
aspectDescriptor,
aspectConfiguration
- );
+ ));
}
+ private static final Interner<AspectKey> aspectKeyInterner = BlazeInterners.newWeakInterner();
public static AspectKey createAspectKey(
Label label,
BuildConfiguration baseConfiguration, AspectDescriptor aspectDescriptor,
BuildConfiguration aspectConfiguration) {
- return new AspectKey(
- label, baseConfiguration, ImmutableList.<AspectKey>of(),
- aspectDescriptor, aspectConfiguration);
+ return aspectKeyInterner.intern(
+ new AspectKey(
+ label, baseConfiguration, ImmutableList.of(), aspectDescriptor, aspectConfiguration));
}
+ private static final Interner<SkylarkAspectLoadingKey> skylarkAspectKeyInterner =
+ BlazeInterners.newWeakInterner();
public static SkylarkAspectLoadingKey createSkylarkAspectKey(
Label targetLabel,
@@ -417,7 +474,12 @@
BuildConfiguration targetConfiguration,
SkylarkImport skylarkImport,
String skylarkExportName) {
- return new SkylarkAspectLoadingKey(
- targetLabel, aspectConfiguration, targetConfiguration, skylarkImport, skylarkExportName);
+ return skylarkAspectKeyInterner.intern(
+ new SkylarkAspectLoadingKey(
+ targetLabel,
+ aspectConfiguration,
+ targetConfiguration,
+ skylarkImport,
+ skylarkExportName));
}
}
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 62878a5..c80875d 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
@@ -58,7 +58,7 @@
public SkyValue compute(SkyKey skyKey, Environment env) throws InterruptedException {
final BuildInfoKeyAndConfig keyAndConfig = (BuildInfoKeyAndConfig) skyKey.argument();
WorkspaceStatusValue infoArtifactValue =
- (WorkspaceStatusValue) env.getValue(WorkspaceStatusValue.SKY_KEY);
+ (WorkspaceStatusValue) env.getValue(WorkspaceStatusValue.BUILD_INFO_KEY);
if (infoArtifactValue == null) {
return null;
}
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 6486939..8695042 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
@@ -14,12 +14,14 @@
package com.google.devtools.build.lib.skyframe;
import com.google.common.base.Preconditions;
+import com.google.common.collect.Interner;
import com.google.devtools.build.lib.actions.ActionKeyContext;
import com.google.devtools.build.lib.actions.ActionLookupValue;
import com.google.devtools.build.lib.analysis.buildinfo.BuildInfoCollection;
import com.google.devtools.build.lib.analysis.buildinfo.BuildInfoFactory;
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 java.util.Objects;
@@ -47,18 +49,26 @@
return getStringHelper().add("collection", collection).toString();
}
+ private static final Interner<BuildInfoKeyAndConfig> keyInterner =
+ BlazeInterners.newWeakInterner();
+
+ public static BuildInfoKeyAndConfig key(
+ BuildInfoFactory.BuildInfoKey key, BuildConfiguration config) {
+ return keyInterner.intern(new BuildInfoKeyAndConfig(key, config));
+ }
+
/** Key for BuildInfoCollectionValues. */
public static class BuildInfoKeyAndConfig extends ActionLookupKey {
private final BuildInfoFactory.BuildInfoKey infoKey;
private final BuildConfiguration config;
- public BuildInfoKeyAndConfig(BuildInfoFactory.BuildInfoKey key, BuildConfiguration config) {
+ private BuildInfoKeyAndConfig(BuildInfoFactory.BuildInfoKey key, BuildConfiguration config) {
this.infoKey = Preconditions.checkNotNull(key, config);
this.config = Preconditions.checkNotNull(config, key);
}
@Override
- protected SkyFunctionName getType() {
+ public SkyFunctionName functionName() {
return SkyFunctions.BUILD_INFO_COLLECTION;
}
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/CompletionFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/CompletionFunction.java
index 02bb53f..db8d44c 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/CompletionFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/CompletionFunction.java
@@ -103,8 +103,7 @@
public ConfiguredTargetValue getValueFromSkyKey(SkyKey skyKey, Environment env)
throws InterruptedException {
TargetCompletionKey tcKey = (TargetCompletionKey) skyKey.argument();
- ConfiguredTargetKey lac = tcKey.configuredTargetKey();
- return (ConfiguredTargetValue) env.getValue(lac.getSkyKey());
+ return (ConfiguredTargetValue) env.getValue(tcKey.configuredTargetKey());
}
@Override
@@ -179,7 +178,7 @@
throws InterruptedException {
AspectCompletionKey acKey = (AspectCompletionKey) skyKey.argument();
AspectKey aspectKey = acKey.aspectKey();
- return (AspectValue) env.getValue(aspectKey.getSkyKey());
+ return (AspectValue) env.getValue(aspectKey);
}
@Override
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 af83e3c..258223b 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
@@ -604,9 +604,13 @@
if (env.valuesMissing()) {
return null;
}
- CachingAnalysisEnvironment analysisEnvironment = view.createAnalysisEnvironment(
- new ConfiguredTargetKey(target.getLabel(), ownerConfig), false,
- events, env, configuration);
+ CachingAnalysisEnvironment analysisEnvironment =
+ view.createAnalysisEnvironment(
+ ConfiguredTargetKey.of(target.getLabel(), ownerConfig),
+ false,
+ events,
+ env,
+ configuration);
if (env.valuesMissing()) {
return null;
}
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 c02fd99..6cdd959 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
@@ -14,13 +14,14 @@
package com.google.devtools.build.lib.skyframe;
-import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Preconditions;
+import com.google.common.collect.Interner;
import com.google.devtools.build.lib.actions.ActionLookupValue.ActionLookupKey;
import com.google.devtools.build.lib.analysis.AliasProvider;
import com.google.devtools.build.lib.analysis.ConfiguredTarget;
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 java.util.Objects;
import javax.annotation.Nullable;
@@ -36,16 +37,13 @@
@Nullable
private final BuildConfiguration configuration;
- public ConfiguredTargetKey(Label label, @Nullable BuildConfiguration configuration) {
+ private transient int hashCode;
+
+ private ConfiguredTargetKey(Label label, @Nullable BuildConfiguration configuration) {
this.label = Preconditions.checkNotNull(label);
this.configuration = configuration;
}
- @VisibleForTesting
- public ConfiguredTargetKey(ConfiguredTarget rule) {
- this(rule.getTarget().getLabel(), rule.getConfiguration());
- }
-
public static ConfiguredTargetKey of(ConfiguredTarget configuredTarget) {
AliasProvider aliasProvider = configuredTarget.getProvider(AliasProvider.class);
Label label =
@@ -53,8 +51,14 @@
return of(label, configuredTarget.getConfiguration());
}
+ /**
+ * Cache 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();
+
public static ConfiguredTargetKey of(Label label, @Nullable BuildConfiguration configuration) {
- return new ConfiguredTargetKey(label, configuration);
+ return interner.intern(new ConfiguredTargetKey(label, configuration));
}
@Override
@@ -63,7 +67,7 @@
}
@Override
- protected SkyFunctionName getType() {
+ public SkyFunctionName functionName() {
return SkyFunctions.CONFIGURED_TARGET;
}
@@ -74,6 +78,31 @@
@Override
public int hashCode() {
+ // We use the hash code caching strategy employed by java.lang.String. There are three subtle
+ // things going on here:
+ //
+ // (1) We use a value of 0 to indicate that the hash code hasn't been computed and cached yet.
+ // Yes, this means that if the hash code is really 0 then we will "recompute" it each time. But
+ // this isn't a problem in practice since a hash code of 0 should be rare.
+ //
+ // (2) Since we have no synchronization, multiple threads can race here thinking there are the
+ // first one to compute and cache the hash code.
+ //
+ // (3) Moreover, since 'hashCode' is non-volatile, the cached hash code value written from one
+ // thread may not be visible by another.
+ //
+ // All three of these issues are benign from a correctness perspective; in the end we have no
+ // overhead from synchronization, at the cost of potentially computing the hash code more than
+ // once.
+ int h = hashCode;
+ if (h == 0) {
+ h = computeHashCode();
+ hashCode = h;
+ }
+ return h;
+ }
+
+ private int computeHashCode() {
int configVal = configuration == null ? 79 : configuration.hashCode();
return 31 * label.hashCode() + configVal;
}
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ConfiguredTargetValue.java b/src/main/java/com/google/devtools/build/lib/skyframe/ConfiguredTargetValue.java
index a11e27a..fc4ebd6 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/ConfiguredTargetValue.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/ConfiguredTargetValue.java
@@ -100,13 +100,13 @@
@VisibleForTesting
public static SkyKey key(Label label, BuildConfiguration configuration) {
- return key(new ConfiguredTargetKey(label, configuration));
+ return ConfiguredTargetKey.of(label, configuration);
}
static ImmutableList<SkyKey> keys(Iterable<ConfiguredTargetKey> lacs) {
ImmutableList.Builder<SkyKey> keys = ImmutableList.builder();
for (ConfiguredTargetKey lac : lacs) {
- keys.add(key(lac));
+ keys.add(lac);
}
return keys.build();
}
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/CoverageReportFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/CoverageReportFunction.java
index 64fdcc4..3fbf2d9 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/CoverageReportFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/CoverageReportFunction.java
@@ -41,8 +41,10 @@
@Override
public SkyValue compute(SkyKey skyKey, Environment env) throws InterruptedException {
Preconditions.checkState(
- CoverageReportValue.SKY_KEY.equals(skyKey), String.format(
- "Expected %s for SkyKey but got %s instead", CoverageReportValue.SKY_KEY, skyKey));
+ CoverageReportValue.COVERAGE_REPORT_KEY.equals(skyKey),
+ String.format(
+ "Expected %s for SkyKey but got %s instead",
+ CoverageReportValue.COVERAGE_REPORT_KEY, skyKey));
ImmutableList<ActionAnalysisMetadata> actions = PrecomputedValue.COVERAGE_REPORT_KEY.get(env);
if (actions == null) {
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/CoverageReportValue.java b/src/main/java/com/google/devtools/build/lib/skyframe/CoverageReportValue.java
index fde0599..fd9bace 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/CoverageReportValue.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/CoverageReportValue.java
@@ -18,10 +18,7 @@
import com.google.devtools.build.lib.actions.ActionAnalysisMetadata;
import com.google.devtools.build.lib.actions.ActionKeyContext;
import com.google.devtools.build.lib.actions.ActionLookupValue;
-import com.google.devtools.build.lib.actions.ArtifactOwner;
-import com.google.devtools.build.skyframe.LegacySkyKey;
import com.google.devtools.build.skyframe.SkyFunctionName;
-import com.google.devtools.build.skyframe.SkyKey;
/**
* A SkyValue to store the coverage report Action and Artifacts.
@@ -29,8 +26,7 @@
public class CoverageReportValue extends ActionLookupValue {
// There should only ever be one CoverageReportValue value in the graph.
- public static final ArtifactOwner ARTIFACT_OWNER = new CoverageReportKey();
- static final SkyKey SKY_KEY = LegacySkyKey.create(SkyFunctions.COVERAGE_REPORT, ARTIFACT_OWNER);
+ public static final CoverageReportKey COVERAGE_REPORT_KEY = new CoverageReportKey();
CoverageReportValue(
ActionKeyContext actionKeyContext,
@@ -39,15 +35,13 @@
super(actionKeyContext, coverageReportActions, removeActionsAfterEvaluation);
}
- private static class CoverageReportKey extends ActionLookupKey {
- @Override
- protected SkyFunctionName getType() {
- throw new UnsupportedOperationException();
- }
+ static class CoverageReportKey extends ActionLookupKey {
+ private CoverageReportKey() {}
@Override
- protected SkyKey getSkyKeyInternal() {
- return SKY_KEY;
+ public SkyFunctionName functionName() {
+ return SkyFunctions.COVERAGE_REPORT;
}
+
}
}
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/PostConfiguredTargetFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/PostConfiguredTargetFunction.java
index 779dbaa..3ae57a5 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/PostConfiguredTargetFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/PostConfiguredTargetFunction.java
@@ -51,12 +51,12 @@
public class PostConfiguredTargetFunction implements SkyFunction {
private static final Function<Dependency, SkyKey> TO_KEYS =
new Function<Dependency, SkyKey>() {
- @Override
- public SkyKey apply(Dependency input) {
- return PostConfiguredTargetValue.key(
- new ConfiguredTargetKey(input.getLabel(), input.getConfiguration()));
- }
- };
+ @Override
+ public SkyKey apply(Dependency input) {
+ return PostConfiguredTargetValue.key(
+ ConfiguredTargetKey.of(input.getLabel(), input.getConfiguration()));
+ }
+ };
private final SkyframeExecutor.BuildViewProvider buildViewProvider;
private final RuleClassProvider ruleClassProvider;
@@ -74,8 +74,8 @@
throws SkyFunctionException, InterruptedException {
ImmutableMap<ActionAnalysisMetadata, ConflictException> badActions =
PrecomputedValue.BAD_ACTIONS.get(env);
- ConfiguredTargetValue ctValue = (ConfiguredTargetValue)
- env.getValue(ConfiguredTargetValue.key((ConfiguredTargetKey) skyKey.argument()));
+ ConfiguredTargetValue ctValue =
+ (ConfiguredTargetValue) env.getValue((ConfiguredTargetKey) skyKey.argument());
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 d8f7b1b..ad531b3 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
@@ -106,7 +106,7 @@
label ->
LegacySkyKey.create(
SkyFunctions.CONFIGURED_TARGET,
- new ConfiguredTargetKey(label, configuration)))
+ ConfiguredTargetKey.of(label, configuration)))
.collect(ImmutableList.toImmutableList());
Map<SkyKey, ValueOrException<ConfiguredValueCreationException>> values =
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SequencedSkyframeExecutor.java b/src/main/java/com/google/devtools/build/lib/skyframe/SequencedSkyframeExecutor.java
index 8adef6d..0313d47 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/SequencedSkyframeExecutor.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/SequencedSkyframeExecutor.java
@@ -743,7 +743,6 @@
// well as ActionExecutionValues, since they do not depend on ActionLookupValues.
SkyFunctionName.functionIsIn(ImmutableSet.of(
SkyFunctions.CONFIGURED_TARGET,
- SkyFunctions.ACTION_LOOKUP,
SkyFunctions.BUILD_INFO,
SkyFunctions.TARGET_COMPLETION,
SkyFunctions.BUILD_INFO_COLLECTION,
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkyFunctions.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkyFunctions.java
index 4f7479b..c701fa6 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/SkyFunctions.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkyFunctions.java
@@ -86,7 +86,6 @@
public static final SkyFunctionName ARTIFACT = SkyFunctionName.create("ARTIFACT");
public static final SkyFunctionName ACTION_EXECUTION =
SkyFunctionName.create("ACTION_EXECUTION");
- public static final SkyFunctionName ACTION_LOOKUP = SkyFunctionName.create("ACTION_LOOKUP");
public static final SkyFunctionName RECURSIVE_FILESYSTEM_TRAVERSAL =
SkyFunctionName.create("RECURSIVE_DIRECTORY_TRAVERSAL");
public static final SkyFunctionName FILESET_ENTRY = SkyFunctionName.create("FILESET_ENTRY");
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 a62d501..a137435 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
@@ -60,7 +60,6 @@
import com.google.devtools.build.lib.pkgcache.LoadingPhaseRunner;
import com.google.devtools.build.lib.skyframe.AspectFunction.AspectCreationException;
import com.google.devtools.build.lib.skyframe.AspectValue.AspectValueKey;
-import com.google.devtools.build.lib.skyframe.BuildInfoCollectionValue.BuildInfoKeyAndConfig;
import com.google.devtools.build.lib.skyframe.ConfiguredTargetFunction.ConfiguredValueCreationException;
import com.google.devtools.build.lib.skyframe.SkyframeActionExecutor.ConflictException;
import com.google.devtools.build.lib.skyframe.SkylarkImportLookupFunction.SkylarkImportFailedException;
@@ -221,7 +220,7 @@
NestedSetBuilder<Package> packages =
singleSourceRoot == null ? NestedSetBuilder.stableOrder() : null;
for (AspectValueKey aspectKey : aspectKeys) {
- AspectValue value = (AspectValue) result.get(aspectKey.getSkyKey());
+ AspectValue value = (AspectValue) result.get(aspectKey);
if (value == null) {
// Skip aspects that couldn't be applied to targets.
continue;
@@ -237,8 +236,7 @@
// list of values, i.e., that the order is deterministic.
Collection<ConfiguredTarget> goodCts = Lists.newArrayListWithCapacity(values.size());
for (ConfiguredTargetKey value : values) {
- ConfiguredTargetValue ctValue =
- (ConfiguredTargetValue) result.get(ConfiguredTargetValue.key(value));
+ ConfiguredTargetValue ctValue = (ConfiguredTargetValue) result.get(value);
if (ctValue == null) {
continue;
}
@@ -450,13 +448,13 @@
BuildConfiguration config,
ImmutableMap<BuildInfoKey, BuildInfoFactory> buildInfoFactories)
throws InterruptedException {
- env.getValue(WorkspaceStatusValue.SKY_KEY);
+ env.getValue(WorkspaceStatusValue.BUILD_INFO_KEY);
// These factories may each create their own build info artifacts, all depending on the basic
// build-info.txt and build-changelist.txt.
List<SkyKey> depKeys = Lists.newArrayList();
for (BuildInfoKey key : buildInfoFactories.keySet()) {
if (buildInfoFactories.get(key).isEnabled(config)) {
- depKeys.add(BuildInfoCollectionValue.key(new BuildInfoKeyAndConfig(key, config)));
+ depKeys.add(BuildInfoCollectionValue.key(key, config));
}
}
env.getValues(depKeys);
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 a59f865..429caf8 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
@@ -752,20 +752,21 @@
// TODO(janakr): don't invalidate here, just use different keys for different configs. Can't
// be done right now because of lack of configuration trimming and fact that everything
// depends on workspace status action.
- invalidate(WorkspaceStatusValue.SKY_KEY::equals);
+ invalidate(WorkspaceStatusValue.BUILD_INFO_KEY::equals);
}
}
private WorkspaceStatusAction makeWorkspaceStatusAction(String workspaceName) {
return workspaceStatusActionFactory.createWorkspaceStatusAction(
- artifactFactory.get(), WorkspaceStatusValue.ARTIFACT_OWNER, buildId, workspaceName);
+ artifactFactory.get(), WorkspaceStatusValue.BUILD_INFO_KEY, buildId, workspaceName);
}
@VisibleForTesting
@Nullable
public WorkspaceStatusAction getLastWorkspaceStatusAction() throws InterruptedException {
WorkspaceStatusValue workspaceStatusValue =
- (WorkspaceStatusValue) memoizingEvaluator.getExistingValue(WorkspaceStatusValue.SKY_KEY);
+ (WorkspaceStatusValue)
+ memoizingEvaluator.getExistingValue(WorkspaceStatusValue.BUILD_INFO_KEY);
return workspaceStatusValue == null
? null
: (WorkspaceStatusAction) workspaceStatusValue.getAction(0);
@@ -818,11 +819,14 @@
public Collection<Artifact> getWorkspaceStatusArtifacts(ExtendedEventHandler eventHandler)
throws InterruptedException {
// Should already be present, unless the user didn't request any targets for analysis.
- EvaluationResult<WorkspaceStatusValue> result = buildDriver.evaluate(
- ImmutableList.of(WorkspaceStatusValue.SKY_KEY), /*keepGoing=*/true, /*numThreads=*/1,
- eventHandler);
+ EvaluationResult<WorkspaceStatusValue> result =
+ buildDriver.evaluate(
+ ImmutableList.of(WorkspaceStatusValue.BUILD_INFO_KEY),
+ /*keepGoing=*/ true,
+ /*numThreads=*/ 1,
+ eventHandler);
WorkspaceStatusValue value =
- Preconditions.checkNotNull(result.get(WorkspaceStatusValue.SKY_KEY));
+ Preconditions.checkNotNull(result.get(WorkspaceStatusValue.BUILD_INFO_KEY));
return ImmutableList.of(value.getStableArtifact(), value.getVolatileArtifact());
}
@@ -1313,8 +1317,8 @@
for (BuildConfiguration depConfig : configs.get(key)) {
skyKeys.add(ConfiguredTargetValue.key(key.getLabel(), depConfig));
for (AspectDescriptor aspectDescriptor : key.getAspects().getAllAspects()) {
- skyKeys.add(ActionLookupValue.key(AspectValue.createAspectKey(key.getLabel(), depConfig,
- aspectDescriptor, depConfig)));
+ skyKeys.add(
+ AspectValue.createAspectKey(key.getLabel(), depConfig, aspectDescriptor, depConfig));
}
}
}
@@ -1346,8 +1350,8 @@
List<ConfiguredAspect> configuredAspects = new ArrayList<>();
for (AspectDescriptor aspectDescriptor : key.getAspects().getAllAspects()) {
- SkyKey aspectKey = ActionLookupValue.key(AspectValue.createAspectKey(key.getLabel(),
- depConfig, aspectDescriptor, depConfig));
+ SkyKey aspectKey =
+ AspectValue.createAspectKey(key.getLabel(), depConfig, aspectDescriptor, depConfig);
if (result.get(aspectKey) == null) {
continue DependentNodeLoop;
}
@@ -1642,7 +1646,7 @@
List<SkyKey> keys = new ArrayList<>(ConfiguredTargetValue.keys(values));
for (AspectValueKey aspectKey : aspectKeys) {
- keys.add(aspectKey.getSkyKey());
+ keys.add(aspectKey);
}
EvaluationResult<ActionLookupValue> result =
buildDriver.evaluate(keys, keepGoing, numThreads, eventHandler);
@@ -1746,8 +1750,7 @@
ArtifactOwner artifactOwner = artifact.getArtifactOwner();
Preconditions.checkState(artifactOwner instanceof ActionLookupValue.ActionLookupKey,
"%s %s", artifact, artifactOwner);
- SkyKey actionLookupKey =
- ActionLookupValue.key((ActionLookupValue.ActionLookupKey) artifactOwner);
+ SkyKey actionLookupKey = (ActionLookupValue.ActionLookupKey) artifactOwner;
synchronized (valueLookupLock) {
// Note that this will crash (attempting to run a configured target value builder after
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/TestCompletionFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/TestCompletionFunction.java
index 32e5ec5..4a9946a 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/TestCompletionFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/TestCompletionFunction.java
@@ -32,13 +32,13 @@
public SkyValue compute(SkyKey skyKey, Environment env) throws InterruptedException {
TestCompletionValue.TestCompletionKey key =
(TestCompletionValue.TestCompletionKey) skyKey.argument();
- ConfiguredTargetKey lac = key.configuredTargetKey();
+ ConfiguredTargetKey ctKey = key.configuredTargetKey();
TopLevelArtifactContext ctx = key.topLevelArtifactContext();
- if (env.getValue(TargetCompletionValue.key(lac, ctx, /*willTest=*/true)) == null) {
+ if (env.getValue(TargetCompletionValue.key(ctKey, ctx, /*willTest=*/ true)) == null) {
return null;
}
- ConfiguredTargetValue ctValue = (ConfiguredTargetValue) env.getValue(lac.getSkyKey());
+ ConfiguredTargetValue ctValue = (ConfiguredTargetValue) env.getValue(ctKey);
if (ctValue == null) {
return null;
}
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 a47e373..11e44fc 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
@@ -144,11 +144,11 @@
SkyKey executionPlatformKey =
LegacySkyKey.create(
SkyFunctions.CONFIGURED_TARGET,
- new ConfiguredTargetKey(executionPlatformLabel, configuration));
+ ConfiguredTargetKey.of(executionPlatformLabel, configuration));
SkyKey targetPlatformKey =
LegacySkyKey.create(
SkyFunctions.CONFIGURED_TARGET,
- new ConfiguredTargetKey(targetPlatformLabel, configuration));
+ ConfiguredTargetKey.of(targetPlatformLabel, configuration));
Map<SkyKey, ValueOrException<ConfiguredValueCreationException>> values =
env.getValuesOrThrow(
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ToplevelSkylarkAspectFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/ToplevelSkylarkAspectFunction.java
index e9ee29e..82de66d 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/ToplevelSkylarkAspectFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/ToplevelSkylarkAspectFunction.java
@@ -17,7 +17,6 @@
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.Iterables;
-import com.google.devtools.build.lib.actions.ActionLookupValue;
import com.google.devtools.build.lib.cmdline.Label;
import com.google.devtools.build.lib.events.Event;
import com.google.devtools.build.lib.packages.AspectDescriptor;
@@ -84,11 +83,11 @@
throw new LoadSkylarkAspectFunctionException(e);
}
SkyKey aspectKey =
- ActionLookupValue.key(AspectValue.createAspectKey(
- aspectLoadingKey.getTargetLabel(), aspectLoadingKey.getTargetConfiguration(),
+ AspectValue.createAspectKey(
+ aspectLoadingKey.getTargetLabel(),
+ aspectLoadingKey.getTargetConfiguration(),
new AspectDescriptor(skylarkAspect.getAspectClass(), AspectParameters.EMPTY),
- aspectLoadingKey.getAspectConfiguration()
- ));
+ aspectLoadingKey.getAspectConfiguration());
return env.getValue(aspectKey);
}
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/WorkspaceStatusFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/WorkspaceStatusFunction.java
index 817c2fa..b231bae 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/WorkspaceStatusFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/WorkspaceStatusFunction.java
@@ -43,7 +43,7 @@
@Override
public SkyValue compute(SkyKey skyKey, Environment env) throws InterruptedException {
Preconditions.checkState(
- WorkspaceStatusValue.SKY_KEY.equals(skyKey), WorkspaceStatusValue.SKY_KEY);
+ WorkspaceStatusValue.BUILD_INFO_KEY.equals(skyKey), WorkspaceStatusValue.BUILD_INFO_KEY);
WorkspaceNameValue workspaceNameValue =
(WorkspaceNameValue) env.getValue(WorkspaceNameValue.key());
if (env.valuesMissing()) {
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/WorkspaceStatusValue.java b/src/main/java/com/google/devtools/build/lib/skyframe/WorkspaceStatusValue.java
index 78ff03d..1b69592 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/WorkspaceStatusValue.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/WorkspaceStatusValue.java
@@ -16,11 +16,8 @@
import com.google.devtools.build.lib.actions.ActionKeyContext;
import com.google.devtools.build.lib.actions.ActionLookupValue;
import com.google.devtools.build.lib.actions.Artifact;
-import com.google.devtools.build.lib.actions.ArtifactOwner;
import com.google.devtools.build.lib.analysis.WorkspaceStatusAction;
-import com.google.devtools.build.skyframe.LegacySkyKey;
import com.google.devtools.build.skyframe.SkyFunctionName;
-import com.google.devtools.build.skyframe.SkyKey;
/**
* Value that stores the workspace status artifacts and their generating action. There should be
@@ -33,8 +30,7 @@
private final Artifact volatileArtifact;
// There should only ever be one BuildInfo value in the graph.
- static final ArtifactOwner ARTIFACT_OWNER = new BuildInfoKey();
- public static final SkyKey SKY_KEY = LegacySkyKey.create(SkyFunctions.BUILD_INFO, ARTIFACT_OWNER);
+ public static final BuildInfoKey BUILD_INFO_KEY = new BuildInfoKey();
WorkspaceStatusValue(
ActionKeyContext actionKeyContext,
@@ -55,15 +51,13 @@
return volatileArtifact;
}
- private static class BuildInfoKey extends ActionLookupKey {
- @Override
- protected SkyFunctionName getType() {
- throw new UnsupportedOperationException();
- }
+ static class BuildInfoKey extends ActionLookupKey {
+ private BuildInfoKey() {}
@Override
- protected SkyKey getSkyKeyInternal() {
- return SKY_KEY;
+ public SkyFunctionName functionName() {
+ return SkyFunctions.BUILD_INFO;
}
+
}
}
