From ce6ad29c88084fb327856149a39e2771e544cfa5 Mon Sep 17 00:00:00 2001 From: Googler Date: Fri, 20 Dec 2019 10:29:21 -0800 Subject: [PATCH] Remove dep fingerprinting, part 2 of 2. Removes SkyValue#getValueFingerprint and Version#getFingerprint. This should also free a little bit of memory that was used in configured target values. getValueFingerprint is kept in two SkyValue subclasses (FileStateValue and FileArtifactValue) where it is actually used for another purpose. However, the method is removed from the SkyValue interface and other classes where it is not being used. PiperOrigin-RevId: 286601483 --- .../build/lib/actions/ActionLookupValue.java | 18 +------ .../lib/actions/BasicActionLookupValue.java | 12 ++--- .../build/lib/actions/FileArtifactValue.java | 1 - .../build/lib/actions/FileStateValue.java | 1 - .../lib/skyframe/ActionExecutionValue.java | 49 +------------------ .../ActionTemplateExpansionValue.java | 2 +- .../skyframe/AggregatingArtifactValue.java | 27 ---------- .../build/lib/skyframe/AspectFunction.java | 14 ++---- .../build/lib/skyframe/AspectValue.java | 6 +-- .../skyframe/BuildInfoCollectionValue.java | 2 +- .../skyframe/ConfiguredTargetFunction.java | 16 ++---- .../lib/skyframe/CoverageReportValue.java | 2 +- .../NonRuleConfiguredTargetValue.java | 8 ++- .../lib/skyframe/PackageLookupValue.java | 31 ++---------- .../skyframe/RuleConfiguredTargetValue.java | 10 +--- .../lib/skyframe/RunfilesArtifactValue.java | 8 --- .../build/lib/skyframe/SkyframeExecutor.java | 13 +---- .../build/lib/skyframe/TreeArtifactValue.java | 13 ----- .../lib/skyframe/WorkspaceStatusValue.java | 4 +- .../build/skyframe/MinimalVersion.java | 18 ++----- .../devtools/build/skyframe/SkyValue.java | 11 ----- .../devtools/build/skyframe/Version.java | 8 --- .../ActionTemplateExpansionFunctionTest.java | 3 +- .../lib/skyframe/ArtifactFunctionTest.java | 13 +---- ...SingleToolchainResolutionFunctionTest.java | 5 +- .../skyframe/TimestampBuilderTestCase.java | 3 +- .../skyframe/TreeArtifactMetadataTest.java | 3 +- 27 files changed, 42 insertions(+), 259 deletions(-) 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 b99ad97b497a9e..6af1c134a666b2 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 @@ -18,18 +18,11 @@ import com.google.devtools.build.lib.cmdline.Label; import com.google.devtools.build.skyframe.SkyKey; import com.google.devtools.build.skyframe.SkyValue; -import java.math.BigInteger; import javax.annotation.Nullable; -/** - * Base class for all values which can provide the generating action of an artifact. - */ +/** Base class for all values which can provide the generating action of an artifact. */ public abstract class ActionLookupValue implements SkyValue { - @Nullable private final transient BigInteger nonceVersion; - protected ActionLookupValue(@Nullable BigInteger nonceVersion) { - this.nonceVersion = nonceVersion; - } /** Returns a list of actions registered by this {@link SkyValue}. */ public abstract ImmutableList getActions(); @@ -57,9 +50,7 @@ public ActionTemplate getActionTemplate(int index) { return (ActionTemplate) result; } - /** - * Returns the number of {@link Action} objects present in this value. - */ + /** Returns the number of {@link Action} objects present in this value. */ public int getNumActions() { return getActions().size(); } @@ -70,11 +61,6 @@ public SourceArtifact getSourceArtifact() { return null; } - @Override - public BigInteger getValueFingerprint() { - return nonceVersion; - } - /** * 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 diff --git a/src/main/java/com/google/devtools/build/lib/actions/BasicActionLookupValue.java b/src/main/java/com/google/devtools/build/lib/actions/BasicActionLookupValue.java index 2e3eef90018947..06d511ac5badd0 100644 --- a/src/main/java/com/google/devtools/build/lib/actions/BasicActionLookupValue.java +++ b/src/main/java/com/google/devtools/build/lib/actions/BasicActionLookupValue.java @@ -17,8 +17,6 @@ import com.google.common.base.MoreObjects; import com.google.common.base.MoreObjects.ToStringHelper; import com.google.common.collect.ImmutableList; -import java.math.BigInteger; -import javax.annotation.Nullable; /** * Basic implementation of {@link ActionLookupValue} where the value itself owns and maintains @@ -27,17 +25,13 @@ public class BasicActionLookupValue extends ActionLookupValue { protected final ImmutableList actions; - protected BasicActionLookupValue( - ImmutableList actions, - @Nullable BigInteger nonceVersion) { - super(nonceVersion); + protected BasicActionLookupValue(ImmutableList actions) { this.actions = actions; } @VisibleForTesting - public BasicActionLookupValue( - Actions.GeneratingActions generatingActions, @Nullable BigInteger nonceVersion) { - this(generatingActions.getActions(), nonceVersion); + public BasicActionLookupValue(Actions.GeneratingActions generatingActions) { + this(generatingActions.getActions()); } @Override diff --git a/src/main/java/com/google/devtools/build/lib/actions/FileArtifactValue.java b/src/main/java/com/google/devtools/build/lib/actions/FileArtifactValue.java index 2b76b3f01086d4..b988f18b69c8b1 100644 --- a/src/main/java/com/google/devtools/build/lib/actions/FileArtifactValue.java +++ b/src/main/java/com/google/devtools/build/lib/actions/FileArtifactValue.java @@ -92,7 +92,6 @@ public abstract class FileArtifactValue implements SkyValue, HasDigest { public abstract FileContentsProxy getContentsProxy(); @Nullable - @Override public BigInteger getValueFingerprint() { byte[] digest = getDigest(); if (digest != null) { diff --git a/src/main/java/com/google/devtools/build/lib/actions/FileStateValue.java b/src/main/java/com/google/devtools/build/lib/actions/FileStateValue.java index ecd302a4ac66db..3d400b24a41dc8 100644 --- a/src/main/java/com/google/devtools/build/lib/actions/FileStateValue.java +++ b/src/main/java/com/google/devtools/build/lib/actions/FileStateValue.java @@ -180,7 +180,6 @@ public byte[] getDigest() { throw new IllegalStateException(); } - @Override public abstract BigInteger getValueFingerprint(); @Override diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ActionExecutionValue.java b/src/main/java/com/google/devtools/build/lib/skyframe/ActionExecutionValue.java index 8fc740763a6aac..5db43282923f12 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/ActionExecutionValue.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/ActionExecutionValue.java @@ -31,20 +31,13 @@ import com.google.devtools.build.lib.collect.nestedset.NestedSet; import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable; import com.google.devtools.build.lib.concurrent.ThreadSafety.ThreadSafe; -import com.google.devtools.build.lib.util.BigIntegerFingerprint; import com.google.devtools.build.skyframe.SkyValue; -import java.math.BigInteger; -import java.util.Comparator; import java.util.Map; -import java.util.Map.Entry; import java.util.function.Function; import java.util.stream.Collectors; -import java.util.stream.Stream; import javax.annotation.Nullable; -/** - * A value representing an executed action. - */ +/** A value representing an executed action. */ @Immutable @ThreadSafe public class ActionExecutionValue implements SkyValue { @@ -59,11 +52,6 @@ public class ActionExecutionValue implements SkyValue { @Nullable private final NestedSet discoveredModules; - /** - * Transient because it can be reconstituted on demand, and {@link BigInteger} isn't serializable. - */ - @Nullable private transient BigInteger valueFingerprint; - /** * @param artifactData Map from Artifacts to corresponding {@link FileArtifactValue}. * @param treeArtifactData All tree artifact data. @@ -172,38 +160,6 @@ public NestedSet getDiscoveredModules() { return discoveredModules; } - @Override - public BigInteger getValueFingerprint() { - if (valueFingerprint == null) { - BigIntegerFingerprint fp = new BigIntegerFingerprint(); - sortMapByArtifactExecPathAndStream(artifactData) - .forEach( - (entry) -> { - fp.addPath(entry.getKey().getExecPath()); - fp.addBigIntegerOrdered(entry.getValue().getValueFingerprint()); - }); - sortMapByArtifactExecPathAndStream(treeArtifactData) - .forEach( - (entry) -> { - fp.addPath(entry.getKey().getExecPath()); - fp.addBigIntegerOrdered(entry.getValue().getValueFingerprint()); - }); - if (outputSymlinks != null) { - for (FilesetOutputSymlink symlink : outputSymlinks) { - fp.addBigIntegerOrdered(symlink.getFingerprint()); - } - } - valueFingerprint = fp.getFingerprint(); - } - return valueFingerprint; - } - - private static Stream> sortMapByArtifactExecPathAndStream( - Map inputMap) { - return inputMap.entrySet().stream() - .sorted(Comparator.comparing(Entry::getKey, Artifact.EXEC_PATH_COMPARATOR)); - } - @Override public String toString() { return MoreObjects.toStringHelper(this) @@ -310,8 +266,7 @@ private static ImmutableMap transformKeys( ActionExecutionValue transformForSharedAction(ImmutableSet outputs) { Map newArtifactMap = - outputs - .stream() + outputs.stream() .collect(Collectors.toMap(OwnerlessArtifactWrapper::new, Function.identity())); // This is only called for shared actions, so we'll almost certainly have to transform all keys // in all sets. 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 e460f47f34d44b..8ee7db49046e1a 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 @@ -27,7 +27,7 @@ */ public final class ActionTemplateExpansionValue extends BasicActionLookupValue { ActionTemplateExpansionValue(GeneratingActions generatingActions) { - super(generatingActions, /*nonceVersion=*/ null); + super(generatingActions); } static ActionTemplateExpansionKey key(ActionLookupKey actionLookupKey, int actionIndex) { diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/AggregatingArtifactValue.java b/src/main/java/com/google/devtools/build/lib/skyframe/AggregatingArtifactValue.java index 6d24c9f373db37..4b9c55bb686784 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/AggregatingArtifactValue.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/AggregatingArtifactValue.java @@ -17,12 +17,9 @@ import com.google.common.collect.ImmutableList; import com.google.devtools.build.lib.actions.Artifact; import com.google.devtools.build.lib.actions.FileArtifactValue; -import com.google.devtools.build.lib.util.BigIntegerFingerprint; import com.google.devtools.build.lib.util.Pair; import com.google.devtools.build.skyframe.SkyValue; -import java.math.BigInteger; import java.util.Collection; -import javax.annotation.Nullable; /** Value for aggregating artifacts, which must be expanded to a set of other artifacts. */ class AggregatingArtifactValue implements SkyValue { @@ -57,30 +54,6 @@ FileArtifactValue getSelfData() { return selfData; } - protected final BigIntegerFingerprint getFingerprintBuilder() { - BigIntegerFingerprint fp = new BigIntegerFingerprint(); - fp.addNullableBigIntegerOrdered(selfData.getValueFingerprint()); - addFingerprint(fileInputs, fp); - addFingerprint(directoryInputs, fp); - return fp; - } - - private static void addFingerprint( - ImmutableList> list, - BigIntegerFingerprint fingerprint) { - list.forEach( - pair -> { - fingerprint.addPath(pair.first.getExecPath()); - fingerprint.addBigIntegerOrdered(pair.second.getValueFingerprint()); - }); - } - - @Nullable - @Override - public BigInteger getValueFingerprint() { - return getFingerprintBuilder().getFingerprint(); - } - @SuppressWarnings("EqualsGetClass") // RunfilesArtifactValue not equal to Aggregating. @Override public boolean equals(Object o) { 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 c11a7484203646..cb3cc28964f44e 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 @@ -74,11 +74,9 @@ import com.google.devtools.build.skyframe.SkyKey; import com.google.devtools.build.skyframe.SkyValue; import com.google.devtools.build.skyframe.ValueOrException; -import java.math.BigInteger; import java.util.ArrayList; import java.util.HashMap; import java.util.Map; -import java.util.function.Supplier; import javax.annotation.Nullable; /** @@ -111,22 +109,18 @@ public final class AspectFunction implements SkyFunction { */ private final boolean storeTransitivePackagesForPackageRootResolution; - private final Supplier nonceVersion; - AspectFunction( BuildViewProvider buildViewProvider, RuleClassProvider ruleClassProvider, @Nullable SkylarkImportLookupFunction skylarkImportLookupFunctionForInlining, boolean storeTransitivePackagesForPackageRootResolution, - BuildOptions defaultBuildOptions, - Supplier nonceVersion) { + BuildOptions defaultBuildOptions) { this.buildViewProvider = buildViewProvider; this.ruleClassProvider = ruleClassProvider; this.skylarkImportLookupFunctionForInlining = skylarkImportLookupFunctionForInlining; this.storeTransitivePackagesForPackageRootResolution = storeTransitivePackagesForPackageRootResolution; this.defaultBuildOptions = defaultBuildOptions; - this.nonceVersion = nonceVersion; } /** @@ -616,8 +610,7 @@ private AspectValue createAliasAspect( originalTarget.getLabel(), originalTarget.getLocation(), ConfiguredAspect.forAlias(real.getConfiguredAspect()), - transitivePackagesForPackageRootResolution, - nonceVersion.get()); + transitivePackagesForPackageRootResolution); } @Nullable @@ -703,8 +696,7 @@ private AspectValue createAspect( configuredAspect, transitivePackagesForPackageRootResolution == null ? null - : transitivePackagesForPackageRootResolution.build(), - nonceVersion.get()); + : transitivePackagesForPackageRootResolution.build()); } @Override 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 288d7f5a6fc607..e3745e6c5a1a40 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 @@ -34,7 +34,6 @@ 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 java.math.BigInteger; import javax.annotation.Nullable; /** An aspect in the context of the Skyframe graph. */ @@ -440,9 +439,8 @@ public AspectValue( Label label, Location location, ConfiguredAspect configuredAspect, - NestedSet transitivePackagesForPackageRootResolution, - BigInteger nonceVersion) { - super(configuredAspect.getActions(), nonceVersion); + NestedSet transitivePackagesForPackageRootResolution) { + super(configuredAspect.getActions()); this.label = Preconditions.checkNotNull(label, actions); this.aspect = Preconditions.checkNotNull(aspect, label); this.location = Preconditions.checkNotNull(location, label); 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 e3acadb28ccdee..298e0021422b41 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 @@ -34,7 +34,7 @@ public class BuildInfoCollectionValue extends BasicActionLookupValue { private final BuildInfoCollection collection; BuildInfoCollectionValue(BuildInfoCollection collection, GeneratingActions generatingActions) { - super(generatingActions, /*nonceVersion=*/ null); + super(generatingActions); this.collection = collection; } 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 c66b6c145cfd61..d5a20778fc5af4 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 @@ -79,7 +79,6 @@ import com.google.devtools.build.skyframe.SkyKey; import com.google.devtools.build.skyframe.SkyValue; import com.google.devtools.build.skyframe.ValueOrException; -import java.math.BigInteger; import java.util.ArrayList; import java.util.Collection; import java.util.HashSet; @@ -90,7 +89,6 @@ import java.util.Set; import java.util.concurrent.Semaphore; import java.util.concurrent.TimeUnit; -import java.util.function.Supplier; import java.util.stream.Collectors; import javax.annotation.Nullable; @@ -139,7 +137,6 @@ public synchronized Exception getCause() { private final Semaphore cpuBoundSemaphore; private final BuildOptions defaultBuildOptions; @Nullable private final ConfiguredTargetProgressReceiver configuredTargetProgress; - private final Supplier nonceVersion; /** * Indicates whether the set of packages transitively loaded for a given {@link @@ -157,8 +154,7 @@ public synchronized Exception getCause() { boolean storeTransitivePackagesForPackageRootResolution, boolean shouldUnblockCpuWorkWhenFetchingDeps, BuildOptions defaultBuildOptions, - @Nullable ConfiguredTargetProgressReceiver configuredTargetProgress, - Supplier nonceVersion) { + @Nullable ConfiguredTargetProgressReceiver configuredTargetProgress) { this.buildViewProvider = buildViewProvider; this.ruleClassProvider = ruleClassProvider; this.cpuBoundSemaphore = cpuBoundSemaphore; @@ -167,7 +163,6 @@ public synchronized Exception getCause() { this.shouldUnblockCpuWorkWhenFetchingDeps = shouldUnblockCpuWorkWhenFetchingDeps; this.defaultBuildOptions = defaultBuildOptions; this.configuredTargetProgress = configuredTargetProgress; - this.nonceVersion = nonceVersion; } private void acquireWithLogging(SkyKey key) throws InterruptedException { @@ -246,8 +241,7 @@ public SkyValue compute(SkyKey key, Environment env) throws ConfiguredTargetFunc GeneratingActions.EMPTY, transitivePackagesForPackageRootResolution == null ? null - : transitivePackagesForPackageRootResolution.build(), - nonceVersion.get()); + : transitivePackagesForPackageRootResolution.build()); } // This line is only needed for accurate error messaging. Say this target has a circular @@ -950,8 +944,7 @@ private ConfiguredTargetValue createConfiguredTarget( ruleConfiguredTarget, transitivePackagesForPackageRootResolution == null ? null - : transitivePackagesForPackageRootResolution.build(), - nonceVersion.get()); + : transitivePackagesForPackageRootResolution.build()); } else { GeneratingActions generatingActions; // Check for conflicting actions within this configured target (that indicates a bug in the @@ -971,8 +964,7 @@ private ConfiguredTargetValue createConfiguredTarget( generatingActions, transitivePackagesForPackageRootResolution == null ? null - : transitivePackagesForPackageRootResolution.build(), - nonceVersion.get()); + : transitivePackagesForPackageRootResolution.build()); } } 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 1b78692bc86852..29726648dff45c 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 @@ -28,7 +28,7 @@ public class CoverageReportValue extends BasicActionLookupValue { @AutoCodec public static final CoverageReportKey COVERAGE_REPORT_KEY = new CoverageReportKey(); CoverageReportValue(GeneratingActions generatingActions) { - super(generatingActions, /*nonceVersion=*/ null); + super(generatingActions); } static class CoverageReportKey extends ActionLookupKey { diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/NonRuleConfiguredTargetValue.java b/src/main/java/com/google/devtools/build/lib/skyframe/NonRuleConfiguredTargetValue.java index 46151e56a73d82..b7a14e383d77b5 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/NonRuleConfiguredTargetValue.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/NonRuleConfiguredTargetValue.java @@ -28,7 +28,6 @@ import com.google.devtools.build.lib.packages.Package; import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec; import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec.VisibleForSerialization; -import java.math.BigInteger; import javax.annotation.Nullable; /** A non-rule configured target in the context of a Skyframe graph. */ @@ -52,7 +51,7 @@ public final class NonRuleConfiguredTargetValue extends BasicActionLookupValue NonRuleConfiguredTargetValue( ImmutableList actions, ConfiguredTarget configuredTarget) { - super(actions, /*nonceVersion=*/ null); + super(actions); this.configuredTarget = configuredTarget; // Transitive packages are not serialized. this.transitivePackagesForPackageRootResolution = null; @@ -61,9 +60,8 @@ public final class NonRuleConfiguredTargetValue extends BasicActionLookupValue NonRuleConfiguredTargetValue( ConfiguredTarget configuredTarget, GeneratingActions generatingActions, - @Nullable NestedSet transitivePackagesForPackageRootResolution, - @Nullable BigInteger nonceVersion) { - super(generatingActions, nonceVersion); + @Nullable NestedSet transitivePackagesForPackageRootResolution) { + super(generatingActions); this.configuredTarget = Preconditions.checkNotNull(configuredTarget, generatingActions); this.transitivePackagesForPackageRootResolution = transitivePackagesForPackageRootResolution; } diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/PackageLookupValue.java b/src/main/java/com/google/devtools/build/lib/skyframe/PackageLookupValue.java index 7d24e17e84a1fc..b2dc34d4e50a58 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/PackageLookupValue.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/PackageLookupValue.java @@ -27,7 +27,6 @@ import com.google.devtools.build.skyframe.SkyFunctionName; import com.google.devtools.build.skyframe.SkyKey; import com.google.devtools.build.skyframe.SkyValue; -import java.math.BigInteger; import javax.annotation.Nullable; /** @@ -65,8 +64,7 @@ enum ErrorReason { REPOSITORY_NOT_FOUND } - protected PackageLookupValue() { - } + protected PackageLookupValue() {} public static PackageLookupValue success( RepositoryValue repository, Root root, BuildFileName buildFileName) { @@ -155,10 +153,11 @@ public SkyFunctionName functionName() { public static class SuccessfulPackageLookupValue extends PackageLookupValue { /** * The repository value the meaning of the path depends on (e.g., an external repository - * controlling a symbolic link the path goes trough). Can be {@code null}, if does not depend - * on such a repository; will always be {@code null} for packages in the main repository. + * controlling a symbolic link the path goes trough). Can be {@code null}, if does not depend on + * such a repository; will always be {@code null} for packages in the main repository. */ @Nullable private final RepositoryValue repository; + private final Root root; private final BuildFileName buildFileName; @@ -199,18 +198,6 @@ public String getErrorMsg() { throw new IllegalStateException(); } - @Nullable - @Override - public BigInteger getValueFingerprint() { - if (repository != null) { - return null; - } - if (buildFileName != BuildFileName.BUILD) { - return null; - } - return root.getFingerprint(); - } - @Override public boolean equals(Object obj) { if (!(obj instanceof SuccessfulPackageLookupValue)) { @@ -248,7 +235,6 @@ public BuildFileName getBuildFileName() { /** Marker value for no build file found. */ public static class NoBuildFilePackageLookupValue extends UnsuccessfulPackageLookupValue { - private static final BigInteger FINGERPRINT = new BigInteger("14769240659748016902"); private NoBuildFilePackageLookupValue() {} @@ -257,12 +243,6 @@ ErrorReason getErrorReason() { return ErrorReason.NO_BUILD_FILE; } - @Nullable - @Override - public BigInteger getValueFingerprint() { - return FINGERPRINT; - } - @Override public String getErrorMsg() { return "BUILD file not found on package path"; @@ -372,8 +352,7 @@ public String toString() { /** Marker value for a deleted package. */ public static class DeletedPackageLookupValue extends UnsuccessfulPackageLookupValue { - private DeletedPackageLookupValue() { - } + private DeletedPackageLookupValue() {} @Override ErrorReason getErrorReason() { diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/RuleConfiguredTargetValue.java b/src/main/java/com/google/devtools/build/lib/skyframe/RuleConfiguredTargetValue.java index 1542b9ebd30103..4c9bdf7aec6b26 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/RuleConfiguredTargetValue.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/RuleConfiguredTargetValue.java @@ -25,7 +25,6 @@ import com.google.devtools.build.lib.concurrent.ThreadSafety.ThreadSafe; import com.google.devtools.build.lib.packages.Package; import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec; -import java.math.BigInteger; import javax.annotation.Nullable; /** A configured target in the context of a Skyframe graph. */ @@ -46,17 +45,12 @@ public final class RuleConfiguredTargetValue extends ActionLookupValue // Transitive packages are not serialized. @AutoCodec.Instantiator RuleConfiguredTargetValue(RuleConfiguredTarget configuredTarget) { - this( - configuredTarget, - /*transitivePackagesForPackageRootResolution=*/ null, - /*nonceVersion=*/ null); + this(configuredTarget, /*transitivePackagesForPackageRootResolution=*/ null); } RuleConfiguredTargetValue( RuleConfiguredTarget configuredTarget, - @Nullable NestedSet transitivePackagesForPackageRootResolution, - @Nullable BigInteger nonceVersion) { - super(nonceVersion); + @Nullable NestedSet transitivePackagesForPackageRootResolution) { this.configuredTarget = Preconditions.checkNotNull(configuredTarget); this.transitivePackagesForPackageRootResolution = transitivePackagesForPackageRootResolution; // These are specifically *not* copied to save memory. diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/RunfilesArtifactValue.java b/src/main/java/com/google/devtools/build/lib/skyframe/RunfilesArtifactValue.java index 8ffa191461d279..ebb4acf87c0868 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/RunfilesArtifactValue.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/RunfilesArtifactValue.java @@ -17,8 +17,6 @@ import com.google.devtools.build.lib.actions.Artifact; import com.google.devtools.build.lib.actions.FileArtifactValue; import com.google.devtools.build.lib.util.Pair; -import java.math.BigInteger; -import javax.annotation.Nullable; /** The artifacts behind a runfiles middleman. */ class RunfilesArtifactValue extends AggregatingArtifactValue { @@ -28,10 +26,4 @@ class RunfilesArtifactValue extends AggregatingArtifactValue { FileArtifactValue selfData) { super(fileInputs, directoryInputs, selfData); } - - @Nullable - @Override - public BigInteger getValueFingerprint() { - return getFingerprintBuilder().addBoolean(Boolean.TRUE).getFingerprint(); - } } 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 40fc2ed9fa2e11..6fb0998d38a297 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 @@ -189,7 +189,6 @@ import com.google.errorprone.annotations.ForOverride; import java.io.IOException; import java.io.PrintStream; -import java.math.BigInteger; import java.util.ArrayList; import java.util.Collection; import java.util.Collections; @@ -331,7 +330,6 @@ public abstract class SkyframeExecutor implements WalkableGraphFactory { private final PathResolverFactory pathResolverFactory = new PathResolverFactoryImpl(); @Nullable private final NonexistentFileReceiver nonexistentFileReceiver; - private final MutableSupplier nonceVersion = new MutableSupplier<>(); private final TrimmedConfigurationCache trimmingCache = TrimmedConfigurationProgressReceiver.buildCache(); @@ -544,8 +542,7 @@ private ImmutableMap skyFunctions(PackageFactory p shouldStoreTransitivePackagesInLoadingAndAnalysis(), shouldUnblockCpuWorkWhenFetchingDeps, defaultBuildOptions, - configuredTargetProgress, - nonceVersion)); + configuredTargetProgress)); map.put( SkyFunctions.ASPECT, new AspectFunction( @@ -553,8 +550,7 @@ private ImmutableMap skyFunctions(PackageFactory p ruleClassProvider, skylarkImportLookupFunctionForInlining, shouldStoreTransitivePackagesInLoadingAndAnalysis(), - defaultBuildOptions, - nonceVersion)); + defaultBuildOptions)); map.put( SkyFunctions.LOAD_SKYLARK_ASPECT, new ToplevelSkylarkAspectFunction(skylarkImportLookupFunctionForInlining)); @@ -670,11 +666,6 @@ protected final PerBuildSyscallCache getPerBuildSyscallCache(int concurrencyLeve return perBuildSyscallCache; } - /** Do not use except in subclasses. */ - protected void setNonceVersion(BigInteger nonceVersion) { - this.nonceVersion.set(nonceVersion); - } - @ThreadCompatible public void setActive(boolean active) { this.active = active; diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/TreeArtifactValue.java b/src/main/java/com/google/devtools/build/lib/skyframe/TreeArtifactValue.java index 4336a0b70e6c7a..5b7490650ec442 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/TreeArtifactValue.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/TreeArtifactValue.java @@ -25,7 +25,6 @@ import com.google.devtools.build.lib.actions.HasDigest; import com.google.devtools.build.lib.actions.cache.DigestUtils; import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec; -import com.google.devtools.build.lib.util.BigIntegerFingerprint; import com.google.devtools.build.lib.vfs.Dirent; import com.google.devtools.build.lib.vfs.Dirent.Type; import com.google.devtools.build.lib.vfs.Path; @@ -33,7 +32,6 @@ import com.google.devtools.build.lib.vfs.Symlinks; import com.google.devtools.build.skyframe.SkyValue; import java.io.IOException; -import java.math.BigInteger; import java.util.Arrays; import java.util.Collection; import java.util.Map; @@ -55,7 +53,6 @@ public class TreeArtifactValue implements HasDigest, SkyValue { private final byte[] digest; private final ImmutableSortedMap childData; - private BigInteger valueFingerprint; private final boolean remote; @AutoCodec.VisibleForSerialization @@ -125,16 +122,6 @@ public boolean isRemote() { return remote; } - @Override - public BigInteger getValueFingerprint() { - if (valueFingerprint == null) { - BigIntegerFingerprint fp = new BigIntegerFingerprint(); - fp.addDigestedBytes(digest); - valueFingerprint = fp.getFingerprint(); - } - return valueFingerprint; - } - @Override public int hashCode() { return Arrays.hashCode(digest); 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 da22b10bdd0004..bb379d742c6094 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 @@ -38,9 +38,7 @@ public class WorkspaceStatusValue extends BasicActionLookupValue { Artifact stableArtifact, Artifact volatileArtifact, WorkspaceStatusAction workspaceStatusAction) { - super( - Actions.GeneratingActions.fromSingleAction(workspaceStatusAction, BUILD_INFO_KEY), - /*nonceVersion=*/ null); + super(Actions.GeneratingActions.fromSingleAction(workspaceStatusAction, BUILD_INFO_KEY)); this.stableArtifact = stableArtifact; this.volatileArtifact = volatileArtifact; } diff --git a/src/main/java/com/google/devtools/build/skyframe/MinimalVersion.java b/src/main/java/com/google/devtools/build/skyframe/MinimalVersion.java index 5dbcb3ee2af60e..fef53b032871dd 100644 --- a/src/main/java/com/google/devtools/build/skyframe/MinimalVersion.java +++ b/src/main/java/com/google/devtools/build/skyframe/MinimalVersion.java @@ -13,27 +13,17 @@ // limitations under the License. package com.google.devtools.build.skyframe; -import java.math.BigInteger; - /** - * A Version "less than" all other versions, other than itself. Only used as initializer in - * node entry. + * A Version "less than" all other versions, other than itself. Only used as initializer in node + * entry. */ -class MinimalVersion implements Version { +final class MinimalVersion implements Version { static final MinimalVersion INSTANCE = new MinimalVersion(); - // Randomly generated. - private static final BigInteger FINGERPRINT = BigInteger.valueOf(1184854353641063846L); - private MinimalVersion() { - } + private MinimalVersion() {} @Override public boolean atMost(Version other) { return true; } - - @Override - public BigInteger getFingerprint() { - return FINGERPRINT; - } } diff --git a/src/main/java/com/google/devtools/build/skyframe/SkyValue.java b/src/main/java/com/google/devtools/build/skyframe/SkyValue.java index efbabda50fbea5..86f707eca3e3b5 100644 --- a/src/main/java/com/google/devtools/build/skyframe/SkyValue.java +++ b/src/main/java/com/google/devtools/build/skyframe/SkyValue.java @@ -14,22 +14,11 @@ package com.google.devtools.build.skyframe; import java.io.Serializable; -import java.math.BigInteger; -import javax.annotation.Nullable; /** * A return value of a {@code SkyFunction}. */ public interface SkyValue extends Serializable { - /** - * If non-null, a fingerprint for this value such that two values are equal iff they have the same - * fingerprint. For use during dep-fingerprint-based change pruning: see {@link - * NodeEntry#canPruneDepsByFingerprint}. - */ - @Nullable - default BigInteger getValueFingerprint() { - return null; - } /** * Returns true for values that may compare objects that must be compared using reference diff --git a/src/main/java/com/google/devtools/build/skyframe/Version.java b/src/main/java/com/google/devtools/build/skyframe/Version.java index e0a89b38ac281f..2922f3ef7d0dd4 100644 --- a/src/main/java/com/google/devtools/build/skyframe/Version.java +++ b/src/main/java/com/google/devtools/build/skyframe/Version.java @@ -13,9 +13,6 @@ // limitations under the License. package com.google.devtools.build.skyframe; -import java.math.BigInteger; -import javax.annotation.Nullable; - /** * A Version defines a value in a version tree used in persistent data structures. * See http://en.wikipedia.org/wiki/Persistent_data_structure. @@ -44,9 +41,4 @@ public interface Version { default boolean lowerThan(Version other) { return atMost(other) && !equals(other); } - - @Nullable - default BigInteger getFingerprint() { - return null; - } } 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 91100ab6bb7545..30e0a8efd10cd2 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 @@ -213,8 +213,7 @@ private static ConfiguredTargetValue createConfiguredTargetValue( return new NonRuleConfiguredTargetValue( Mockito.mock(ConfiguredTarget.class), Actions.GeneratingActions.fromSingleAction(actionTemplate, CTKEY), - NestedSetBuilder.stableOrder().build(), - /*nonceVersion=*/ null); + NestedSetBuilder.stableOrder().build()); } private SpecialArtifact createTreeArtifact(String path) { 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 ef150b366bde1a..b96d5c1ed17102 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 @@ -287,15 +287,7 @@ public void actionExecutionValueSerialization() throws Exception { ImmutableList.of(filesetOutputSymlink), null, true); - ActionExecutionValue valueWithFingerprint = - ActionExecutionValue.create( - ImmutableMap.of(artifact1, metadata1, artifact3, FileArtifactValue.DEFAULT_MIDDLEMAN), - ImmutableMap.of(treeArtifact, treeArtifactValue), - ImmutableList.of(filesetOutputSymlink), - null, - true); - valueWithFingerprint.getValueFingerprint(); - new SerializationTester(actionExecutionValue, valueWithFingerprint) + new SerializationTester(actionExecutionValue) .addDependency(FileSystem.class, root.getFileSystem()) .runTests(); } @@ -395,8 +387,7 @@ private void setGeneratingActions() throws InterruptedException, ActionConflictE actionKeyContext, ImmutableList.copyOf(actions), ALL_OWNER, - /*outputFiles=*/ null), - /*nonceVersion=*/ null))); + /*outputFiles=*/ null)))); } } diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/SingleToolchainResolutionFunctionTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/SingleToolchainResolutionFunctionTest.java index 5fd90bd20ea3e7..933956aad65a9b 100644 --- a/src/test/java/com/google/devtools/build/lib/skyframe/SingleToolchainResolutionFunctionTest.java +++ b/src/test/java/com/google/devtools/build/lib/skyframe/SingleToolchainResolutionFunctionTest.java @@ -69,10 +69,7 @@ public void setUpKeys() { private static ConfiguredTargetValue createConfiguredTargetValue( ConfiguredTarget configuredTarget) { return new NonRuleConfiguredTargetValue( - configuredTarget, - GeneratingActions.EMPTY, - NestedSetBuilder.emptySet(Order.STABLE_ORDER), - /*nonceVersion=*/ null); + configuredTarget, GeneratingActions.EMPTY, NestedSetBuilder.emptySet(Order.STABLE_ORDER)); } private EvaluationResult invokeToolchainResolution(SkyKey key) 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 5ca63259283d98..89f548d600dfc6 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 @@ -289,8 +289,7 @@ private void setGeneratingActions() throws ActionConflictException { actionKeyContext, ImmutableList.copyOf(actions), ACTION_LOOKUP_KEY, - /*outputFiles=*/ null), - /*nonceVersion=*/ null))); + /*outputFiles=*/ null)))); } } 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 fcf94f5566b356..df33768478cb25 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 @@ -260,8 +260,7 @@ private void setGeneratingActions() throws InterruptedException, ActionConflictE actionKeyContext, ImmutableList.copyOf(actions), ALL_OWNER, - /*outputFiles=*/ null), - /*nonceVersion=*/ null))); + /*outputFiles=*/ null)))); } }