Post AnalysisPhaseCompleteEvent for Skymeld.

Many classes, especially the ones collecting metrics around a build, rely on AnalysisPhaseCompleteEvent. For Skymeld, that is problematic since we don't have a clear boundary between analysis/execution.

With this cl, we redefine the meaning of AnalysisPhaseCompleteEvent with --skymeld: it's the moment when all analysis-related work is concluded in the build, and blaze is expected to already be deep in execution work when this moment happens.

This also means that BEP's analysis_phase_time_in_ms [1] keeps its original meaning with Skymeld: it's the wall time duration between the start and the end of analysis. The difference: it's no longer true that total wall time = analysis wall time + execution wall time, as analysis and execution overlap.

To implement this, we introduce AnalysisOperationWatcher, which:
- Has a pre-filled expected set of BuildDriverKeys,
- Listens to TopLevelEntityAnalysisConcludedEvents which include information of the respective BuildDriverKey,
- Removes the BuildDriverKey from the expected set upon receiving the events, and
- Posts an AnalysisPhaseCompleteEvent when the expected set is empty.

The analysis work of a top level target is considered "concluded" when its analysis either fails or succeeds (which also includes work outside of the main Skyframe evaluation, like action conflict checking and compatibility checking).

In case of --nokeep_going and an analysis error occurs, there's no AnalysisPhaseCompleteEvent. This is consistent with the behavior of --noskymeld in the same scenario.

[1] https://github.com/bazelbuild/bazel/blob/1941e2b1b5be8596e984b02fd5dd37e3ed25a81f/src/main/java/com/google/devtools/build/lib/buildeventstream/proto/build_event_stream.proto#L948

PiperOrigin-RevId: 461154740
Change-Id: I731c817280e66a6aad3781cc578c60a708a56ceb

Author: joeleba

src/main/java/com/google/devtools/build/lib/analysis/AnalysisOperationWatcher.java

@@ -0,0 +1,74 @@
+// Copyright 2022 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.analysis;
+
+import com.google.common.eventbus.EventBus;
+import com.google.common.eventbus.Subscribe;
+import com.google.devtools.build.lib.skyframe.TopLevelStatusEvents.TopLevelEntityAnalysisConcludedEvent;
+import com.google.devtools.build.skyframe.SkyKey;
+import java.util.Set;
+
+/**
+ * A watcher for analysis-related work that sends out a signal when all such work in the build is
+ * done. There's one instance of this class per build.
+ */
+public class AnalysisOperationWatcher implements AutoCloseable {
+  // Since the events are fired from within a SkyFunction, it's possible that the same event is
+  // fired multiple times. A simple counter would therefore not suffice.
+  private final Set<SkyKey> threadSafeExpectedKeys;
+  private final EventBus eventBus;
+  private final AnalysisOperationWatcherFinisher finisher;
+
+  private AnalysisOperationWatcher(
+      Set<SkyKey> threadSafeExpectedKeys,
+      EventBus eventBus,
+      AnalysisOperationWatcherFinisher finisher) {
+    this.threadSafeExpectedKeys = threadSafeExpectedKeys;
+    this.eventBus = eventBus;
+    this.finisher = finisher;
+  }
+
+  /** Creates an AnalysisOperationWatcher and registers it with the provided eventBus. */
+  public static AnalysisOperationWatcher createAndRegisterWithEventBus(
+      Set<SkyKey> threadSafeExpectedKeys,
+      EventBus eventBus,
+      AnalysisOperationWatcherFinisher finisher) {
+    AnalysisOperationWatcher watcher =
+        new AnalysisOperationWatcher(threadSafeExpectedKeys, eventBus, finisher);
+    eventBus.register(watcher);
+    return watcher;
+  }
+
+  @Subscribe
+  public void handleTopLevelEntityAnalysisConcluded(TopLevelEntityAnalysisConcludedEvent e) {
+    if (threadSafeExpectedKeys.isEmpty()) {
+      return;
+    }
+    threadSafeExpectedKeys.remove(e.getAnalyzedTopLevelKey());
+    if (threadSafeExpectedKeys.isEmpty()) {
+      finisher.sendAnalysisPhaseCompleteEvent();
+    }
+  }
+
+  @Override
+  public void close() {
+    eventBus.unregister(this);
+  }
+
+  /** A callback to be called when all the expected keys have been analyzed. */
+  @FunctionalInterface
+  public interface AnalysisOperationWatcherFinisher {
+    void sendAnalysisPhaseCompleteEvent();
+  }
+}

src/main/java/com/google/devtools/build/lib/analysis/AnalysisPhaseCompleteEvent.java

