Actions can now request Skyframe dependencies and bypass action cache checking.

By implementing the SkyframeAwareAction interface an Action gains the power to
request Skyframe dependencies after its declared inputs have been built and
discovered but before execution. Such actions bypass the action cache check
and rely on Skyframe's dependency tracking to decide when to be reexecuted.

--
MOS_MIGRATED_REVID=97947856

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