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;
}
+
}
}
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/AspectValueTest.java b/src/test/java/com/google/devtools/build/lib/analysis/AspectValueTest.java
index 4b71f81..b0cf53b 100644
--- a/src/test/java/com/google/devtools/build/lib/analysis/AspectValueTest.java
+++ b/src/test/java/com/google/devtools/build/lib/analysis/AspectValueTest.java
@@ -15,7 +15,6 @@
import com.google.common.collect.ImmutableList;
import com.google.common.testing.EqualsTester;
-import com.google.devtools.build.lib.actions.ActionLookupValue;
import com.google.devtools.build.lib.analysis.config.BuildConfiguration;
import com.google.devtools.build.lib.analysis.util.AnalysisTestCase;
import com.google.devtools.build.lib.analysis.util.TestAspects;
@@ -220,10 +219,11 @@
private static SkyKey createKey(
Label label, BuildConfiguration baseConfiguration, NativeAspectClass aspectClass,
AspectParameters parameters, BuildConfiguration aspectConfiguration) {
- return ActionLookupValue.key(AspectValue.createAspectKey(
- label, baseConfiguration, new AspectDescriptor(aspectClass, parameters),
- aspectConfiguration
- ));
+ return AspectValue.createAspectKey(
+ label,
+ baseConfiguration,
+ new AspectDescriptor(aspectClass, parameters),
+ aspectConfiguration);
}
private static SkyKey createDerivedKey(
@@ -234,11 +234,12 @@
BuildConfiguration aspectConfiguration2) {
AspectKey baseKey = AspectValue.createAspectKey(label, baseConfiguration,
new AspectDescriptor(aspectClass1, parameters1), aspectConfiguration1);
- return ActionLookupValue.key(AspectValue.createAspectKey(
- label, baseConfiguration,
- ImmutableList.of(baseKey), new AspectDescriptor(aspectClass2, parameters2),
- aspectConfiguration2
- ));
+ return AspectValue.createAspectKey(
+ label,
+ baseConfiguration,
+ ImmutableList.of(baseKey),
+ new AspectDescriptor(aspectClass2, parameters2),
+ aspectConfiguration2);
}
}
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/util/AnalysisTestCase.java b/src/test/java/com/google/devtools/build/lib/analysis/util/AnalysisTestCase.java
index 60fc89a..fdddbee 100644
--- a/src/test/java/com/google/devtools/build/lib/analysis/util/AnalysisTestCase.java
+++ b/src/test/java/com/google/devtools/build/lib/analysis/util/AnalysisTestCase.java
@@ -437,10 +437,12 @@
protected Artifact getBinArtifact(String packageRelativePath, ConfiguredTarget owner)
throws InterruptedException {
Label label = owner.getLabel();
- return buildView.getArtifactFactory().getDerivedArtifact(
- label.getPackageFragment().getRelative(packageRelativePath),
- getTargetConfiguration().getBinDirectory(label.getPackageIdentifier().getRepository()),
- new ConfiguredTargetKey(owner));
+ return buildView
+ .getArtifactFactory()
+ .getDerivedArtifact(
+ label.getPackageFragment().getRelative(packageRelativePath),
+ getTargetConfiguration().getBinDirectory(label.getPackageIdentifier().getRepository()),
+ ConfiguredTargetKey.of(owner));
}
protected Set<SkyKey> getSkyframeEvaluatedTargetKeys() {
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewTestCase.java b/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewTestCase.java
index 10e012a..457d184 100644
--- a/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewTestCase.java
+++ b/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewTestCase.java
@@ -34,7 +34,6 @@
import com.google.devtools.build.lib.actions.ActionGraph;
import com.google.devtools.build.lib.actions.ActionInput;
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.actions.CommandLineExpansionException;
@@ -1022,11 +1021,10 @@
* be "foo.o".
*/
protected Artifact getBinArtifact(String packageRelativePath, String owner) {
- ConfiguredTargetKey config = makeConfiguredTargetKey(owner);
return getPackageRelativeDerivedArtifact(
packageRelativePath,
- config.getConfiguration().getBinDirectory(RepositoryName.MAIN),
- config);
+ getConfiguration(owner).getBinDirectory(RepositoryName.MAIN),
+ makeConfiguredTargetKey(owner));
}
/**
@@ -1036,9 +1034,10 @@
* be "foo.o".
*/
protected Artifact getBinArtifact(String packageRelativePath, ConfiguredTarget owner) {
- return getPackageRelativeDerivedArtifact(packageRelativePath,
+ return getPackageRelativeDerivedArtifact(
+ packageRelativePath,
owner.getConfiguration().getBinDirectory(RepositoryName.MAIN),
- new ConfiguredTargetKey(owner));
+ ConfiguredTargetKey.of(owner));
}
/**
@@ -1074,10 +1073,11 @@
packageRelativePath,
owner.getConfiguration().getBinDirectory(RepositoryName.MAIN),
(AspectValue.AspectKey)
- ActionLookupValue.key(AspectValue.createAspectKey(
- owner.getLabel(), owner.getConfiguration(),
- new AspectDescriptor(creatingAspectFactory, parameters), owner.getConfiguration()
- ))
+ AspectValue.createAspectKey(
+ owner.getLabel(),
+ owner.getConfiguration(),
+ new AspectDescriptor(creatingAspectFactory, parameters),
+ owner.getConfiguration())
.argument());
}
@@ -1101,8 +1101,9 @@
* just be "foo.o".
*/
protected Artifact getGenfilesArtifact(String packageRelativePath, String owner) {
- ConfiguredTargetKey configKey = makeConfiguredTargetKey(owner);
- return getGenfilesArtifact(packageRelativePath, configKey, configKey.getConfiguration());
+ BuildConfiguration config = getConfiguration(owner);
+ return getGenfilesArtifact(
+ packageRelativePath, ConfiguredTargetKey.of(makeLabel(owner), config), config);
}
/**
@@ -1112,8 +1113,8 @@
* just be "foo.o".
*/
protected Artifact getGenfilesArtifact(String packageRelativePath, ConfiguredTarget owner) {
- ConfiguredTargetKey configKey = new ConfiguredTargetKey(owner);
- return getGenfilesArtifact(packageRelativePath, configKey, configKey.getConfiguration());
+ ConfiguredTargetKey configKey = ConfiguredTargetKey.of(owner);
+ return getGenfilesArtifact(packageRelativePath, configKey, owner.getConfiguration());
}
/**
@@ -1138,13 +1139,16 @@
AspectParameters params) {
return getPackageRelativeDerivedArtifact(
packageRelativePath,
- owner.getConfiguration().getGenfilesDirectory(
- owner.getTarget().getLabel().getPackageIdentifier().getRepository()),
+ owner
+ .getConfiguration()
+ .getGenfilesDirectory(
+ owner.getTarget().getLabel().getPackageIdentifier().getRepository()),
(AspectValue.AspectKey)
- ActionLookupValue.key(AspectValue.createAspectKey(
- owner.getLabel(), owner.getConfiguration(),
- new AspectDescriptor(creatingAspectFactory, params), owner.getConfiguration()
- ))
+ AspectValue.createAspectKey(
+ owner.getLabel(),
+ owner.getConfiguration(),
+ new AspectDescriptor(creatingAspectFactory, params),
+ owner.getConfiguration())
.argument());
}
@@ -1202,9 +1206,10 @@
* @param owner the artifact's owner.
*/
protected Artifact getSharedArtifact(String rootRelativePath, ConfiguredTarget owner) {
- return getDerivedArtifact(PathFragment.create(rootRelativePath),
+ return getDerivedArtifact(
+ PathFragment.create(rootRelativePath),
targetConfig.getBinDirectory(RepositoryName.MAIN),
- new ConfiguredTargetKey(owner));
+ ConfiguredTargetKey.of(owner));
}
protected Action getGeneratingActionForLabel(String label) throws Exception {
@@ -1293,7 +1298,7 @@
}
}
- private ConfiguredTargetKey makeConfiguredTargetKey(String label) {
+ private BuildConfiguration getConfiguration(String label) {
BuildConfiguration config;
try {
config = getConfiguredTarget(label).getConfiguration();
@@ -1304,7 +1309,11 @@
//TODO(b/36585204): Clean this up
throw new RuntimeException(e);
}
- return new ConfiguredTargetKey(makeLabel(label), config);
+ return config;
+ }
+
+ private ConfiguredTargetKey makeConfiguredTargetKey(String label) {
+ return ConfiguredTargetKey.of(makeLabel(label), getConfiguration(label));
}
protected static List<String> actionInputsToPaths(Iterable<? extends ActionInput> actionInputs) {
@@ -1923,7 +1932,7 @@
root = configuration.getGenfilesDirectory(repository);
}
ArtifactOwner owner =
- new ConfiguredTargetKey(target.getTarget().getLabel(), target.getConfiguration());
+ ConfiguredTargetKey.of(target.getTarget().getLabel(), target.getConfiguration());
RawAttributeMapper attr = RawAttributeMapper.of(associatedRule);
diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/ActionTemplateExpansionFunctionTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/ActionTemplateExpansionFunctionTest.java
index 3fdb7a0..8e55e34 100644
--- a/src/test/java/com/google/devtools/build/lib/skyframe/ActionTemplateExpansionFunctionTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skyframe/ActionTemplateExpansionFunctionTest.java
@@ -104,8 +104,7 @@
List<Action> actions = evaluate(spawnActionTemplate);
assertThat(actions).hasSize(3);
- ArtifactOwner owner = ActionTemplateExpansionValue.createActionTemplateExpansionKey(
- spawnActionTemplate);
+ ArtifactOwner owner = ActionTemplateExpansionValue.key(spawnActionTemplate);
int i = 0;
for (Action action : actions) {
String childName = "child" + i;
diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/ArtifactFunctionTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/ArtifactFunctionTest.java
index 6e321c5..aba2931 100644
--- a/src/test/java/com/google/devtools/build/lib/skyframe/ArtifactFunctionTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skyframe/ArtifactFunctionTest.java
@@ -321,10 +321,10 @@
}
private void setGeneratingActions() throws InterruptedException {
- if (evaluator.getExistingValue(OWNER_KEY) == null) {
+ if (evaluator.getExistingValue(ALL_OWNER) == null) {
differencer.inject(
ImmutableMap.of(
- OWNER_KEY,
+ ALL_OWNER,
new ActionLookupValue(
actionKeyContext, ImmutableList.<ActionAnalysisMetadata>copyOf(actions), false)));
}
diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/ArtifactFunctionTestCase.java b/src/test/java/com/google/devtools/build/lib/skyframe/ArtifactFunctionTestCase.java
index ff83c4a..375c715 100644
--- a/src/test/java/com/google/devtools/build/lib/skyframe/ArtifactFunctionTestCase.java
+++ b/src/test/java/com/google/devtools/build/lib/skyframe/ArtifactFunctionTestCase.java
@@ -32,7 +32,6 @@
import com.google.devtools.build.lib.vfs.Path;
import com.google.devtools.build.lib.vfs.inmemoryfs.InMemoryFileSystem;
import com.google.devtools.build.skyframe.InMemoryMemoizingEvaluator;
-import com.google.devtools.build.skyframe.LegacySkyKey;
import com.google.devtools.build.skyframe.MemoizingEvaluator;
import com.google.devtools.build.skyframe.RecordingDifferencer;
import com.google.devtools.build.skyframe.SequencedRecordingDifferencer;
@@ -51,7 +50,6 @@
abstract class ArtifactFunctionTestCase {
static final ActionLookupKey ALL_OWNER = new SingletonActionLookupKey();
- static final SkyKey OWNER_KEY = LegacySkyKey.create(SkyFunctions.ACTION_LOOKUP, ALL_OWNER);
protected Set<ActionAnalysisMetadata> actions;
protected boolean fastDigest = false;
@@ -158,13 +156,8 @@
private static class SingletonActionLookupKey extends ActionLookupKey {
@Override
- protected SkyKey getSkyKeyInternal() {
- return OWNER_KEY;
- }
-
- @Override
- protected SkyFunctionName getType() {
- throw new UnsupportedOperationException();
+ public SkyFunctionName functionName() {
+ return SkyFunctions.CONFIGURED_TARGET;
}
}
diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/FilesystemValueCheckerTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/FilesystemValueCheckerTest.java
index eb2d141..8c77934 100644
--- a/src/test/java/com/google/devtools/build/lib/skyframe/FilesystemValueCheckerTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skyframe/FilesystemValueCheckerTest.java
@@ -23,7 +23,6 @@
import com.google.common.collect.ImmutableSet;
import com.google.common.util.concurrent.Runnables;
import com.google.devtools.build.lib.actions.Action;
-import com.google.devtools.build.lib.actions.ActionLookupValue;
import com.google.devtools.build.lib.actions.ActionLookupValue.ActionLookupKey;
import com.google.devtools.build.lib.actions.Artifact;
import com.google.devtools.build.lib.actions.Artifact.SpecialArtifact;
@@ -360,13 +359,12 @@
FileSystemUtils.writeContentAsLatin1(out2.getPath(), "fizzlepop");
SkyKey actionLookupKey =
- ActionLookupValue.key(
- new ActionLookupKey() {
- @Override
- protected SkyFunctionName getType() {
- return SkyFunctionName.FOR_TESTING;
- }
- });
+ new ActionLookupKey() {
+ @Override
+ public SkyFunctionName functionName() {
+ return SkyFunctionName.FOR_TESTING;
+ }
+ };
SkyKey actionKey1 = ActionExecutionValue.key(actionLookupKey, 0);
SkyKey actionKey2 = ActionExecutionValue.key(actionLookupKey, 1);
differencer.inject(
@@ -440,13 +438,12 @@
FileSystemUtils.createDirectoryAndParents(last.getPath());
SkyKey actionLookupKey =
- ActionLookupValue.key(
- new ActionLookupKey() {
- @Override
- protected SkyFunctionName getType() {
- return SkyFunctionName.FOR_TESTING;
- }
- });
+ new ActionLookupKey() {
+ @Override
+ public SkyFunctionName functionName() {
+ return SkyFunctionName.FOR_TESTING;
+ }
+ };
SkyKey actionKey1 = ActionExecutionValue.key(actionLookupKey, 0);
SkyKey actionKey2 = ActionExecutionValue.key(actionLookupKey, 1);
SkyKey actionKeyEmpty = ActionExecutionValue.key(actionLookupKey, 2);
diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/SkyframeAwareActionTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/SkyframeAwareActionTest.java
index 015b1f3..e497ab7 100644
--- a/src/test/java/com/google/devtools/build/lib/skyframe/SkyframeAwareActionTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skyframe/SkyframeAwareActionTest.java
@@ -389,7 +389,7 @@
null);
// Sanity check that our invalidation receiver is working correctly. We'll rely on it again.
- SkyKey actionKey = ActionExecutionValue.key(OWNER_KEY, 0);
+ SkyKey actionKey = ActionExecutionValue.key(ACTION_LOOKUP_KEY, 0);
TrackingEvaluationProgressReceiver.EvaluatedEntry evaluatedAction =
progressReceiver.getEvalutedEntry(actionKey);
assertThat(evaluatedAction).isNotNull();
diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/TimestampBuilderTestCase.java b/src/test/java/com/google/devtools/build/lib/skyframe/TimestampBuilderTestCase.java
index a6b2483..8e569fc 100644
--- a/src/test/java/com/google/devtools/build/lib/skyframe/TimestampBuilderTestCase.java
+++ b/src/test/java/com/google/devtools/build/lib/skyframe/TimestampBuilderTestCase.java
@@ -81,7 +81,6 @@
import com.google.devtools.build.skyframe.EvaluationProgressReceiver;
import com.google.devtools.build.skyframe.EvaluationResult;
import com.google.devtools.build.skyframe.InMemoryMemoizingEvaluator;
-import com.google.devtools.build.skyframe.LegacySkyKey;
import com.google.devtools.build.skyframe.RecordingDifferencer;
import com.google.devtools.build.skyframe.SequencedRecordingDifferencer;
import com.google.devtools.build.skyframe.SequentialBuildDriver;
@@ -109,10 +108,8 @@
* The common code that's shared between various builder tests.
*/
public abstract class TimestampBuilderTestCase extends FoundationTestCase {
- protected static final ActionLookupValue.ActionLookupKey ALL_OWNER =
+ protected static final ActionLookupValue.ActionLookupKey ACTION_LOOKUP_KEY =
new SingletonActionLookupKey();
- protected static final SkyKey OWNER_KEY =
- LegacySkyKey.create(SkyFunctions.ACTION_LOOKUP, ALL_OWNER);
protected static final Predicate<Action> ALWAYS_EXECUTE_FILTER = Predicates.alwaysTrue();
protected static final String CYCLE_MSG = "Yarrrr, there be a cycle up in here";
@@ -238,10 +235,10 @@
return new Builder() {
private void setGeneratingActions() {
- if (evaluator.getExistingValue(OWNER_KEY) == null) {
+ if (evaluator.getExistingValue(ACTION_LOOKUP_KEY) == null) {
differencer.inject(
ImmutableMap.of(
- OWNER_KEY,
+ ACTION_LOOKUP_KEY,
new ActionLookupValue(actionKeyContext, ImmutableList.copyOf(actions), false)));
}
}
@@ -342,7 +339,10 @@
PathFragment execPath = PathFragment.create("out").getRelative(name);
Path path = execRoot.getRelative(execPath);
return new Artifact(
- path, Root.asDerivedRoot(execRoot, execRoot.getRelative("out")), execPath, ALL_OWNER);
+ path,
+ Root.asDerivedRoot(execRoot, execRoot.getRelative("out")),
+ execPath,
+ ACTION_LOOKUP_KEY);
}
/**
@@ -497,13 +497,8 @@
private static class SingletonActionLookupKey extends ActionLookupValue.ActionLookupKey {
@Override
- protected SkyKey getSkyKeyInternal() {
- return OWNER_KEY;
- }
-
- @Override
- protected SkyFunctionName getType() {
- throw new UnsupportedOperationException();
+ public SkyFunctionName functionName() {
+ return SkyFunctions.CONFIGURED_TARGET;
}
}
diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/TreeArtifactBuildTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/TreeArtifactBuildTest.java
index e0bb1f7..3846003 100644
--- a/src/test/java/com/google/devtools/build/lib/skyframe/TreeArtifactBuildTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skyframe/TreeArtifactBuildTest.java
@@ -1182,7 +1182,10 @@
PathFragment execPath = PathFragment.create("out").getRelative(name);
Path path = execRoot.getRelative(execPath);
return new SpecialArtifact(
- path, Root.asDerivedRoot(execRoot, execRoot.getRelative("out")), execPath, ALL_OWNER,
+ path,
+ Root.asDerivedRoot(execRoot, execRoot.getRelative("out")),
+ execPath,
+ ACTION_LOOKUP_KEY,
SpecialArtifactType.TREE);
}
diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/TreeArtifactMetadataTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/TreeArtifactMetadataTest.java
index 3e156be..0f346d6 100644
--- a/src/test/java/com/google/devtools/build/lib/skyframe/TreeArtifactMetadataTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skyframe/TreeArtifactMetadataTest.java
@@ -224,10 +224,10 @@
}
private void setGeneratingActions() throws InterruptedException {
- if (evaluator.getExistingValue(OWNER_KEY) == null) {
+ if (evaluator.getExistingValue(ALL_OWNER) == null) {
differencer.inject(
ImmutableMap.of(
- OWNER_KEY,
+ ALL_OWNER,
new ActionLookupValue(
actionKeyContext, ImmutableList.<ActionAnalysisMetadata>copyOf(actions), false)));
}