@@ -32,6 +32,7 @@ public class AnalysisPhaseCompleteEvent {
   private final PackageManagerStatistics pkgManagerStats;
   private final TotalAndConfiguredTargetOnlyMetric actionsConstructed;
   private final boolean analysisCacheDropped;
+  private final boolean skymeldEnabled;
 
   public AnalysisPhaseCompleteEvent(
       Collection<? extends ConfiguredTarget> topLevelTargets,
@@ -40,12 +41,56 @@ public AnalysisPhaseCompleteEvent(
       long timeInMs,
       PackageManagerStatistics pkgManagerStats,
       boolean analysisCacheDropped) {
+    this(
+        topLevelTargets,
+        targetsConfigured,
+        actionsConstructed,
+        timeInMs,
+        pkgManagerStats,
+        analysisCacheDropped,
+        /*skymeldEnabled=*/ false);
+  }
+
+  private AnalysisPhaseCompleteEvent(
+      Collection<? extends ConfiguredTarget> topLevelTargets,
+      TotalAndConfiguredTargetOnlyMetric targetsConfigured,
+      TotalAndConfiguredTargetOnlyMetric actionsConstructed,
+      long timeInMs,
+      PackageManagerStatistics pkgManagerStats,
+      boolean analysisCacheDropped,
+      boolean skymeldEnabled) {
     this.timeInMs = timeInMs;
     this.topLevelTargets = ImmutableList.copyOf(topLevelTargets);
     this.targetsConfigured = checkNotNull(targetsConfigured);
     this.pkgManagerStats = pkgManagerStats;
     this.actionsConstructed = checkNotNull(actionsConstructed);
     this.analysisCacheDropped = analysisCacheDropped;
+    this.skymeldEnabled = skymeldEnabled;
+  }
+
+  /**
+   * A factory method for the AnalysisPhaseCompleteEvent that originates from Skymeld.
+   *
+   * <p>This marks the end of the analysis-related work within the build. Contrary to the
+   * traditional build where there is a distinct separation between the loading/analysis and
+   * execution phases, overlapping is possible with Skymeld. We are likely already deep into action
+   * execution when this event is posted.
+   */
+  public static AnalysisPhaseCompleteEvent fromSkymeld(
+      Collection<? extends ConfiguredTarget> topLevelTargets,
+      TotalAndConfiguredTargetOnlyMetric targetsConfigured,
+      TotalAndConfiguredTargetOnlyMetric actionsConstructed,
+      long timeInMs,
+      PackageManagerStatistics pkgManagerStats,
+      boolean analysisCacheDropped) {
+    return new AnalysisPhaseCompleteEvent(
+        topLevelTargets,
+        targetsConfigured,
+        actionsConstructed,
+        timeInMs,
+        pkgManagerStats,
+        analysisCacheDropped,
+        /*skymeldEnabled=*/ true);
   }
 
   /**
@@ -74,6 +119,14 @@ public boolean wasAnalysisCacheDropped() {
     return analysisCacheDropped;
   }
 
+  /**
+   * Returns whether this event originated from Skymeld. Some subscribers are incompatible with
+   * Skymeld and this distinction is required for now.
+   */
+  public boolean isOriginatedFromSkymeld() {
+    return skymeldEnabled;
+  }
+
   /**
    * Returns package manager statistics.
    */

src/main/java/com/google/devtools/build/lib/analysis/BUILD

@@ -480,6 +480,16 @@ java_library(
     ],
 )
 
+java_library(
+    name = "analysis_operation_watcher",
+    srcs = ["AnalysisOperationWatcher.java"],
+    deps = [
+        "//src/main/java/com/google/devtools/build/lib/skyframe:top_level_status_events",
+        "//src/main/java/com/google/devtools/build/skyframe:skyframe-objects",
+        "//third_party:guava",
+    ],
+)
+
 java_library(
     name = "analysis_phase_complete_event",
     srcs = ["AnalysisPhaseCompleteEvent.java"],
@@ -625,6 +635,7 @@ java_library(
         "//src/main/java/com/google/devtools/build/lib/profiler",
         "//src/main/java/com/google/devtools/build/lib/skyframe:aspect_key_creator",
         "//src/main/java/com/google/devtools/build/lib/skyframe:build_configuration",
+        "//src/main/java/com/google/devtools/build/lib/skyframe:build_result_listener",
         "//src/main/java/com/google/devtools/build/lib/skyframe:configured_target_key",
         "//src/main/java/com/google/devtools/build/lib/skyframe:coverage_report_value",
         "//src/main/java/com/google/devtools/build/lib/skyframe:skyframe_cluster",

src/main/java/com/google/devtools/build/lib/analysis/BuildView.java

@@ -80,6 +80,7 @@
 import com.google.devtools.build.lib.skyframe.AspectKeyCreator.AspectKey;
 import com.google.devtools.build.lib.skyframe.AspectKeyCreator.TopLevelAspectsKey;
 import com.google.devtools.build.lib.skyframe.BuildConfigurationKey;
+import com.google.devtools.build.lib.skyframe.BuildResultListener;
 import com.google.devtools.build.lib.skyframe.ConfiguredTargetKey;
 import com.google.devtools.build.lib.skyframe.CoverageReportValue;
 import com.google.devtools.build.lib.skyframe.PrepareAnalysisPhaseValue;
@@ -213,7 +214,8 @@ public AnalysisResult update(
       BugReporter bugReporter,
       boolean includeExecutionPhase,
       int mergedPhasesExecutionJobsCount,
-      @Nullable ResourceManager resourceManager)
+      @Nullable ResourceManager resourceManager,
+      @Nullable BuildResultListener buildResultListener)
       throws ViewCreationFailedException, InvalidConfigurationException, InterruptedException,
           BuildFailedException, TestExecException {
     logger.atInfo().log("Starting analysis");
@@ -410,9 +412,6 @@ public AnalysisResult update(
       } else {
         skyframeExecutor.setRuleContextConstraintSemantics(
             (RuleContextConstraintSemantics) ruleClassProvider.getConstraintSemantics());
-        // For the Skymeld code path, we expect action execution and hence a non-null resource
-        // manager.
-        Preconditions.checkNotNull(resourceManager);
         skyframeAnalysisResult =
             skyframeBuildView.analyzeAndExecuteTargets(
                 eventHandler,
@@ -425,7 +424,8 @@ public AnalysisResult update(
                 explicitTargetPatterns,
                 eventBus,
                 bugReporter,
-                resourceManager,
+                Preconditions.checkNotNull(resourceManager), // non-null for skymeld.
+                Preconditions.checkNotNull(buildResultListener), // non-null for skymeld.
                 keepGoing,
                 viewOptions.strictConflictChecks,
                 checkForActionConflicts,

src/main/java/com/google/devtools/build/lib/buildtool/AnalysisAndExecutionPhaseRunner.java

@@ -207,7 +207,8 @@ private static AnalysisAndExecutionResult runAnalysisAndExecutionPhase(
             env.getRuntime().getBugReporter(),
             /*includeExecutionPhase=*/ true,
             request.getBuildOptions().jobs,
-            env.getLocalResourceManager());
+            env.getLocalResourceManager(),
+            env.getBuildResultListener());
   }
 
   /**

src/main/java/com/google/devtools/build/lib/buildtool/AnalysisPhaseRunner.java

@@ -242,7 +242,8 @@ private static AnalysisResult runAnalysisPhase(
               env.getRuntime().getBugReporter(),
               /*includeExecutionPhase=*/ false,
               /*mergedPhasesExecutionJobsCount=*/ 0,
-              /*resourceManager=*/ null);
+              /*resourceManager=*/ null,
+              /*buildResultListener=*/ null);
     } catch (BuildFailedException | TestExecException unexpected) {
       throw new IllegalStateException("Unexpected execution exception type: ", unexpected);
     }

src/main/java/com/google/devtools/build/lib/runtime/UiEventHandler.java

@@ -555,6 +555,10 @@ public void loadingComplete(LoadingPhaseCompleteEvent event) {
 
   @Subscribe
   public synchronized void analysisComplete(AnalysisPhaseCompleteEvent event) {
+    // TODO(b/215335350): Make this work with Skymeld. Ignore for now.
+    if (event.isOriginatedFromSkymeld()) {
+      return;
+    }
     String analysisSummary = stateTracker.analysisComplete();
     handle(Event.info(null, analysisSummary));
   }

src/main/java/com/google/devtools/build/lib/skyframe/BUILD

@@ -106,6 +106,7 @@ java_library(
         ":build_driver_key",
         ":build_driver_value",
         ":build_info_collection_value",
+        ":build_result_listener",
         ":bzl_compile",
         ":bzl_load_value",
         ":cached_bzl_load_value_and_deps",
@@ -228,7 +229,9 @@ java_library(
         "//src/main/java/com/google/devtools/build/lib/actions:resource_manager",
         "//src/main/java/com/google/devtools/build/lib/actions:thread_state_receiver",
         "//src/main/java/com/google/devtools/build/lib/analysis:analysis_cluster",
+        "//src/main/java/com/google/devtools/build/lib/analysis:analysis_operation_watcher",
         "//src/main/java/com/google/devtools/build/lib/analysis:analysis_options",
+        "//src/main/java/com/google/devtools/build/lib/analysis:analysis_phase_complete_event",
         "//src/main/java/com/google/devtools/build/lib/analysis:aspect_collection",
         "//src/main/java/com/google/devtools/build/lib/analysis:blaze_directories",
         "//src/main/java/com/google/devtools/build/lib/analysis:buildinfo/build_info_collection",
@@ -2256,6 +2259,7 @@ java_library(
         "//src/main/java/com/google/devtools/build/lib/events",
         "//src/main/java/com/google/devtools/build/lib/packages",
         "//src/main/java/com/google/devtools/build/lib/pkgcache",
+        "//src/main/java/com/google/devtools/build/lib/skyframe:top_level_status_events",
         "//src/main/java/com/google/devtools/build/lib/util:detailed_exit_code",
         "//src/main/java/com/google/devtools/build/skyframe",
         "//src/main/java/com/google/devtools/build/skyframe:skyframe-objects",
@@ -2650,6 +2654,7 @@ java_library(
         "//src/main/java/com/google/devtools/build/lib/analysis:config/build_configuration",
         "//src/main/java/com/google/devtools/build/lib/analysis:configured_target",
         "//src/main/java/com/google/devtools/build/lib/events",
+        "//src/main/java/com/google/devtools/build/skyframe:skyframe-objects",
         "//third_party:auto_value",
     ],
 )

src/main/java/com/google/devtools/build/lib/skyframe/BuildDriverFunction.java

@@ -56,6 +56,7 @@
 import com.google.devtools.build.lib.skyframe.TopLevelStatusEvents.AspectAnalyzedEvent;
 import com.google.devtools.build.lib.skyframe.TopLevelStatusEvents.SomeExecutionStartedEvent;
 import com.google.devtools.build.lib.skyframe.TopLevelStatusEvents.TestAnalyzedEvent;
+import com.google.devtools.build.lib.skyframe.TopLevelStatusEvents.TopLevelEntityAnalysisConcludedEvent;
 import com.google.devtools.build.lib.skyframe.TopLevelStatusEvents.TopLevelTargetAnalyzedEvent;
 import com.google.devtools.build.lib.skyframe.TopLevelStatusEvents.TopLevelTargetPendingExecutionEvent;
 import com.google.devtools.build.lib.skyframe.TopLevelStatusEvents.TopLevelTargetSkippedEvent;
@@ -146,6 +147,10 @@ public SkyValue compute(SkyKey skyKey, Environment env)
     if (topLevelSkyValue instanceof ConfiguredTargetValue) {
       ConfiguredTarget configuredTarget =
           ((ConfiguredTargetValue) topLevelSkyValue).getConfiguredTarget();
+      // At this point, the target is considered "analyzed". It's important that this event is sent
+      // before the TopLevelEntityAnalysisConcludedEvent: when the last of the analysis work is
+      // concluded, we need to have the *complete* list of analyzed targets ready in
+      // BuildResultListener.
       env.getListener().post(TopLevelTargetAnalyzedEvent.create(configuredTarget));
 
       BuildConfigurationValue buildConfigurationValue =
@@ -178,6 +183,9 @@ public SkyValue compute(SkyKey skyKey, Environment env)
                       TestAnalyzedEvent.create(
                           configuredTarget, buildConfigurationValue, /*isSkipped=*/ true));
             }
+            // Only send the event now to include the compatibility check in the measurement for
+            // time spent on analysis work.
+            env.getListener().post(TopLevelEntityAnalysisConcludedEvent.create(buildDriverKey));
             // We consider the evaluation of this BuildDriverKey successful at this point, even when
             // the target is skipped.
             return new BuildDriverValue(topLevelSkyValue, /*skipped=*/ true);
@@ -187,6 +195,7 @@ public SkyValue compute(SkyKey skyKey, Environment env)
         }
       }
 
+      env.getListener().post(TopLevelEntityAnalysisConcludedEvent.create(buildDriverKey));
       env.getListener()
           .post(
               TopLevelTargetPendingExecutionEvent.create(
@@ -199,6 +208,7 @@ public SkyValue compute(SkyKey skyKey, Environment env)
           env,
           topLevelArtifactContext);
     } else {
+      env.getListener().post(TopLevelEntityAnalysisConcludedEvent.create(buildDriverKey));
       requestAspectExecution((TopLevelAspectsValue) topLevelSkyValue, env, topLevelArtifactContext);
     }