From 5b52f018b193be02fd11de9bc89c9f9d49bf5b3f Mon Sep 17 00:00:00 2001 From: Googler Date: Thu, 7 Dec 2023 03:28:21 -0800 Subject: [PATCH] [GOOGLE][Skymeld] Make orphaned artifact detection available for Skymeld. Internal change. PiperOrigin-RevId: 588730288 Change-Id: Ib9e13e4271e2ec555d14aa743acca33c1b66b814 --- .../build/lib/buildtool/ExecutionTool.java | 4 +- .../nestedset/ArtifactNestedSetKey.java | 23 ++- .../build/lib/collect/nestedset/BUILD | 4 +- .../lib/skyframe/ActionExecutionFunction.java | 77 +++++++++- .../skyframe/ArtifactNestedSetFunction.java | 16 +- .../google/devtools/build/lib/skyframe/BUILD | 15 ++ .../skyframe/ConsumedArtifactsTracker.java | 138 ++++++++++++++++++ .../skyframe/SequencedSkyframeExecutor.java | 15 -- .../build/lib/skyframe/SkyframeExecutor.java | 23 ++- .../SequencedSkyframeExecutorTest.java | 17 +-- .../skyframe/TimestampBuilderTestCase.java | 3 +- .../rewinding/RewindingTestsHelper.java | 4 +- 12 files changed, 294 insertions(+), 45 deletions(-) create mode 100644 src/main/java/com/google/devtools/build/lib/skyframe/ConsumedArtifactsTracker.java 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 65365afe785978..2365251d0df467 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 @@ -140,6 +140,7 @@ public class ExecutionTool { private BlazeExecutor executor; private final ActionInputPrefetcher prefetcher; private final ImmutableSet executorLifecycleListeners; + private final SpawnStrategyRegistry spawnStrategyRegistry; private final ModuleActionContextRegistry actionContextRegistry; @@ -349,7 +350,8 @@ public void prepareForExecution(Stopwatch executionTimer) for (ExecutorLifecycleListener executorLifecycleListener : executorLifecycleListeners) { try (SilentCloseable c = Profiler.instance().profile(executorLifecycleListener + ".executionPhaseStarting")) { - executorLifecycleListener.executionPhaseStarting(null, () -> null, null); + executorLifecycleListener.executionPhaseStarting( + null, () -> null, skyframeExecutor.getEphemeralCheckIfOutputConsumed()); } } try (SilentCloseable c = Profiler.instance().profile("configureResourceManager")) { diff --git a/src/main/java/com/google/devtools/build/lib/collect/nestedset/ArtifactNestedSetKey.java b/src/main/java/com/google/devtools/build/lib/collect/nestedset/ArtifactNestedSetKey.java index f6f4b4e79ae43d..e0134d83603ac0 100644 --- a/src/main/java/com/google/devtools/build/lib/collect/nestedset/ArtifactNestedSetKey.java +++ b/src/main/java/com/google/devtools/build/lib/collect/nestedset/ArtifactNestedSetKey.java @@ -22,8 +22,6 @@ import com.google.devtools.build.skyframe.ExecutionPhaseSkyKey; import com.google.devtools.build.skyframe.SkyFunctionName; import com.google.devtools.build.skyframe.SkyKey; -import java.util.ArrayList; -import java.util.List; import java.util.Set; /** @@ -69,8 +67,8 @@ private ArtifactNestedSetKey(Object[] children) { * Returns a list of this key's direct dependencies, including {@link Artifact#key} for leaves and * {@link ArtifactNestedSetKey} for non-leaves. */ - public List getDirectDepKeys() { - List depKeys = new ArrayList<>(children.length); + public ImmutableList getDirectDepKeys() { + ImmutableList.Builder depKeys = ImmutableList.builderWithExpectedSize(children.length); for (Object child : children) { if (child instanceof Artifact) { depKeys.add(Artifact.key((Artifact) child)); @@ -78,7 +76,16 @@ public List getDirectDepKeys() { depKeys.add(createInternal((Object[]) child)); } } - return depKeys; + return depKeys.build(); + } + + /** Applies a consumer function to the direct artifacts of this nested set. */ + public void applyToDirectArtifacts(DirectArtifactConsumer function) throws InterruptedException { + for (Object child : children) { + if (child instanceof Artifact) { + function.accept((Artifact) child); + } + } } @Override @@ -175,4 +182,10 @@ public boolean equals(Object that) { public String toString() { return String.format("ArtifactNestedSetKey[%s]@%s", children.length, hashCode()); } + + /** A consumer to be applied to each direct artifact. */ + @FunctionalInterface + public interface DirectArtifactConsumer { + void accept(Artifact artifact) throws InterruptedException; + } } diff --git a/src/main/java/com/google/devtools/build/lib/collect/nestedset/BUILD b/src/main/java/com/google/devtools/build/lib/collect/nestedset/BUILD index c1402b68d00cb5..a4aebd84d1eb53 100644 --- a/src/main/java/com/google/devtools/build/lib/collect/nestedset/BUILD +++ b/src/main/java/com/google/devtools/build/lib/collect/nestedset/BUILD @@ -52,7 +52,9 @@ java_library( java_library( name = "artifact_nested_set_key", - srcs = ["ArtifactNestedSetKey.java"], + srcs = [ + "ArtifactNestedSetKey.java", + ], deps = [ ":nestedset", "//src/main/java/com/google/devtools/build/lib/actions:artifacts", diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ActionExecutionFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/ActionExecutionFunction.java index 5e9baf69864eb7..7cb8d15d189f06 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/ActionExecutionFunction.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/ActionExecutionFunction.java @@ -159,6 +159,7 @@ public final class ActionExecutionFunction implements SkyFunction { private final BlazeDirectories directories; private final Supplier tsgm; private final BugReporter bugReporter; + private final Supplier consumedArtifactsTrackerSupplier; // TODO(b/314282963): Remove this after the rollout. private final Supplier clearNestedSetAfterActionExecution; @@ -170,6 +171,7 @@ public ActionExecutionFunction( BlazeDirectories directories, Supplier tsgm, BugReporter bugReporter, + Supplier consumedArtifactsTrackerSupplier, Supplier clearNestedSetAfterActionExecution) { this.actionRewindStrategy = checkNotNull(actionRewindStrategy); this.skyframeActionExecutor = checkNotNull(skyframeActionExecutor); @@ -178,6 +180,7 @@ public ActionExecutionFunction( this.tsgm = checkNotNull(tsgm); this.bugReporter = checkNotNull(bugReporter); this.clearNestedSetAfterActionExecution = clearNestedSetAfterActionExecution; + this.consumedArtifactsTrackerSupplier = consumedArtifactsTrackerSupplier; } @Override @@ -291,7 +294,13 @@ private SkyValue computeInternal(SkyKey skyKey, Environment env) if (!state.hasArtifactData()) { Iterable depKeys = - getInputDepKeys(allInputs, action.getSchedulingDependencies(), state); + getInputDepKeys( + action, + consumedArtifactsTrackerSupplier.get(), + allInputs, + action.getSchedulingDependencies(), + state); + SkyframeLookupResult inputDeps = env.getValuesAndExceptions(depKeys); if (previousExecution == null) { // Do we actually need to find our metadata? @@ -363,7 +372,12 @@ private SkyValue computeInternal(SkyKey skyKey, Environment env) actionStartTime, env, allInputs, - getInputDepKeys(allInputs, action.getSchedulingDependencies(), state), + getInputDepKeys( + action, + /* consumedArtifactsTracker= */ null, + allInputs, + action.getSchedulingDependencies(), + state), state); } catch (ActionExecutionException e) { // In this case we do not report the error to the action reporter because we have already @@ -385,13 +399,67 @@ private SkyValue computeInternal(SkyKey skyKey, Environment env) action.getInputs().clearMemo(); allInputs.clearMemo(); } + // After the action execution is finalized, unregister the outputs from the consumed set to save + // memory. + // Note: This can theoretically lead to infinite action rewinding if we're unlucky enough. + // Consider an action foo whose outputs A and B are needed by 2 separate actions consumerA and + // consumerB. If these 2 actions trigger rewinding alternately, at the correct timing, e.g.: + // 1. consumerA requests for A. A is registered. foo produces only A since B isn't registered. A + // is de-registered. consumerA isn't executed yet. + // 2. consumerB requests for B. B is registered. foo is rewound and produces only B since A + // isn't registered. B is de-registered. consumerB isn't executed yet. + // 3. Before consumerA enters execution, A falls out of the CAS. consumerA sees that A is + // missing and triggers rewinding for A. Repeat step (1). + // 4. Before consumerB enters execution, B falls out of the CAS. consumerB sees that B is + // missing and triggers rewinding for B. Repeat step (2). + if (consumedArtifactsTrackerSupplier.get() != null) { + consumedArtifactsTrackerSupplier + .get() + .unregisterOutputsAfterExecutionDone(action.getOutputs()); + } + return result; } private static Iterable getInputDepKeys( + Action action, + ConsumedArtifactsTracker consumedArtifactsTracker, NestedSet allInputs, NestedSet schedulingDependencies, - InputDiscoveryState state) { + InputDiscoveryState state) + throws InterruptedException { + + // Register the action's inputs and scheduling deps as "consumed" in the build. + // As a general rule, we do it before requesting for the evaluation of these artifacts. This + // would provide a good estimate of which outputs are consumed. + // + // The exception to this rule is middleman actions: it's possible that the output of a + // middleman action is only reachable via middleman actions in the action graph. In that case, + // we don't want to store any of the underlying artifacts, until we've discovered that there's + // a non-middleman action in its path. + if (!state.checkedForConsumedArtifactRegistration && consumedArtifactsTracker != null) { + // Special case: middleman actions. + // Delay registering the artifacts under this middleman action until we know that the + // middleman artifact itself is consumed by other non-middleman actions. + if (action.getActionType().isMiddleman()) { + consumedArtifactsTracker.skipRegisteringArtifactsUnderMiddleman(action.getPrimaryOutput()); + // Skip the ArtifactNestedSetFunction altogether for this special case. Any artifact + // requested via ArtifactNestedSetFunction would be registered as consumed. + return Iterables.concat( + Artifact.keys(allInputs.toList()), Artifact.keys(schedulingDependencies.toList())); + } else { + // Only registering the leaves here, since the Artifacts under non-leaves will be registered + // in ArtifactNestedSetFunction. Similarly for the non-singleton Scheduling Dependencies. + for (Artifact input : allInputs.getLeaves()) { + consumedArtifactsTracker.registerConsumedArtifact(input); + } + if (schedulingDependencies.isSingleton()) { + consumedArtifactsTracker.registerConsumedArtifact(schedulingDependencies.getSingleton()); + } + } + state.checkedForConsumedArtifactRegistration = true; + } + // We "unwrap" the NestedSet and evaluate the first layer of direct Artifacts here in order to // save memory: // - This top layer costs 1 extra ArtifactNestedSetKey node. @@ -399,6 +467,7 @@ private static Iterable getInputDepKeys( // => the top layer offers little in terms of reusability. // More details: b/143205147. FluentIterable result = FluentIterable.from(Artifact.keys(allInputs.getLeaves())); + if (schedulingDependencies.isSingleton()) { result = result.append(Artifact.key(schedulingDependencies.getSingleton())); } @@ -1486,6 +1555,8 @@ static class InputDiscoveryState implements SkyKeyComputeState { boolean preparedInputDiscovery = false; boolean actionInputCollectedEventSent = false; + boolean checkedForConsumedArtifactRegistration = false; + /** * Stores the ArtifactNestedSetKeys created from the inputs of this actions. Objective: avoid * creating a new ArtifactNestedSetKey for the same NestedSet each time we run diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ArtifactNestedSetFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/ArtifactNestedSetFunction.java index c68452d62e2a36..66f3b3f341263e 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/ArtifactNestedSetFunction.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/ArtifactNestedSetFunction.java @@ -13,6 +13,7 @@ // limitations under the License. package com.google.devtools.build.lib.skyframe; +import com.google.common.collect.ImmutableList; import com.google.devtools.build.lib.actions.ActionExecutionException; import com.google.devtools.build.lib.collect.nestedset.ArtifactNestedSetKey; import com.google.devtools.build.lib.collect.nestedset.NestedSet; @@ -25,7 +26,7 @@ import com.google.devtools.build.skyframe.SkyKey; import com.google.devtools.build.skyframe.SkyValue; import com.google.devtools.build.skyframe.SkyframeLookupResult; -import java.util.List; +import java.util.function.Supplier; import javax.annotation.Nullable; /** @@ -48,11 +49,22 @@ */ final class ArtifactNestedSetFunction implements SkyFunction { + private final Supplier consumedArtifactsTrackerSupplier; + + ArtifactNestedSetFunction(Supplier consumedArtifactsTrackerSupplier) { + this.consumedArtifactsTrackerSupplier = consumedArtifactsTrackerSupplier; + } + @Override @Nullable public SkyValue compute(SkyKey skyKey, Environment env) throws InterruptedException, ArtifactNestedSetFunctionException { - List depKeys = ((ArtifactNestedSetKey) skyKey).getDirectDepKeys(); + ArtifactNestedSetKey artifactNestedSetKey = (ArtifactNestedSetKey) skyKey; + if (consumedArtifactsTrackerSupplier.get() != null) { + artifactNestedSetKey.applyToDirectArtifacts( + (x) -> consumedArtifactsTrackerSupplier.get().registerConsumedArtifact(x)); + } + ImmutableList depKeys = artifactNestedSetKey.getDirectDepKeys(); SkyframeLookupResult depsEvalResult = env.getValuesAndExceptions(depKeys); NestedSetBuilder> transitiveExceptionsBuilder = diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/BUILD b/src/main/java/com/google/devtools/build/lib/skyframe/BUILD index f50a7ee50aa525..8f84e48f62daaf 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/BUILD +++ b/src/main/java/com/google/devtools/build/lib/skyframe/BUILD @@ -115,6 +115,7 @@ java_library( ":configured_target_key", ":configured_target_progress_receiver", ":configured_value_creation_exception", + ":consumed_artifacts_tracker", ":containing_package_lookup_function", ":containing_package_lookup_value", ":coverage_report_value", @@ -124,6 +125,7 @@ java_library( ":diff_awareness_manager", ":directory_listing_function", ":directory_listing_state_value", + ":ephemeral_check_if_output_consumed", ":exclusive_test_build_driver_value", ":execution_finished_event", ":file_function", @@ -700,6 +702,7 @@ java_library( deps = [ ":artifact_function", ":artifact_nested_set_value", + ":consumed_artifacts_tracker", "//src/main/java/com/google/devtools/build/lib/actions", "//src/main/java/com/google/devtools/build/lib/collect/nestedset", "//src/main/java/com/google/devtools/build/lib/collect/nestedset:artifact_nested_set_key", @@ -1108,6 +1111,18 @@ java_library( ], ) +java_library( + name = "consumed_artifacts_tracker", + srcs = ["ConsumedArtifactsTracker.java"], + deps = [ + ":ephemeral_check_if_output_consumed", + "//src/main/java/com/google/devtools/build/lib/actions", + "//src/main/java/com/google/devtools/build/lib/actions:artifacts", + "//src/main/java/com/google/devtools/build/skyframe", + "//third_party:guava", + ], +) + java_library( name = "containing_package_lookup_function", srcs = ["ContainingPackageLookupFunction.java"], diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ConsumedArtifactsTracker.java b/src/main/java/com/google/devtools/build/lib/skyframe/ConsumedArtifactsTracker.java new file mode 100644 index 00000000000000..3579ea01080d46 --- /dev/null +++ b/src/main/java/com/google/devtools/build/lib/skyframe/ConsumedArtifactsTracker.java @@ -0,0 +1,138 @@ +// Copyright 2023 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.Preconditions; +import com.google.devtools.build.lib.actions.Action; +import com.google.devtools.build.lib.actions.ActionLookupValue; +import com.google.devtools.build.lib.actions.Artifact; +import com.google.devtools.build.lib.actions.Artifact.DerivedArtifact; +import com.google.devtools.build.skyframe.MemoizingEvaluator; +import java.util.Collection; +import java.util.HashSet; +import java.util.Set; +import java.util.concurrent.ConcurrentHashMap; +import java.util.function.Supplier; + +/** Tracks whether an artifact was "consumed" by any action in the build. */ +public final class ConsumedArtifactsTracker implements EphemeralCheckIfOutputConsumed { + + private final Set consumed = ConcurrentHashMap.newKeySet(524288); + private final Set middlemanArtifactSkippedRegistering = + ConcurrentHashMap.newKeySet(2048); + private final Set middlemanArtifactBackFilled = ConcurrentHashMap.newKeySet(2048); + + private final Supplier evaluatorSupplier; + + public ConsumedArtifactsTracker(Supplier evaluatorSupplier) { + this.evaluatorSupplier = evaluatorSupplier; + } + + /** + * This method guarantees the best estimate before the execution of the action that would generate + * this artifact. The return value is undefined afterwards. + */ + @Override + public boolean test(Artifact artifact) { + return consumed.contains(artifact); + } + + void unregisterOutputsAfterExecutionDone(Collection outputs) { + consumed.removeAll(outputs); + } + + /** + * Register the provided artifact as "consumed". + * + *

If the provided artifact is a middleman artifact, expand it and register the underlying + * artifacts. + */ + void registerConsumedArtifact(Artifact artifact) throws InterruptedException { + if (artifact.isMiddlemanArtifact() + && wasRegistrationSkippedForArtifactsUnderMiddleman(artifact)) { + // Special case: this means the action that generates this middleman was already evaluated as + // a top-level middleman action and the registration of its underlying artifacts were skipped. + // We therefore need to do it again here. + backfillArtifactsUnderMiddleman((DerivedArtifact) artifact); + return; + } + storeConsumedStatusIfRequired(artifact); + } + + private void storeConsumedStatusIfRequired(Artifact artifact) { + if (shouldStoreConsumedStatus(artifact)) { + consumed.add(artifact); + } + } + + void skipRegisteringArtifactsUnderMiddleman(Artifact middlemanArtifact) { + middlemanArtifactSkippedRegistering.add(middlemanArtifact); + } + + boolean wasRegistrationSkippedForArtifactsUnderMiddleman(Artifact middlemanArtifact) { + return middlemanArtifactSkippedRegistering.contains(middlemanArtifact); + } + + // TODO(b/304440811) Remove this after we removed the concept of middleman. + private void backfillArtifactsUnderMiddleman(DerivedArtifact middlemanArtifact) + throws InterruptedException { + Set toBackfill = new HashSet<>(); + + recursivelyCollectArtifactsToBackfill(middlemanArtifact, toBackfill); + + for (Artifact expandedInput : toBackfill) { + storeConsumedStatusIfRequired(expandedInput); + } + } + + // In case we're backfilling for a middleman artifact that includes other middleman artifacts. + private void recursivelyCollectArtifactsToBackfill( + DerivedArtifact middlemanArtifact, Set toBackfill) throws InterruptedException { + // We only need to do the backfilling once for each middleman artifact. + if (!middlemanArtifactBackFilled.add(middlemanArtifact)) { + return; + } + + var generatingActionKey = middlemanArtifact.getGeneratingActionKey(); + // Avoid establishing a skyframe dependency. + ActionLookupValue actionLookupValue = + (ActionLookupValue) + Preconditions.checkNotNull( + evaluatorSupplier + .get() + .getExistingEntryAtCurrentlyEvaluatingVersion( + generatingActionKey.getActionLookupKey())) + .getValue(); + Action middlemanAction = actionLookupValue.getAction(generatingActionKey.getActionIndex()); + + for (Artifact expandedInput : middlemanAction.getInputs().toList()) { + if (expandedInput.isMiddlemanArtifact()) { + recursivelyCollectArtifactsToBackfill((DerivedArtifact) expandedInput, toBackfill); + } else if (shouldStoreConsumedStatus(expandedInput)) { + toBackfill.add(expandedInput); + } + } + } + + /** + * Return whether the consumed status of an artifact should be recorded at all. + * + *

We should only store the consumed status of artifacts that will later on be checked for + * orphaned status directly. This is an optimization to keep the set smaller. + */ + private static boolean shouldStoreConsumedStatus(Artifact artifact) { + return !(artifact.isSourceArtifact() // Source artifacts won't be orphaned. + || artifact.hasParent()); // Will be checked through the parent artifact. + } +} 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 2ca83b93e68c5c..cc56cdabda896c 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 @@ -61,8 +61,6 @@ import com.google.devtools.build.lib.profiler.Profiler; import com.google.devtools.build.lib.profiler.ProfilerTask; import com.google.devtools.build.lib.repository.ExternalPackageHelper; -import com.google.devtools.build.lib.server.FailureDetails.ActionRewinding; -import com.google.devtools.build.lib.server.FailureDetails.FailureDetail; import com.google.devtools.build.lib.skyframe.AspectKeyCreator.AspectKey; import com.google.devtools.build.lib.skyframe.ExternalFilesHelper.ExternalFileAction; import com.google.devtools.build.lib.skyframe.PackageFunction.ActionOnIOExceptionReadingBuildFile; @@ -72,7 +70,6 @@ import com.google.devtools.build.lib.skyframe.config.BuildConfigurationKey; import com.google.devtools.build.lib.skyframe.rewinding.RewindableGraphInconsistencyReceiver; import com.google.devtools.build.lib.util.AbruptExitException; -import com.google.devtools.build.lib.util.DetailedExitCode; import com.google.devtools.build.lib.util.ResourceUsage; import com.google.devtools.build.lib.util.io.TimestampGranularityMonitor; import com.google.devtools.build.lib.vfs.BatchStat; @@ -317,18 +314,6 @@ private boolean rewindingEnabled(OptionsProvider options) throws AbruptExitExcep if (buildRequestOptions == null || !buildRequestOptions.rewindLostInputs) { return false; } - if (isMergedSkyframeAnalysisExecution()) { - throw new AbruptExitException( - DetailedExitCode.of( - FailureDetail.newBuilder() - .setMessage( - "--rewind_lost_inputs is not compatible with Skymeld" - + " (--experimental_merged_skyframe_analysis_execution)") - .setActionRewinding( - ActionRewinding.newBuilder() - .setCode(ActionRewinding.Code.REWIND_LOST_INPUTS_PREREQ_UNMET)) - .build())); - } return true; } 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 74e7613e13ca2d..bda2ee2a5ce3e4 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 @@ -417,6 +417,8 @@ public abstract class SkyframeExecutor implements WalkableGraphFactory { // Reset after each build. @Nullable private IncrementalArtifactConflictFinder incrementalArtifactConflictFinder; + + private ConsumedArtifactsTracker consumedArtifactsTracker; // end: Skymeld-only private RuleContextConstraintSemantics ruleContextConstraintSemantics; @@ -733,7 +735,9 @@ public SkyValue compute(SkyKey skyKey, Environment env) { map.put( SkyFunctions.PLATFORM_MAPPING, new PlatformMappingFunction(ruleClassProvider.getFragmentRegistry().getOptionsClasses())); - map.put(SkyFunctions.ARTIFACT_NESTED_SET, new ArtifactNestedSetFunction()); + map.put( + SkyFunctions.ARTIFACT_NESTED_SET, + new ArtifactNestedSetFunction(this::getConsumedArtifactsTracker)); BuildDriverFunction buildDriverFunction = new BuildDriverFunction( new TransitiveActionLookupValuesHelper() { @@ -774,6 +778,7 @@ protected SkyFunction newActionExecutionFunction() { directories, tsgm::get, bugReporter, + this::getConsumedArtifactsTracker, this::clearingNestedSetAfterActionExecution); } @@ -1475,6 +1480,15 @@ boolean isMergedSkyframeAnalysisExecution() { && mergedSkyframeAnalysisExecutionSupplier.get(); } + @Nullable + ConsumedArtifactsTracker getConsumedArtifactsTracker() { + return consumedArtifactsTracker; + } + + public void initializeConsumedArtifactsTracker() { + consumedArtifactsTracker = new ConsumedArtifactsTracker(this::getEvaluator); + } + /** Sets the eventBus to use for posting events. */ public void setEventBus(@Nullable EventBus eventBus) { this.eventBus.set(eventBus); @@ -2103,6 +2117,7 @@ public void clearExecutionStatesSkymeld(ExtendedEventHandler eventHandler) { cleanUpAfterSingleEvaluationWithActionExecution(eventHandler); statusReporterRef.get().unregisterFromEventBus(); setActionExecutionProgressReportingObjects(null, null, null); + consumedArtifactsTracker = null; } /** @@ -2212,6 +2227,12 @@ public IncrementalArtifactConflictFinder getIncrementalArtifactConflictFinder() return incrementalArtifactConflictFinder; } + /** Whether an artifact is consumed in this build. */ + @Nullable + public EphemeralCheckIfOutputConsumed getEphemeralCheckIfOutputConsumed() { + return consumedArtifactsTracker; + } + /** * Checks the action lookup values owning the given artifacts for action conflicts. Artifacts * satisfying the returned predicate are known to be transitively free from action conflicts. diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/SequencedSkyframeExecutorTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/SequencedSkyframeExecutorTest.java index 58ce21840a2580..1f1e0bc6c20c05 100644 --- a/src/test/java/com/google/devtools/build/lib/skyframe/SequencedSkyframeExecutorTest.java +++ b/src/test/java/com/google/devtools/build/lib/skyframe/SequencedSkyframeExecutorTest.java @@ -105,7 +105,6 @@ import com.google.devtools.build.lib.query2.common.QueryTransitivePackagePreloader; import com.google.devtools.build.lib.runtime.KeepGoingOption; import com.google.devtools.build.lib.runtime.QuiescingExecutorsImpl; -import com.google.devtools.build.lib.server.FailureDetails.ActionRewinding; import com.google.devtools.build.lib.server.FailureDetails.Crash; import com.google.devtools.build.lib.server.FailureDetails.FailureDetail; import com.google.devtools.build.lib.server.FailureDetails.Spawn; @@ -2659,14 +2658,10 @@ public NestedSet discoverInputs(ActionExecutionContext actionExecution } @Test - public void rewindingPrerequisites( - @TestParameter boolean trackIncrementalState, @TestParameter boolean skymeldEnabled) + public void rewindingPrerequisites(@TestParameter boolean trackIncrementalState) throws Exception { initializeSkyframeExecutor(); - options.parse( - "--rewind_lost_inputs", - "--experimental_merged_skyframe_analysis_execution=" + skymeldEnabled); - skyframeExecutor.setMergedSkyframeAnalysisExecutionSupplier(() -> skymeldEnabled); + options.parse("--rewind_lost_inputs"); skyframeExecutor.setActive(false); skyframeExecutor.decideKeepIncrementalState( @@ -2678,13 +2673,7 @@ public void rewindingPrerequisites( reporter); skyframeExecutor.setActive(true); - if (skymeldEnabled) { - AbruptExitException e = assertThrows(AbruptExitException.class, this::syncSkyframeExecutor); - assertThat(e.getDetailedExitCode().getFailureDetail().getActionRewinding().getCode()) - .isEqualTo(ActionRewinding.Code.REWIND_LOST_INPUTS_PREREQ_UNMET); - } else { - syncSkyframeExecutor(); // Permitted. - } + syncSkyframeExecutor(); // Permitted. } private void syncSkyframeExecutor() throws InterruptedException, AbruptExitException { 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 c831c6921bcd71..6dc530ce703ded 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 @@ -258,6 +258,7 @@ protected BuilderWithResult createBuilder( directories, () -> tsgm, BugReporter.defaultInstance(), + () -> null, () -> false)) .put( SkyFunctions.PACKAGE, @@ -297,7 +298,7 @@ protected BuilderWithResult createBuilder( .put( SkyFunctions.ACTION_TEMPLATE_EXPANSION, new DelegatingActionTemplateExpansionFunction()) - .put(SkyFunctions.ARTIFACT_NESTED_SET, new ArtifactNestedSetFunction()) + .put(SkyFunctions.ARTIFACT_NESTED_SET, new ArtifactNestedSetFunction(() -> null)) .buildOrThrow(), differencer, evaluationProgressReceiver, diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/rewinding/RewindingTestsHelper.java b/src/test/java/com/google/devtools/build/lib/skyframe/rewinding/RewindingTestsHelper.java index 0c09a7570340fd..6035eaa692faff 100644 --- a/src/test/java/com/google/devtools/build/lib/skyframe/rewinding/RewindingTestsHelper.java +++ b/src/test/java/com/google/devtools/build/lib/skyframe/rewinding/RewindingTestsHelper.java @@ -2406,11 +2406,11 @@ && isActionExecutionKey(context, fail)) { testCase.assertContainsError("Executing genrule //foo:fail failed"); } - private static boolean isActionExecutionKey(Object key, Label label) { + static boolean isActionExecutionKey(Object key, Label label) { return key instanceof ActionLookupData && label.equals(((ActionLookupData) key).getLabel()); } - private static void awaitUninterruptibly(CountDownLatch latch) { + static void awaitUninterruptibly(CountDownLatch latch) { assertThat( Uninterruptibles.awaitUninterruptibly(latch, TestUtils.WAIT_TIMEOUT_SECONDS, SECONDS)) .isTrue();