Split AspectKey subclasses out of AspectValue. PiperOrigin-RevId: 306879050
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 69919c8..1a71603 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
@@ -25,6 +25,8 @@ import com.google.devtools.build.lib.packages.Rule; import com.google.devtools.build.lib.skyframe.AspectCreationException; import com.google.devtools.build.lib.skyframe.AspectValue; +import com.google.devtools.build.lib.skyframe.AspectValueKey; +import com.google.devtools.build.lib.skyframe.AspectValueKey.AspectKey; import com.google.devtools.build.lib.skyframe.ConfiguredTargetAndData; import com.google.devtools.build.lib.skyframe.ConfiguredTargetKey; import com.google.devtools.build.lib.util.OrderedSetMultimap; @@ -145,21 +147,25 @@ return result; } - private static AspectValue.AspectKey buildAspectKey(AspectCollection.AspectDeps aspectDeps, - HashMap<AspectDescriptor, SkyKey> result, Dependency dep) { + private static AspectKey buildAspectKey( + AspectCollection.AspectDeps aspectDeps, + HashMap<AspectDescriptor, SkyKey> result, + Dependency dep) { if (result.containsKey(aspectDeps.getAspect())) { - return (AspectValue.AspectKey) result.get(aspectDeps.getAspect()).argument(); + return (AspectKey) result.get(aspectDeps.getAspect()).argument(); } - ImmutableList.Builder<AspectValue.AspectKey> dependentAspects = ImmutableList.builder(); + ImmutableList.Builder<AspectKey> dependentAspects = ImmutableList.builder(); for (AspectCollection.AspectDeps path : aspectDeps.getDependentAspects()) { dependentAspects.add(buildAspectKey(path, result, dep)); } - AspectValue.AspectKey aspectKey = AspectValue.createAspectKey( - dep.getLabel(), dep.getConfiguration(), - dependentAspects.build(), - aspectDeps.getAspect(), - dep.getAspectConfiguration(aspectDeps.getAspect())); + AspectKey aspectKey = + AspectValueKey.createAspectKey( + dep.getLabel(), + dep.getConfiguration(), + dependentAspects.build(), + aspectDeps.getAspect(), + dep.getAspectConfiguration(aspectDeps.getAspect())); 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 d28875b..d6313b6 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
@@ -67,8 +67,8 @@ import com.google.devtools.build.lib.profiler.Profiler; import com.google.devtools.build.lib.profiler.SilentCloseable; import com.google.devtools.build.lib.skyframe.AspectValue; -import com.google.devtools.build.lib.skyframe.AspectValue.AspectKey; -import com.google.devtools.build.lib.skyframe.AspectValue.AspectValueKey; +import com.google.devtools.build.lib.skyframe.AspectValueKey; +import com.google.devtools.build.lib.skyframe.AspectValueKey.AspectKey; import com.google.devtools.build.lib.skyframe.BuildConfigurationValue; import com.google.devtools.build.lib.skyframe.ConfiguredTargetKey; import com.google.devtools.build.lib.skyframe.CoverageReportValue; @@ -343,7 +343,7 @@ aspectConfigurations.put( Pair.of(targetSpec.getLabel(), aspect), targetSpec.getConfiguration()); aspectKeys.add( - AspectValue.createSkylarkAspectKey( + AspectValueKey.createSkylarkAspectKey( targetSpec.getLabel(), // For invoking top-level aspects, use the top-level configuration for both the // aspect and the base target while the top-level configuration is untrimmed. @@ -368,12 +368,11 @@ BuildConfiguration configuration = targetSpec.getConfiguration(); aspectConfigurations.put(Pair.of(targetSpec.getLabel(), aspect), configuration); aspectKeys.add( - AspectValue.createAspectKey( + AspectValueKey.createAspectKey( targetSpec.getLabel(), configuration, new AspectDescriptor(aspectFactoryClass, AspectParameters.EMPTY), - configuration - )); + configuration)); } } else { throw new ViewCreationFailedException("Aspect '" + aspect + "' is unknown");
diff --git a/src/main/java/com/google/devtools/build/lib/buildtool/ExecutionProgressReceiver.java b/src/main/java/com/google/devtools/build/lib/buildtool/ExecutionProgressReceiver.java index a8b4ddb..24c7806 100644 --- a/src/main/java/com/google/devtools/build/lib/buildtool/ExecutionProgressReceiver.java +++ b/src/main/java/com/google/devtools/build/lib/buildtool/ExecutionProgressReceiver.java
@@ -20,7 +20,7 @@ import com.google.devtools.build.lib.actions.ActionLookupData; import com.google.devtools.build.lib.skyframe.ActionExecutionInactivityWatchdog; import com.google.devtools.build.lib.skyframe.AspectCompletionValue; -import com.google.devtools.build.lib.skyframe.AspectValue.AspectKey; +import com.google.devtools.build.lib.skyframe.AspectValueKey.AspectKey; import com.google.devtools.build.lib.skyframe.ConfiguredTargetKey; import com.google.devtools.build.lib.skyframe.SkyFunctions; import com.google.devtools.build.lib.skyframe.SkyframeActionExecutor;
diff --git a/src/main/java/com/google/devtools/build/lib/buildtool/ExecutionTool.java b/src/main/java/com/google/devtools/build/lib/buildtool/ExecutionTool.java index 0a92a54..3251b18 100644 --- a/src/main/java/com/google/devtools/build/lib/buildtool/ExecutionTool.java +++ b/src/main/java/com/google/devtools/build/lib/buildtool/ExecutionTool.java
@@ -81,7 +81,7 @@ import com.google.devtools.build.lib.runtime.BlazeRuntime; import com.google.devtools.build.lib.runtime.CommandEnvironment; import com.google.devtools.build.lib.skyframe.AspectValue; -import com.google.devtools.build.lib.skyframe.AspectValue.AspectKey; +import com.google.devtools.build.lib.skyframe.AspectValueKey.AspectKey; import com.google.devtools.build.lib.skyframe.Builder; import com.google.devtools.build.lib.skyframe.ConfiguredTargetKey; import com.google.devtools.build.lib.skyframe.SkyframeExecutor;
diff --git a/src/main/java/com/google/devtools/build/lib/buildtool/SkyframeBuilder.java b/src/main/java/com/google/devtools/build/lib/buildtool/SkyframeBuilder.java index 7df88ce..2b3ab20 100644 --- a/src/main/java/com/google/devtools/build/lib/buildtool/SkyframeBuilder.java +++ b/src/main/java/com/google/devtools/build/lib/buildtool/SkyframeBuilder.java
@@ -40,7 +40,7 @@ import com.google.devtools.build.lib.runtime.KeepGoingOption; import com.google.devtools.build.lib.skyframe.ActionExecutionInactivityWatchdog; import com.google.devtools.build.lib.skyframe.AspectValue; -import com.google.devtools.build.lib.skyframe.AspectValue.AspectKey; +import com.google.devtools.build.lib.skyframe.AspectValueKey.AspectKey; import com.google.devtools.build.lib.skyframe.Builder; import com.google.devtools.build.lib.skyframe.ConfiguredTargetKey; import com.google.devtools.build.lib.skyframe.SkyframeExecutor;
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/AspectCompletionValue.java b/src/main/java/com/google/devtools/build/lib/skyframe/AspectCompletionValue.java index a31bf58..6aa9f18 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/AspectCompletionValue.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/AspectCompletionValue.java
@@ -16,7 +16,7 @@ import com.google.auto.value.AutoValue; import com.google.common.collect.Iterables; import com.google.devtools.build.lib.analysis.TopLevelArtifactContext; -import com.google.devtools.build.lib.skyframe.AspectValue.AspectKey; +import com.google.devtools.build.lib.skyframe.AspectValueKey.AspectKey; import com.google.devtools.build.lib.skyframe.CompletionFunction.TopLevelActionLookupKey; import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec; import com.google.devtools.build.skyframe.SkyFunctionName;
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 35db830..6057530 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
@@ -61,7 +61,7 @@ import com.google.devtools.build.lib.packages.Target; import com.google.devtools.build.lib.packages.Type.ConversionException; import com.google.devtools.build.lib.profiler.memory.CurrentRuleTracker; -import com.google.devtools.build.lib.skyframe.AspectValue.AspectKey; +import com.google.devtools.build.lib.skyframe.AspectValueKey.AspectKey; import com.google.devtools.build.lib.skyframe.ConfiguredTargetFunction.ConfiguredTargetFunctionException; import com.google.devtools.build.lib.skyframe.SkyframeExecutor.BuildViewProvider; import com.google.devtools.build.lib.skyframe.StarlarkImportLookupFunction.StarlarkImportFailedException;
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 d2222b5..e860509 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
@@ -14,425 +14,20 @@ package com.google.devtools.build.lib.skyframe; -import com.google.common.base.MoreObjects; -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.BasicActionLookupValue; import com.google.devtools.build.lib.analysis.ConfiguredAspect; import com.google.devtools.build.lib.analysis.ProviderCollection; -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.packages.Aspect; -import com.google.devtools.build.lib.packages.AspectClass; -import com.google.devtools.build.lib.packages.AspectDescriptor; -import com.google.devtools.build.lib.packages.AspectParameters; import com.google.devtools.build.lib.packages.Package; -import com.google.devtools.build.lib.skyframe.BuildConfigurationValue.Key; -import com.google.devtools.build.lib.skyframe.ConfiguredTargetKey.KeyAndHost; -import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec; +import com.google.devtools.build.lib.skyframe.AspectValueKey.AspectKey; import com.google.devtools.build.lib.syntax.Location; -import com.google.devtools.build.skyframe.SkyFunctionName; import javax.annotation.Nullable; /** An aspect in the context of the Skyframe graph. */ public final class AspectValue extends BasicActionLookupValue implements ConfiguredObjectValue { - /** - * A base class for keys that have AspectValue as a Sky value. - */ - public abstract static class AspectValueKey extends ActionLookupKey { - public abstract String getDescription(); - - @Override - public abstract Label getLabel(); - } - - /** A base class for a key representing an aspect applied to a particular target. */ - @AutoCodec - public static class AspectKey extends AspectValueKey { - private final Label label; - private final ImmutableList<AspectKey> baseKeys; - private final BuildConfigurationValue.Key aspectConfigurationKey; - private final ConfiguredTargetKey baseConfiguredTargetKey; - private final AspectDescriptor aspectDescriptor; - private int hashCode; - - private AspectKey( - Label label, - BuildConfigurationValue.Key aspectConfigurationKey, - ConfiguredTargetKey baseConfiguredTargetKey, - ImmutableList<AspectKey> baseKeys, - AspectDescriptor aspectDescriptor) { - this.baseKeys = baseKeys; - this.label = label; - this.aspectConfigurationKey = aspectConfigurationKey; - this.baseConfiguredTargetKey = baseConfiguredTargetKey; - this.aspectDescriptor = aspectDescriptor; - } - - @AutoCodec.VisibleForSerialization - @AutoCodec.Instantiator - static AspectKey createAspectKey( - Label label, - ConfiguredTargetKey baseConfiguredTargetKey, - ImmutableList<AspectKey> baseKeys, - AspectDescriptor aspectDescriptor, - BuildConfigurationValue.Key aspectConfigurationKey, - boolean aspectConfigurationIsHost) { - return aspectKeyInterner.intern( - aspectConfigurationIsHost - ? new HostAspectKey( - label, - aspectConfigurationKey, - baseConfiguredTargetKey, - baseKeys, - aspectDescriptor) - : new AspectKey( - label, - aspectConfigurationKey, - baseConfiguredTargetKey, - baseKeys, - aspectDescriptor)); - } - - @Override - public SkyFunctionName functionName() { - return SkyFunctions.ASPECT; - } - - - @Override - public Label getLabel() { - return label; - } - - public AspectClass getAspectClass() { - return aspectDescriptor.getAspectClass(); - } - - @Nullable - public AspectParameters getParameters() { - return aspectDescriptor.getParameters(); - } - - public AspectDescriptor getAspectDescriptor() { - return aspectDescriptor; - } - - @Nullable - ImmutableList<AspectKey> getBaseKeys() { - return baseKeys; - } - - @Override - public String getDescription() { - if (baseKeys.isEmpty()) { - return String.format("%s of %s", - aspectDescriptor.getAspectClass().getName(), getLabel()); - } else { - return String.format("%s on top of %s", - aspectDescriptor.getAspectClass().getName(), baseKeys.toString()); - } - } - - // Note that this does not factor into equality/hash-code computations because its value is - // already encoded in the aspectConfigurationKey, albeit in an opaque way. - protected boolean aspectConfigurationIsHost() { - return false; - } - - /** - * Returns the key of the configured target of the aspect; that is, the configuration in which - * the aspect will be evaluated. - * - * <p>In trimmed configuration mode, the aspect may require more fragments than the target on - * which it is being evaluated; in addition to configuration fragments required by the target - * and its dependencies, an aspect has configuration fragment requirements of its own, as well - * as dependencies of its own with their own configuration fragment requirements. - * - * <p>The aspect configuration contains all of these fragments, and is used to create the - * aspect's RuleContext and to retrieve the dependencies. Note that dependencies will have their - * configurations trimmed from this one as normal. - * - * <p>Because of these properties, this configuration is always a superset of the base target's - * configuration. In untrimmed configuration mode, this configuration will be equivalent to the - * base target's configuration. - */ - BuildConfigurationValue.Key getAspectConfigurationKey() { - return aspectConfigurationKey; - } - - /** Returns the key for the base configured target for this aspect. */ - ConfiguredTargetKey getBaseConfiguredTargetKey() { - return baseConfiguredTargetKey; - } - - @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, aspectConfigurationKey, baseConfiguredTargetKey, aspectDescriptor); - } - - @Override - public boolean equals(Object other) { - if (this == other) { - return true; - } - - if (!(other instanceof AspectKey)) { - return false; - } - - AspectKey that = (AspectKey) other; - return Objects.equal(label, that.label) - && Objects.equal(baseKeys, that.baseKeys) - && Objects.equal(aspectConfigurationKey, that.aspectConfigurationKey) - && Objects.equal(baseConfiguredTargetKey, that.baseConfiguredTargetKey) - && Objects.equal(aspectDescriptor, that.aspectDescriptor); - } - - public String prettyPrint() { - if (label == null) { - return "null"; - } - - String baseKeysString = - baseKeys.isEmpty() - ? "" - : String.format(" (over %s)", baseKeys.toString()); - return String.format( - "%s with aspect %s%s%s", - label.toString(), - aspectDescriptor.getAspectClass().getName(), - (aspectConfigurationKey != null && aspectConfigurationIsHost()) ? "(host) " : "", - baseKeysString); - } - - @Override - public String toString() { - return (baseKeys == null ? label : baseKeys.toString()) - + "#" - + aspectDescriptor - + " " - + aspectConfigurationKey - + " " - + baseConfiguredTargetKey - + " " - + aspectDescriptor.getParameters() - + (aspectConfigurationIsHost() ? " (host)" : ""); - } - - AspectKey withLabel(Label label) { - ImmutableList.Builder<AspectKey> newBaseKeys = ImmutableList.builder(); - for (AspectKey baseKey : baseKeys) { - newBaseKeys.add(baseKey.withLabel(label)); - } - - return createAspectKey( - label, - ConfiguredTargetKey.of( - label, - baseConfiguredTargetKey.getConfigurationKey(), - baseConfiguredTargetKey.isHostConfiguration()), - newBaseKeys.build(), - aspectDescriptor, - aspectConfigurationKey, - aspectConfigurationIsHost()); - } - } - - /** An {@link AspectKey} for an aspect in the host configuration. */ - static class HostAspectKey extends AspectKey { - private HostAspectKey( - Label label, - Key aspectConfigurationKey, - ConfiguredTargetKey baseConfiguredTargetKey, - ImmutableList<AspectKey> baseKeys, - AspectDescriptor aspectDescriptor) { - super(label, aspectConfigurationKey, baseConfiguredTargetKey, baseKeys, aspectDescriptor); - } - - @Override - protected boolean aspectConfigurationIsHost() { - return true; - } - } - - /** The key for a Starlark aspect. */ - public static class SkylarkAspectLoadingKey extends AspectValueKey { - - private final Label targetLabel; - private final BuildConfigurationValue.Key aspectConfigurationKey; - private final ConfiguredTargetKey baseConfiguredTargetKey; - private final Label skylarkFileLabel; - private final String skylarkValueName; - private int hashCode; - - private SkylarkAspectLoadingKey( - Label targetLabel, - BuildConfigurationValue.Key aspectConfigurationKey, - ConfiguredTargetKey baseConfiguredTargetKey, - Label skylarkFileLabel, - String skylarkFunctionName) { - this.targetLabel = targetLabel; - this.aspectConfigurationKey = aspectConfigurationKey; - this.baseConfiguredTargetKey = baseConfiguredTargetKey; - this.skylarkFileLabel = skylarkFileLabel; - this.skylarkValueName = skylarkFunctionName; - } - - @Override - public SkyFunctionName functionName() { - return SkyFunctions.LOAD_STARLARK_ASPECT; - } - - String getSkylarkValueName() { - return skylarkValueName; - } - - Label getSkylarkFileLabel() { - return skylarkFileLabel; - } - - protected boolean isAspectConfigurationHost() { - return false; - } - - @Override - public Label getLabel() { - return targetLabel; - } - - @Override - public String getDescription() { - // Starlark aspects are referred to on command line with <file>%<value ame> - return String.format("%s%%%s of %s", skylarkFileLabel, skylarkValueName, targetLabel); - } - - @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, - aspectConfigurationKey, - baseConfiguredTargetKey, - skylarkFileLabel, - skylarkValueName); - } - - @Override - public boolean equals(Object o) { - if (o == this) { - return true; - } - - if (!(o instanceof SkylarkAspectLoadingKey)) { - return false; - } - SkylarkAspectLoadingKey that = (SkylarkAspectLoadingKey) o; - return Objects.equal(targetLabel, that.targetLabel) - && Objects.equal(aspectConfigurationKey, that.aspectConfigurationKey) - && Objects.equal(baseConfiguredTargetKey, that.baseConfiguredTargetKey) - && Objects.equal(skylarkFileLabel, that.skylarkFileLabel) - && Objects.equal(skylarkValueName, that.skylarkValueName); - } - - @Override - public String toString() { - return MoreObjects.toStringHelper(this) - .add("targetLabel", targetLabel) - .add("aspectConfigurationKey", aspectConfigurationKey) - .add("baseConfiguredTargetKey", baseConfiguredTargetKey) - .add("skylarkFileLabel", skylarkFileLabel) - .add("skylarkValueName", skylarkValueName) - .toString(); - } - - AspectKey toAspectKey(AspectClass aspectClass) { - return AspectKey.createAspectKey( - targetLabel, - baseConfiguredTargetKey, - ImmutableList.of(), - new AspectDescriptor(aspectClass, AspectParameters.EMPTY), - aspectConfigurationKey, - isAspectConfigurationHost()); - } - } - - /** A {@link SkylarkAspectLoadingKey} for an aspect in the host configuration. */ - private static class HostSkylarkAspectLoadingKey extends SkylarkAspectLoadingKey { - - private HostSkylarkAspectLoadingKey( - Label targetLabel, - Key aspectConfigurationKey, - ConfiguredTargetKey baseConfiguredTargetKey, - Label skylarkFileLabel, - String skylarkFunctionName) { - super( - targetLabel, - aspectConfigurationKey, - baseConfiguredTargetKey, - skylarkFileLabel, - skylarkFunctionName); - } - - @Override - protected boolean isAspectConfigurationHost() { - return true; - } - } // These variables are only non-final because they may be clear()ed to save memory. They are null // only after they are cleared except for transitivePackagesForPackageRootResolution. @@ -517,65 +112,4 @@ public ProviderCollection getConfiguredObject() { return getConfiguredAspect(); } - - public static AspectKey createAspectKey( - Label label, - BuildConfiguration baseConfiguration, - ImmutableList<AspectKey> baseKeys, - AspectDescriptor aspectDescriptor, - BuildConfiguration aspectConfiguration) { - KeyAndHost aspectKeyAndHost = ConfiguredTargetKey.keyFromConfiguration(aspectConfiguration); - return AspectKey.createAspectKey( - label, - ConfiguredTargetKey.of(label, baseConfiguration), - baseKeys, - aspectDescriptor, - aspectKeyAndHost.key, - aspectKeyAndHost.isHost); - } - - private static final Interner<AspectKey> aspectKeyInterner = BlazeInterners.newWeakInterner(); - - public static AspectKey createAspectKey( - Label label, - BuildConfiguration baseConfiguration, - AspectDescriptor aspectDescriptor, - BuildConfiguration aspectConfiguration) { - KeyAndHost aspectKeyAndHost = ConfiguredTargetKey.keyFromConfiguration(aspectConfiguration); - return AspectKey.createAspectKey( - label, - ConfiguredTargetKey.of(label, baseConfiguration), - ImmutableList.of(), - aspectDescriptor, - aspectKeyAndHost.key, - aspectKeyAndHost.isHost); - } - - private static final Interner<SkylarkAspectLoadingKey> skylarkAspectKeyInterner = - BlazeInterners.newWeakInterner(); - - public static SkylarkAspectLoadingKey createSkylarkAspectKey( - Label targetLabel, - BuildConfiguration aspectConfiguration, - BuildConfiguration targetConfiguration, - Label skylarkFileLabel, - String skylarkExportName) { - KeyAndHost keyAndHost = ConfiguredTargetKey.keyFromConfiguration(aspectConfiguration); - SkylarkAspectLoadingKey key = - keyAndHost.isHost - ? new HostSkylarkAspectLoadingKey( - targetLabel, - keyAndHost.key, - ConfiguredTargetKey.of(targetLabel, targetConfiguration), - skylarkFileLabel, - skylarkExportName) - : new SkylarkAspectLoadingKey( - targetLabel, - keyAndHost.key, - ConfiguredTargetKey.of(targetLabel, targetConfiguration), - skylarkFileLabel, - skylarkExportName); - - return skylarkAspectKeyInterner.intern(key); - } }
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/AspectValueKey.java b/src/main/java/com/google/devtools/build/lib/skyframe/AspectValueKey.java new file mode 100644 index 0000000..77ee261 --- /dev/null +++ b/src/main/java/com/google/devtools/build/lib/skyframe/AspectValueKey.java
@@ -0,0 +1,487 @@ +// Copyright 2020 The Bazel Authors. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +package com.google.devtools.build.lib.skyframe; + +import com.google.common.base.MoreObjects; +import com.google.common.base.Objects; +import com.google.common.collect.ImmutableList; +import com.google.common.collect.Interner; +import com.google.devtools.build.lib.actions.ActionLookupValue.ActionLookupKey; +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.lib.packages.AspectClass; +import com.google.devtools.build.lib.packages.AspectDescriptor; +import com.google.devtools.build.lib.packages.AspectParameters; +import com.google.devtools.build.lib.skyframe.ConfiguredTargetKey.KeyAndHost; +import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec; +import com.google.devtools.build.skyframe.SkyFunctionName; +import javax.annotation.Nullable; + +/** A base class for keys that have AspectValue as a Sky value. */ +public abstract class AspectValueKey extends ActionLookupKey { + + private static final Interner<AspectKey> aspectKeyInterner = BlazeInterners.newWeakInterner(); + private static final Interner<SkylarkAspectLoadingKey> skylarkAspectKeyInterner = + BlazeInterners.newWeakInterner(); + + public abstract String getDescription(); + + @Override + public abstract Label getLabel(); + + // Methods to create aspect keys. + + public static AspectKey createAspectKey( + Label label, + BuildConfiguration baseConfiguration, + ImmutableList<AspectKey> baseKeys, + AspectDescriptor aspectDescriptor, + BuildConfiguration aspectConfiguration) { + KeyAndHost aspectKeyAndHost = ConfiguredTargetKey.keyFromConfiguration(aspectConfiguration); + return AspectKey.createAspectKey( + label, + ConfiguredTargetKey.of(label, baseConfiguration), + baseKeys, + aspectDescriptor, + aspectKeyAndHost.key, + aspectKeyAndHost.isHost); + } + + public static AspectKey createAspectKey( + Label label, + BuildConfiguration baseConfiguration, + AspectDescriptor aspectDescriptor, + BuildConfiguration aspectConfiguration) { + KeyAndHost aspectKeyAndHost = ConfiguredTargetKey.keyFromConfiguration(aspectConfiguration); + return AspectKey.createAspectKey( + label, + ConfiguredTargetKey.of(label, baseConfiguration), + ImmutableList.of(), + aspectDescriptor, + aspectKeyAndHost.key, + aspectKeyAndHost.isHost); + } + + public static SkylarkAspectLoadingKey createSkylarkAspectKey( + Label targetLabel, + BuildConfiguration aspectConfiguration, + BuildConfiguration targetConfiguration, + Label skylarkFileLabel, + String skylarkExportName) { + KeyAndHost keyAndHost = ConfiguredTargetKey.keyFromConfiguration(aspectConfiguration); + SkylarkAspectLoadingKey key = + keyAndHost.isHost + ? new HostSkylarkAspectLoadingKey( + targetLabel, + keyAndHost.key, + ConfiguredTargetKey.of(targetLabel, targetConfiguration), + skylarkFileLabel, + skylarkExportName) + : new SkylarkAspectLoadingKey( + targetLabel, + keyAndHost.key, + ConfiguredTargetKey.of(targetLabel, targetConfiguration), + skylarkFileLabel, + skylarkExportName); + + return skylarkAspectKeyInterner.intern(key); + } + + // Specific subtypes of aspect keys. + + /** A base class for a key representing an aspect applied to a particular target. */ + @AutoCodec + public static class AspectKey extends AspectValueKey { + private final Label label; + private final ImmutableList<AspectKey> baseKeys; + private final BuildConfigurationValue.Key aspectConfigurationKey; + private final ConfiguredTargetKey baseConfiguredTargetKey; + private final AspectDescriptor aspectDescriptor; + private int hashCode; + + private AspectKey( + Label label, + BuildConfigurationValue.Key aspectConfigurationKey, + ConfiguredTargetKey baseConfiguredTargetKey, + ImmutableList<AspectKey> baseKeys, + AspectDescriptor aspectDescriptor) { + this.baseKeys = baseKeys; + this.label = label; + this.aspectConfigurationKey = aspectConfigurationKey; + this.baseConfiguredTargetKey = baseConfiguredTargetKey; + this.aspectDescriptor = aspectDescriptor; + } + + @AutoCodec.VisibleForSerialization + @AutoCodec.Instantiator + static AspectKey createAspectKey( + Label label, + ConfiguredTargetKey baseConfiguredTargetKey, + ImmutableList<AspectKey> baseKeys, + AspectDescriptor aspectDescriptor, + BuildConfigurationValue.Key aspectConfigurationKey, + boolean aspectConfigurationIsHost) { + return aspectKeyInterner.intern( + aspectConfigurationIsHost + ? new HostAspectKey( + label, + aspectConfigurationKey, + baseConfiguredTargetKey, + baseKeys, + aspectDescriptor) + : new AspectKey( + label, + aspectConfigurationKey, + baseConfiguredTargetKey, + baseKeys, + aspectDescriptor)); + } + + @Override + public SkyFunctionName functionName() { + return SkyFunctions.ASPECT; + } + + + @Override + public Label getLabel() { + return label; + } + + public AspectClass getAspectClass() { + return aspectDescriptor.getAspectClass(); + } + + @Nullable + public AspectParameters getParameters() { + return aspectDescriptor.getParameters(); + } + + public AspectDescriptor getAspectDescriptor() { + return aspectDescriptor; + } + + @Nullable + ImmutableList<AspectKey> getBaseKeys() { + return baseKeys; + } + + @Override + public String getDescription() { + if (baseKeys.isEmpty()) { + return String.format("%s of %s", + aspectDescriptor.getAspectClass().getName(), getLabel()); + } else { + return String.format("%s on top of %s", + aspectDescriptor.getAspectClass().getName(), baseKeys.toString()); + } + } + + // Note that this does not factor into equality/hash-code computations because its value is + // already encoded in the aspectConfigurationKey, albeit in an opaque way. + protected boolean aspectConfigurationIsHost() { + return false; + } + + /** + * Returns the key of the configured target of the aspect; that is, the configuration in which + * the aspect will be evaluated. + * + * <p>In trimmed configuration mode, the aspect may require more fragments than the target on + * which it is being evaluated; in addition to configuration fragments required by the target + * and its dependencies, an aspect has configuration fragment requirements of its own, as well + * as dependencies of its own with their own configuration fragment requirements. + * + * <p>The aspect configuration contains all of these fragments, and is used to create the + * aspect's RuleContext and to retrieve the dependencies. Note that dependencies will have their + * configurations trimmed from this one as normal. + * + * <p>Because of these properties, this configuration is always a superset of the base target's + * configuration. In untrimmed configuration mode, this configuration will be equivalent to the + * base target's configuration. + */ + BuildConfigurationValue.Key getAspectConfigurationKey() { + return aspectConfigurationKey; + } + + /** Returns the key for the base configured target for this aspect. */ + ConfiguredTargetKey getBaseConfiguredTargetKey() { + return baseConfiguredTargetKey; + } + + @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, aspectConfigurationKey, baseConfiguredTargetKey, aspectDescriptor); + } + + @Override + public boolean equals(Object other) { + if (this == other) { + return true; + } + + if (!(other instanceof AspectKey)) { + return false; + } + + AspectKey that = (AspectKey) other; + return Objects.equal(label, that.label) + && Objects.equal(baseKeys, that.baseKeys) + && Objects.equal(aspectConfigurationKey, that.aspectConfigurationKey) + && Objects.equal(baseConfiguredTargetKey, that.baseConfiguredTargetKey) + && Objects.equal(aspectDescriptor, that.aspectDescriptor); + } + + public String prettyPrint() { + if (label == null) { + return "null"; + } + + String baseKeysString = + baseKeys.isEmpty() + ? "" + : String.format(" (over %s)", baseKeys.toString()); + return String.format( + "%s with aspect %s%s%s", + label.toString(), + aspectDescriptor.getAspectClass().getName(), + (aspectConfigurationKey != null && aspectConfigurationIsHost()) ? "(host) " : "", + baseKeysString); + } + + @Override + public String toString() { + return (baseKeys == null ? label : baseKeys.toString()) + + "#" + + aspectDescriptor + + " " + + aspectConfigurationKey + + " " + + baseConfiguredTargetKey + + " " + + aspectDescriptor.getParameters() + + (aspectConfigurationIsHost() ? " (host)" : ""); + } + + AspectKey withLabel(Label label) { + ImmutableList.Builder<AspectKey> newBaseKeys = ImmutableList.builder(); + for (AspectKey baseKey : baseKeys) { + newBaseKeys.add(baseKey.withLabel(label)); + } + + return createAspectKey( + label, + ConfiguredTargetKey.of( + label, + baseConfiguredTargetKey.getConfigurationKey(), + baseConfiguredTargetKey.isHostConfiguration()), + newBaseKeys.build(), + aspectDescriptor, + aspectConfigurationKey, + aspectConfigurationIsHost()); + } + } + + /** An {@link AspectKey} for an aspect in the host configuration. */ + static class HostAspectKey extends AspectKey { + private HostAspectKey( + Label label, + BuildConfigurationValue.Key aspectConfigurationKey, + ConfiguredTargetKey baseConfiguredTargetKey, + ImmutableList<AspectKey> baseKeys, + AspectDescriptor aspectDescriptor) { + super(label, aspectConfigurationKey, baseConfiguredTargetKey, baseKeys, aspectDescriptor); + } + + @Override + protected boolean aspectConfigurationIsHost() { + return true; + } + } + + /** The key for a Starlark aspect. */ + public static class SkylarkAspectLoadingKey extends AspectValueKey { + + private final Label targetLabel; + private final BuildConfigurationValue.Key aspectConfigurationKey; + private final ConfiguredTargetKey baseConfiguredTargetKey; + private final Label skylarkFileLabel; + private final String skylarkValueName; + private int hashCode; + + private SkylarkAspectLoadingKey( + Label targetLabel, + BuildConfigurationValue.Key aspectConfigurationKey, + ConfiguredTargetKey baseConfiguredTargetKey, + Label skylarkFileLabel, + String skylarkFunctionName) { + this.targetLabel = targetLabel; + this.aspectConfigurationKey = aspectConfigurationKey; + this.baseConfiguredTargetKey = baseConfiguredTargetKey; + this.skylarkFileLabel = skylarkFileLabel; + this.skylarkValueName = skylarkFunctionName; + } + + @Override + public SkyFunctionName functionName() { + return SkyFunctions.LOAD_STARLARK_ASPECT; + } + + String getSkylarkValueName() { + return skylarkValueName; + } + + Label getSkylarkFileLabel() { + return skylarkFileLabel; + } + + protected boolean isAspectConfigurationHost() { + return false; + } + + @Override + public Label getLabel() { + return targetLabel; + } + + @Override + public String getDescription() { + // Starlark aspects are referred to on command line with <file>%<value ame> + return String.format("%s%%%s of %s", skylarkFileLabel, skylarkValueName, targetLabel); + } + + @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, + aspectConfigurationKey, + baseConfiguredTargetKey, + skylarkFileLabel, + skylarkValueName); + } + + @Override + public boolean equals(Object o) { + if (o == this) { + return true; + } + + if (!(o instanceof SkylarkAspectLoadingKey)) { + return false; + } + SkylarkAspectLoadingKey that = (SkylarkAspectLoadingKey) o; + return Objects.equal(targetLabel, that.targetLabel) + && Objects.equal(aspectConfigurationKey, that.aspectConfigurationKey) + && Objects.equal(baseConfiguredTargetKey, that.baseConfiguredTargetKey) + && Objects.equal(skylarkFileLabel, that.skylarkFileLabel) + && Objects.equal(skylarkValueName, that.skylarkValueName); + } + + @Override + public String toString() { + return MoreObjects.toStringHelper(this) + .add("targetLabel", targetLabel) + .add("aspectConfigurationKey", aspectConfigurationKey) + .add("baseConfiguredTargetKey", baseConfiguredTargetKey) + .add("skylarkFileLabel", skylarkFileLabel) + .add("skylarkValueName", skylarkValueName) + .toString(); + } + + AspectKey toAspectKey(AspectClass aspectClass) { + return AspectKey.createAspectKey( + targetLabel, + baseConfiguredTargetKey, + ImmutableList.of(), + new AspectDescriptor(aspectClass, AspectParameters.EMPTY), + aspectConfigurationKey, + isAspectConfigurationHost()); + } + } + + /** A {@link SkylarkAspectLoadingKey} for an aspect in the host configuration. */ + private static class HostSkylarkAspectLoadingKey extends SkylarkAspectLoadingKey { + + private HostSkylarkAspectLoadingKey( + Label targetLabel, + BuildConfigurationValue.Key aspectConfigurationKey, + ConfiguredTargetKey baseConfiguredTargetKey, + Label skylarkFileLabel, + String skylarkFunctionName) { + super( + targetLabel, + aspectConfigurationKey, + baseConfiguredTargetKey, + skylarkFileLabel, + skylarkFunctionName); + } + + @Override + protected boolean isAspectConfigurationHost() { + return true; + } + } +}
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/Builder.java b/src/main/java/com/google/devtools/build/lib/skyframe/Builder.java index 7ad23c2..de1f8f2 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/Builder.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/Builder.java
@@ -23,7 +23,7 @@ import com.google.devtools.build.lib.analysis.TopLevelArtifactContext; import com.google.devtools.build.lib.concurrent.ThreadSafety.ThreadCompatible; import com.google.devtools.build.lib.events.Reporter; -import com.google.devtools.build.lib.skyframe.AspectValue.AspectKey; +import com.google.devtools.build.lib.skyframe.AspectValueKey.AspectKey; import com.google.devtools.build.lib.util.AbruptExitException; import com.google.devtools.common.options.OptionsProvider; import java.util.Collection;
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ConfiguredTargetCycleReporter.java b/src/main/java/com/google/devtools/build/lib/skyframe/ConfiguredTargetCycleReporter.java index be9f267..674320a 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/ConfiguredTargetCycleReporter.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/ConfiguredTargetCycleReporter.java
@@ -24,7 +24,7 @@ import com.google.devtools.build.lib.cmdline.Label; import com.google.devtools.build.lib.events.ExtendedEventHandler; import com.google.devtools.build.lib.pkgcache.PackageProvider; -import com.google.devtools.build.lib.skyframe.AspectValue.AspectKey; +import com.google.devtools.build.lib.skyframe.AspectValueKey.AspectKey; import com.google.devtools.build.skyframe.CycleInfo; import com.google.devtools.build.skyframe.SkyKey; @@ -104,8 +104,8 @@ return ((ConfiguredTargetKey) key.argument()).prettyPrint(); } else if (SkyFunctions.isSkyFunction(SkyFunctions.ASPECT).apply(key)) { return ((AspectKey) key.argument()).prettyPrint(); - } else if (key instanceof AspectValue.AspectValueKey) { - return ((AspectValue.AspectValueKey) key).getDescription(); + } else if (key instanceof AspectValueKey) { + return ((AspectValueKey) key).getDescription(); } else { return getLabel(key).toString(); }
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 59fd8ea..a7e6ff0 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
@@ -63,7 +63,7 @@ import com.google.devtools.build.lib.query2.aquery.AqueryActionFilter; import com.google.devtools.build.lib.repository.ExternalPackageHelper; import com.google.devtools.build.lib.rules.repository.ManagedDirectoriesKnowledge; -import com.google.devtools.build.lib.skyframe.AspectValue.AspectKey; +import com.google.devtools.build.lib.skyframe.AspectValueKey.AspectKey; import com.google.devtools.build.lib.skyframe.DiffAwarenessManager.ProcessableModifiedFileSet; import com.google.devtools.build.lib.skyframe.DirtinessCheckerUtils.BasicFilesystemDirtinessChecker; import com.google.devtools.build.lib.skyframe.DirtinessCheckerUtils.ExternalDirtinessChecker;
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 80195f1..322f63d 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
@@ -75,7 +75,6 @@ import com.google.devtools.build.lib.profiler.Profiler; import com.google.devtools.build.lib.profiler.SilentCloseable; import com.google.devtools.build.lib.skyframe.ArtifactConflictFinder.ConflictException; -import com.google.devtools.build.lib.skyframe.AspectValue.AspectValueKey; import com.google.devtools.build.lib.util.OrderedSetMultimap; import com.google.devtools.build.lib.util.Pair; import com.google.devtools.build.lib.vfs.Root;
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 57cf396..6366c3e 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
@@ -140,7 +140,6 @@ import com.google.devtools.build.lib.runtime.KeepGoingOption; import com.google.devtools.build.lib.server.FailureDetails; import com.google.devtools.build.lib.server.FailureDetails.FailureDetail; -import com.google.devtools.build.lib.skyframe.AspectValue.AspectValueKey; import com.google.devtools.build.lib.skyframe.DirtinessCheckerUtils.FileDirtinessChecker; import com.google.devtools.build.lib.skyframe.ExternalFilesHelper.ExternalFileAction; import com.google.devtools.build.lib.skyframe.FileFunction.NonexistentFileReceiver; @@ -1759,7 +1758,8 @@ skyKeys.add(ConfiguredTargetKey.of(key.getLabel(), depConfig)); for (AspectDescriptor aspectDescriptor : key.getAspects().getAllAspects()) { skyKeys.add( - AspectValue.createAspectKey(key.getLabel(), depConfig, aspectDescriptor, depConfig)); + AspectValueKey.createAspectKey( + key.getLabel(), depConfig, aspectDescriptor, depConfig)); } } skyKeys.add(PackageValue.key(key.getLabel().getPackageIdentifier())); @@ -1810,7 +1810,8 @@ for (AspectDescriptor aspectDescriptor : key.getAspects().getAllAspects()) { SkyKey aspectKey = - AspectValue.createAspectKey(key.getLabel(), depConfig, aspectDescriptor, depConfig); + AspectValueKey.createAspectKey( + key.getLabel(), depConfig, aspectDescriptor, depConfig); if (result.get(aspectKey) == null) { continue DependentNodeLoop; }
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 b8a994d..d41147e 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,7 @@ import com.google.devtools.build.lib.causes.LabelCause; import com.google.devtools.build.lib.cmdline.Label; import com.google.devtools.build.lib.packages.SkylarkAspect; -import com.google.devtools.build.lib.skyframe.AspectValue.SkylarkAspectLoadingKey; +import com.google.devtools.build.lib.skyframe.AspectValueKey.SkylarkAspectLoadingKey; import com.google.devtools.build.skyframe.SkyFunction; import com.google.devtools.build.skyframe.SkyFunctionException; import com.google.devtools.build.skyframe.SkyKey;