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))); }