Remove EventBus from SkyframeActionExecutor

Post to the Skyframe Environment instead.

PiperOrigin-RevId: 182096559

10 files changed, 88 insertions, 42 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/actions/ActionCompletionEvent.java b/src/main/java/com/google/devtools/build/lib/actions/ActionCompletionEvent.java
index d4621ea827..3850f227b4 100644
--- a/src/main/java/com/google/devtools/build/lib/actions/ActionCompletionEvent.java
+++ b/src/main/java/com/google/devtools/build/lib/actions/ActionCompletionEvent.java
@@ -13,10 +13,13 @@
 // limitations under the License.
 package com.google.devtools.build.lib.actions;
 
+import com.google.common.base.MoreObjects;
+import com.google.devtools.build.lib.events.ExtendedEventHandler.ProgressLike;
+
 /**
  * An event that is fired after an action completes (either successfully or not).
  */
-public final class ActionCompletionEvent {
+public final class ActionCompletionEvent implements ProgressLike {
 
   private final long relativeActionStartTime;
   private final Action action;
@@ -43,4 +46,13 @@ public final class ActionCompletionEvent {
   public ActionLookupData getActionLookupData() {
     return actionLookupData;
   }
+
+  @Override
+  public String toString() {
+    return MoreObjects.toStringHelper("ActionCompletionEvent")
+        .add("relativeActionStartTime", relativeActionStartTime)
+        .add("action", action)
+        .add("actionLookupData", actionLookupData)
+        .toString();
+  }
 }
diff --git a/src/main/java/com/google/devtools/build/lib/actions/ActionExecutedEvent.java b/src/main/java/com/google/devtools/build/lib/actions/ActionExecutedEvent.java
index 3a5bb8d96f..eb45ecc701 100644
--- a/src/main/java/com/google/devtools/build/lib/actions/ActionExecutedEvent.java
+++ b/src/main/java/com/google/devtools/build/lib/actions/ActionExecutedEvent.java
@@ -25,6 +25,7 @@ import com.google.devtools.build.lib.buildeventstream.BuildEventWithConfiguratio
 import com.google.devtools.build.lib.buildeventstream.GenericBuildEvent;
 import com.google.devtools.build.lib.buildeventstream.NullConfiguration;
 import com.google.devtools.build.lib.buildeventstream.PathConverter;
+import com.google.devtools.build.lib.events.ExtendedEventHandler.ProgressLike;
 import com.google.devtools.build.lib.vfs.Path;
 import java.util.Collection;
 import java.util.logging.Level;
@@ -34,7 +35,7 @@ import java.util.logging.Logger;
  * This event is fired during the build, when an action is executed. It contains information about
  * the action: the Action itself, and the output file names its stdout and stderr are recorded in.
  */
-public class ActionExecutedEvent implements BuildEventWithConfiguration {
+public class ActionExecutedEvent implements BuildEventWithConfiguration, ProgressLike {
   private static final Logger logger = Logger.getLogger(ActionExecutedEvent.class.getName());
 
   private final Action action;
diff --git a/src/main/java/com/google/devtools/build/lib/actions/ActionMiddlemanEvent.java b/src/main/java/com/google/devtools/build/lib/actions/ActionMiddlemanEvent.java
index df064d8cfb..6468827938 100644
--- a/src/main/java/com/google/devtools/build/lib/actions/ActionMiddlemanEvent.java
+++ b/src/main/java/com/google/devtools/build/lib/actions/ActionMiddlemanEvent.java
@@ -14,13 +14,14 @@
 package com.google.devtools.build.lib.actions;
 
 import com.google.common.base.Preconditions;
+import com.google.devtools.build.lib.events.ExtendedEventHandler.ProgressLike;
 
 /**
  * This event is fired during the build, when a middleman action is executed. Middleman actions
  * don't usually do any computation but we need them in the critical path because they depend on
  * other actions.
  */
-public class ActionMiddlemanEvent {
+public class ActionMiddlemanEvent implements ProgressLike {
 
   private final Action action;
   private final long nanoTimeStart;
diff --git a/src/main/java/com/google/devtools/build/lib/actions/ActionResultReceivedEvent.java b/src/main/java/com/google/devtools/build/lib/actions/ActionResultReceivedEvent.java
index 3921c57869..cb702c1da4 100644
--- a/src/main/java/com/google/devtools/build/lib/actions/ActionResultReceivedEvent.java
+++ b/src/main/java/com/google/devtools/build/lib/actions/ActionResultReceivedEvent.java
@@ -14,11 +14,13 @@
 
 package com.google.devtools.build.lib.actions;
 
+import com.google.devtools.build.lib.events.ExtendedEventHandler.ProgressLike;
+
 /**
  * An event that is fired when a non-empty {@link ActionResult} is returned by the execution of an
  * {@link Action}.
  */
-public final class ActionResultReceivedEvent {
+public final class ActionResultReceivedEvent implements ProgressLike {
   private final Action action;
   private final ActionResult actionResult;
 
diff --git a/src/main/java/com/google/devtools/build/lib/actions/ActionStartedEvent.java b/src/main/java/com/google/devtools/build/lib/actions/ActionStartedEvent.java
index cf9c902d77..189f36a2c6 100644
--- a/src/main/java/com/google/devtools/build/lib/actions/ActionStartedEvent.java
+++ b/src/main/java/com/google/devtools/build/lib/actions/ActionStartedEvent.java
@@ -13,10 +13,12 @@
 // limitations under the License.
 package com.google.devtools.build.lib.actions;
 
+import com.google.devtools.build.lib.events.ExtendedEventHandler.ProgressLike;
+
 /**
  * This event is fired during the build, when an action is started.
  */
-public class ActionStartedEvent {
+public class ActionStartedEvent implements ProgressLike {
   private final Action action;
   private final long nanoTimeStart;
 
diff --git a/src/main/java/com/google/devtools/build/lib/actions/CachedActionEvent.java b/src/main/java/com/google/devtools/build/lib/actions/CachedActionEvent.java
index d1e7d6680b..240f5bfc66 100644
--- a/src/main/java/com/google/devtools/build/lib/actions/CachedActionEvent.java
+++ b/src/main/java/com/google/devtools/build/lib/actions/CachedActionEvent.java
@@ -14,10 +14,12 @@
 
 package com.google.devtools.build.lib.actions;
 
+import com.google.devtools.build.lib.events.ExtendedEventHandler.ProgressLike;
+
 /**
  * This event is fired during the build if an action was in the action cache.
  */
-public class CachedActionEvent {
+public class CachedActionEvent implements ProgressLike {
 
   private final Action action;
   private final long nanoTimeStart;
diff --git a/src/main/java/com/google/devtools/build/lib/events/ExtendedEventHandler.java b/src/main/java/com/google/devtools/build/lib/events/ExtendedEventHandler.java
index a05b5f5af9..791789bf0c 100644
--- a/src/main/java/com/google/devtools/build/lib/events/ExtendedEventHandler.java
+++ b/src/main/java/com/google/devtools/build/lib/events/ExtendedEventHandler.java
@@ -27,7 +27,7 @@ public interface ExtendedEventHandler extends EventHandler {
   void post(Postable obj);
 
   /**
-   * Interface for declaring postable events that report about progress (as oposed to success or
+   * Interface for declaring postable events that report about progress (as opposed to success or
    * failure) and hence should not be stored and replayed.
    */
   public interface ProgressLike extends Postable {}
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 52b9b26a93..4386023fa2 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
@@ -360,7 +360,8 @@ public class ActionExecutionFunction implements SkyFunction, CompletionReceiver
     // If this is a shared action and the other action is the one that executed, we must use that
     // other action's value, provided here, since it is populated with metadata for the outputs.
     if (!state.hasArtifactData()) {
-      return skyframeActionExecutor.executeAction(action, null, -1, null, actionLookupData);
+      return skyframeActionExecutor
+          .executeAction(env.getListener(), action, null, -1, null, actionLookupData);
     }
     // This may be recreated if we discover inputs.
     ActionMetadataHandler metadataHandler = new ActionMetadataHandler(state.inputArtifactData,
@@ -370,6 +371,7 @@ public class ActionExecutionFunction implements SkyFunction, CompletionReceiver
     if (!state.hasCheckedActionCache()) {
       state.token =
           skyframeActionExecutor.checkActionCache(
+              env.getListener(),
               action,
               metadataHandler,
               actionStartTime,
@@ -443,7 +445,8 @@ public class ActionExecutionFunction implements SkyFunction, CompletionReceiver
       if (!state.hasExecutedAction()) {
         state.value =
             skyframeActionExecutor.executeAction(
-                action, metadataHandler, actionStartTime, actionExecutionContext, actionLookupData);
+                env.getListener(), action, metadataHandler, actionStartTime, actionExecutionContext,
+                actionLookupData);
       }
     } catch (IOException e) {
       throw new ActionExecutionException(
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeActionExecutor.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeActionExecutor.java
index c1d75375cf..97d257c84a 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeActionExecutor.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeActionExecutor.java
@@ -70,6 +70,7 @@ import com.google.devtools.build.lib.concurrent.Sharder;
 import com.google.devtools.build.lib.concurrent.ThrowableRecordingRunnableWrapper;
 import com.google.devtools.build.lib.events.Event;
 import com.google.devtools.build.lib.events.EventHandler;
+import com.google.devtools.build.lib.events.ExtendedEventHandler;
 import com.google.devtools.build.lib.events.Reporter;
 import com.google.devtools.build.lib.profiler.Profiler;
 import com.google.devtools.build.lib.profiler.ProfilerTask;
@@ -117,7 +118,6 @@ public final class SkyframeActionExecutor implements ActionExecutionContextFacto
 
   private final ActionKeyContext actionKeyContext;
   private Reporter reporter;
-  private final AtomicReference<EventBus> eventBus;
   private Map<String, String> clientEnv = ImmutableMap.of();
   private Executor executorEngine;
   private ActionLogBufferPathGenerator actionLogBufferPathGenerator;
@@ -158,10 +158,8 @@ public final class SkyframeActionExecutor implements ActionExecutionContextFacto
 
   SkyframeActionExecutor(
       ActionKeyContext actionKeyContext,
-      AtomicReference<EventBus> eventBus,
       AtomicReference<ActionExecutionStatusReporter> statusReporterRef) {
     this.actionKeyContext = actionKeyContext;
-    this.eventBus = eventBus;
     this.statusReporterRef = statusReporterRef;
   }
 
@@ -385,6 +383,7 @@ public final class SkyframeActionExecutor implements ActionExecutionContextFacto
    * <p>For use from {@link ArtifactFunction} only.
    */
   ActionExecutionValue executeAction(
+      ExtendedEventHandler eventHandler,
       Action action,
       ActionMetadataHandler metadataHandler,
       long actionStartTime,
@@ -400,6 +399,7 @@ public final class SkyframeActionExecutor implements ActionExecutionContextFacto
     FutureTask<ActionExecutionValue> actionTask =
         new FutureTask<>(
             new ActionRunner(
+                eventHandler,
                 action,
                 metadataHandler,
                 actionStartTime,
@@ -489,6 +489,7 @@ public final class SkyframeActionExecutor implements ActionExecutionContextFacto
    * should be provided to the ActionCacheChecker after execution.
    */
   Token checkActionCache(
+      ExtendedEventHandler eventHandler,
       Action action,
       MetadataHandler metadataHandler,
       long actionStartTime,
@@ -503,7 +504,7 @@ public final class SkyframeActionExecutor implements ActionExecutionContextFacto
       boolean eventPosted = false;
       // Notify BlazeRuntimeStatistics about the action middleman 'execution'.
       if (action.getActionType().isMiddleman()) {
-        postEvent(new ActionMiddlemanEvent(action, actionStartTime));
+        eventHandler.post(new ActionMiddlemanEvent(action, actionStartTime));
         eventPosted = true;
       }
 
@@ -542,7 +543,7 @@ public final class SkyframeActionExecutor implements ActionExecutionContextFacto
       // We still need to check the outputs so that output file data is available to the value.
       checkOutputs(action, metadataHandler);
       if (!eventPosted) {
-        postEvent(new CachedActionEvent(action, actionStartTime));
+        eventHandler.post(new CachedActionEvent(action, actionStartTime));
       }
     }
     return token;
@@ -583,8 +584,11 @@ public final class SkyframeActionExecutor implements ActionExecutionContextFacto
    * <p>This method is just a wrapper around {@link Action#discoverInputs} that properly processes
    * any ActionExecutionException thrown before rethrowing it to the caller.
    */
-  Iterable<Artifact> discoverInputs(Action action, PerActionFileCache graphFileCache,
-      MetadataHandler metadataHandler, Environment env)
+  Iterable<Artifact> discoverInputs(
+      Action action,
+      PerActionFileCache graphFileCache,
+      MetadataHandler metadataHandler,
+      Environment env)
       throws ActionExecutionException, InterruptedException {
     ActionExecutionContext actionExecutionContext =
         ActionExecutionContext.forInputDiscovery(
@@ -600,7 +604,11 @@ public final class SkyframeActionExecutor implements ActionExecutionContextFacto
       return action.discoverInputs(actionExecutionContext);
     } catch (ActionExecutionException e) {
       throw processAndThrow(
-          e, action, actionExecutionContext.getFileOutErr(), ErrorTiming.BEFORE_EXECUTION);
+          env.getListener(),
+          e,
+          action,
+          actionExecutionContext.getFileOutErr(),
+          ErrorTiming.BEFORE_EXECUTION);
     }
   }
 
@@ -633,6 +641,7 @@ public final class SkyframeActionExecutor implements ActionExecutionContextFacto
   }
 
   private class ActionRunner implements Callable<ActionExecutionValue> {
+    private final ExtendedEventHandler eventHandler;
     private final Action action;
     private final ActionMetadataHandler metadataHandler;
     private long actionStartTime;
@@ -640,11 +649,13 @@ public final class SkyframeActionExecutor implements ActionExecutionContextFacto
     private final ActionLookupData actionLookupData;
 
     ActionRunner(
+        ExtendedEventHandler eventHandler,
         Action action,
         ActionMetadataHandler metadataHandler,
         long actionStartTime,
         ActionExecutionContext actionExecutionContext,
         ActionLookupData actionLookupData) {
+      this.eventHandler = eventHandler;
       this.action = action;
       this.metadataHandler = metadataHandler;
       this.actionStartTime = actionStartTime;
@@ -676,7 +687,7 @@ public final class SkyframeActionExecutor implements ActionExecutionContextFacto
         Preconditions.checkState(actionExecutionContext.getMetadataHandler() == metadataHandler,
             "%s %s", actionExecutionContext.getMetadataHandler(), metadataHandler);
         prepareScheduleExecuteAndCompleteAction(
-            action, actionExecutionContext, actionStartTime, actionLookupData);
+            eventHandler, action, actionExecutionContext, actionStartTime, actionLookupData);
         return new ActionExecutionValue(
             metadataHandler.getOutputArtifactData(),
             metadataHandler.getOutputTreeArtifactData(),
@@ -788,6 +799,7 @@ public final class SkyframeActionExecutor implements ActionExecutionContextFacto
    * @throws InterruptedException if the thread was interrupted.
    */
   private void prepareScheduleExecuteAndCompleteAction(
+      ExtendedEventHandler eventHandler,
       Action action,
       ActionExecutionContext context,
       long actionStartTime,
@@ -802,23 +814,32 @@ public final class SkyframeActionExecutor implements ActionExecutionContextFacto
       reportError("failed to delete output files before executing action", e, action, null);
     }
 
-    postEvent(new ActionStartedEvent(action, actionStartTime));
+    eventHandler.post(new ActionStartedEvent(action, actionStartTime));
     ActionExecutionStatusReporter statusReporter = statusReporterRef.get();
     try {
       // Mark the current action as being prepared.
       statusReporter.updateStatus(ActionStatusMessage.preparingStrategy(action));
-      boolean outputDumped = executeActionTask(action, context);
-      completeAction(action, context.getMetadataHandler(), context.getFileOutErr(), outputDumped);
+      boolean outputDumped = executeActionTask(eventHandler, action, context);
+      completeAction(
+          eventHandler,
+          action,
+          context.getMetadataHandler(),
+          context.getFileOutErr(),
+          outputDumped);
     } finally {
       statusReporter.remove(action);
-      postEvent(new ActionCompletionEvent(actionStartTime, action, actionLookupData));
+      eventHandler.post(new ActionCompletionEvent(actionStartTime, action, actionLookupData));
     }
   }
 
   private ActionExecutionException processAndThrow(
-      ActionExecutionException e, Action action, FileOutErr outErrBuffer, ErrorTiming errorTiming)
-      throws ActionExecutionException {
-    reportActionExecution(action, e, outErrBuffer, errorTiming);
+      ExtendedEventHandler eventHandler,
+      ActionExecutionException e,
+      Action action,
+      FileOutErr outErrBuffer,
+      ErrorTiming errorTiming)
+          throws ActionExecutionException {
+    reportActionExecution(eventHandler, action, e, outErrBuffer, errorTiming);
     boolean reported = reportErrorIfNotAbortingMode(e, outErrBuffer);
 
     ActionExecutionException toThrow = e;
@@ -856,8 +877,11 @@ public final class SkyframeActionExecutor implements ActionExecutionContextFacto
    * @throws InterruptedException if the thread was interrupted.
    * @return true if the action output was dumped, false otherwise.
    */
-  private boolean executeActionTask(Action action, ActionExecutionContext actionExecutionContext)
-      throws ActionExecutionException, InterruptedException {
+  private boolean executeActionTask(
+      ExtendedEventHandler eventHandler,
+      Action action,
+      ActionExecutionContext actionExecutionContext)
+          throws ActionExecutionException, InterruptedException {
     profiler.startTask(ProfilerTask.ACTION_EXECUTE, action);
     // ActionExecutionExceptions that occur as the thread is interrupted are
     // assumed to be a result of that, so we throw InterruptedException
@@ -866,7 +890,7 @@ public final class SkyframeActionExecutor implements ActionExecutionContextFacto
     try {
       ActionResult actionResult = action.execute(actionExecutionContext);
       if (actionResult != ActionResult.EMPTY) {
-        postEvent(new ActionResultReceivedEvent(action, actionResult));
+        eventHandler.post(new ActionResultReceivedEvent(action, actionResult));
       }
 
       // Action terminated fine, now report the output.
@@ -880,14 +904,18 @@ public final class SkyframeActionExecutor implements ActionExecutionContextFacto
       }
       // Defer reporting action success until outputs are checked
     } catch (ActionExecutionException e) {
-      throw processAndThrow(e, action, outErrBuffer, ErrorTiming.AFTER_EXECUTION);
+      throw processAndThrow(eventHandler, e, action, outErrBuffer, ErrorTiming.AFTER_EXECUTION);
     } finally {
       profiler.completeTask(ProfilerTask.ACTION_EXECUTE);
     }
     return false;
   }
 
-  private void completeAction(Action action, MetadataHandler metadataHandler, FileOutErr fileOutErr,
+  private void completeAction(
+      ExtendedEventHandler eventHandler,
+      Action action,
+      MetadataHandler metadataHandler,
+      FileOutErr fileOutErr,
       boolean outputAlreadyDumped) throws ActionExecutionException {
     try {
       Preconditions.checkState(action.inputsDiscovered(),
@@ -911,14 +939,16 @@ public final class SkyframeActionExecutor implements ActionExecutionContextFacto
         }
       }
 
-      reportActionExecution(action, null, fileOutErr, ErrorTiming.NO_ERROR);
+      reportActionExecution(eventHandler, action, null, fileOutErr, ErrorTiming.NO_ERROR);
     } catch (ActionExecutionException actionException) {
       // Success in execution but failure in completion.
-      reportActionExecution(action, actionException, fileOutErr, ErrorTiming.AFTER_EXECUTION);
+      reportActionExecution(
+          eventHandler, action, actionException, fileOutErr, ErrorTiming.AFTER_EXECUTION);
       throw actionException;
     } catch (IllegalStateException exception) {
       // More serious internal error, but failure still reported.
       reportActionExecution(
+          eventHandler,
           action,
           new ActionExecutionException(exception, action, true),
           fileOutErr,
@@ -993,13 +1023,6 @@ public final class SkyframeActionExecutor implements ActionExecutionContextFacto
     return success;
   }
 
-  private void postEvent(Object event) {
-    EventBus bus = eventBus.get();
-    if (bus != null) {
-      bus.post(event);
-    }
-  }
-
   /**
    * Convenience function for reporting that the action failed due to a
    * the exception cause, if there is an additional explanatory message that
@@ -1100,6 +1123,7 @@ public final class SkyframeActionExecutor implements ActionExecutionContextFacto
   }
 
   private void reportActionExecution(
+      ExtendedEventHandler eventHandler,
       Action action,
       ActionExecutionException exception,
       FileOutErr outErr,
@@ -1113,7 +1137,7 @@ public final class SkyframeActionExecutor implements ActionExecutionContextFacto
     if (outErr.hasRecordedStderr()) {
       stderr = outErr.getErrorPath();
     }
-    postEvent(new ActionExecutedEvent(action, exception, stdout, stderr, errorTiming));
+    eventHandler.post(new ActionExecutedEvent(action, exception, stdout, stderr, errorTiming));
   }
 
   /**
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 2038e11505..d5519db499 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
@@ -316,8 +316,7 @@ public abstract class SkyframeExecutor implements WalkableGraphFactory {
         new SkyframePackageLoader(), new SkyframeTransitivePackageLoader(),
         syscalls, cyclesReporter, pkgLocator, numPackagesLoaded, this);
     this.resourceManager = ResourceManager.instance();
-    this.skyframeActionExecutor =
-        new SkyframeActionExecutor(actionKeyContext, eventBus, statusReporterRef);
+    this.skyframeActionExecutor = new SkyframeActionExecutor(actionKeyContext, statusReporterRef);
     this.fileSystem = fileSystem;
     this.directories = Preconditions.checkNotNull(directories);
     this.actionKeyContext = Preconditions.checkNotNull(actionKeyContext);