diff options
| -rw-r--r-- | src/main/java/com/google/devtools/build/lib/skyframe/ActionExecutionFunction.java | 39 | ||||
| -rw-r--r-- | src/main/java/com/google/devtools/build/lib/skyframe/SkyframeAwareAction.java | 73 |
2 files changed, 111 insertions, 1 deletions
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 ae58479d3d..538601a9c8 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 @@ -92,7 +92,8 @@ public class ActionExecutionFunction implements SkyFunction, CompletionReceiver // legacy when they are not at the top of the action graph. In legacy, they are stored // separately, so notifying non-dirty actions is cheap. In Skyframe, they depend on the // BUILD_ID, forcing invalidation of upward transitive closure on each build. - if (action.isVolatile() || action instanceof NotifyOnActionCacheHit) { + if ((action.isVolatile() && !(action instanceof SkyframeAwareAction)) + || action instanceof NotifyOnActionCacheHit) { // Volatile build actions may need to execute even if none of their known inputs have changed. // Depending on the buildID ensure that these actions have a chance to execute. PrecomputedValue.BUILD_ID.get(env); @@ -140,8 +141,20 @@ public class ActionExecutionFunction implements SkyFunction, CompletionReceiver stateMap.remove(action); throw new ActionExecutionFunctionException(e); } + if (env.valuesMissing()) { // There was missing artifact metadata in the graph. Wait for it to be present. + // We must check this and return here before attempting to establish any Skyframe dependencies + // of the action; see establishSkyframeDependencies why. + return null; + } + + try { + establishSkyframeDependencies(env, action); + } catch (ActionExecutionException e) { + throw new ActionExecutionFunctionException(e); + } + if (env.valuesMissing()) { return null; } @@ -434,6 +447,30 @@ public class ActionExecutionFunction implements SkyFunction, CompletionReceiver return result; } + private void establishSkyframeDependencies(Environment env, Action action) + throws ActionExecutionException { + // Before we may safely establish Skyframe dependencies, we must build all action inputs by + // requesting their ArtifactValues. + // This is very important to do, because the establishSkyframeDependencies method may request + // FileValues for input files of this action (directly requesting them, or requesting some other + // SkyValue whose builder requests FileValues), which may not yet exist if their generating + // actions have not yet run. + // See SkyframeAwareActionTest.testRaceConditionBetweenInputAcquisitionAndSkyframeDeps + Preconditions.checkState(!env.valuesMissing(), action); + + if (action instanceof SkyframeAwareAction) { + // Skyframe-aware actions should be executed unconditionally, i.e. bypass action cache + // checking. See documentation of SkyframeAwareAction. + Preconditions.checkState(action.executeUnconditionally(), action); + + try { + ((SkyframeAwareAction) action).establishSkyframeDependencies(env); + } catch (SkyframeAwareAction.ExceptionBase e) { + throw new ActionExecutionException(e, action, false); + } + } + } + private static Iterable<SkyKey> toKeys(Iterable<Artifact> inputs, Iterable<Artifact> mandatoryInputs) { if (mandatoryInputs == null) { diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeAwareAction.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeAwareAction.java new file mode 100644 index 0000000000..560240cfaf --- /dev/null +++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeAwareAction.java @@ -0,0 +1,73 @@ +// Copyright 2015 Google Inc. 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.devtools.build.skyframe.SkyFunction.Environment; + +/** + * Interface of an Action that is Skyframe-aware. + * + * <p><b>IMPORTANT</b>: actions that implement this interface should override + * {@code Action.executeUnconditionally} and return true. See below for details. + * + * <p>Implementors of this interface can request Skyframe dependencies to perform arbitrary + * computation or establish desired dependencies before they are executed but after their inputs + * have been built. + * + * <p>The {@link ActionExecutionFunction} will make sure that all requested SkyValues are built and + * that the {@link SkyframeAwareAction#establishSkyframeDependencies(Environment)} function + * completed successfully before attempting to execute the action. + * + * <p><b>It is crucial to correct action reexecution that implementors override + * {@code Action.executeUnconditionally} to always return true.</b> Skyframe tracks changes in both + * the input files and in dependencies established through + * {@link #establishSkyframeDependencies(Environment)}, but the action cache only knows about the + * input files. So if only the extra "skyframe dependencies" change, the action cache will believe + * the action to be up-to-date and skip actual execution. Therefore it's crucial to bypass action + * cache checking by marking the action as unconditionally executed. + */ +public interface SkyframeAwareAction { + + /** Wrapper and/or base class for exceptions raised in {@link #establishSkyframeDependencies}. */ + public static class ExceptionBase extends Exception { + public ExceptionBase(String message) { + super(message); + } + + public ExceptionBase(Throwable cause) { + super(cause); + } + } + + /** + * Establish dependencies on Skyframe values before executing the action. + * + * <p><b>IMPORTANT</b>: actions that implement this interface should override + * {@code Action.executeUnconditionally} and return true. See {@link SkyframeAwareAction} why. + * + * <p>This method should perform as little computation as possible: ideally it should request + * one or a few SkyValues, perhaps set some state somewhere and return. If this method needs to + * perform anything more complicated than that, including perhaps some non-trivial computation, + * you should implement that as a SkyFunction and request the corresponding SkyValue in this + * method. + * + * <p>Because the requested SkyValues may not yet be present in the graph, this method must be + * safe to call multiple times, and should always leave the action object in a consistent state. + * + * <p>This method should not attempt to handle errors or missing dependencies (other than wrapping + * exceptions); that is the responsibility of the caller. It should return as soon as possible, + * ready to be called again at a later time if need be. + */ + void establishSkyframeDependencies(Environment env) throws ExceptionBase; +} |