commit c031fab778ea1c5df2101c20521aec7c9cd9c02b
author Googler <leba@google.com> Mon Mar 18 07:34:32 2024 -0700
committer Copybara-Service <copybara-worker@google.com> Mon Mar 18 07:36:07 2024 -0700
tree 381ff2200868eb6644c7d476d294107e3c9b2355
parent 383b6071de2dcb613d0361e0d74956cf1bd5257e

Send `TopLevelEntityAnalysisConcludedEvent` as soon as the analysis of a top level target/aspect fails.

The rest of this description assumes **`--keep_going`**, since it's the main mode affected.

**Background**

Bazel traditionally does a number of things upon the completion of the analysis phase, including dropping states with a certain combination of flags. This is done to prevent OOM-ing.

With Skymeld, the "completion of the analysis phase" mark isn't clear cut. Each top-level target signals a centralized watcher when it's done with analysis. When this watcher sees that every target's analysis has finished, we drop the states.

**The Issue**

We failed to signal the watcher right when a target's analysis **failed**, and instead only did so when going through the errors at the end of the build [1].

While this isn't a correctness concern, it's a huge performance drag. Without the "concluded" signal, the release of the analysis states is delayed, ends up hogging much of the heap and leaves little room for execution. One single analysis failure can therefore tank the whole build via OOMing.

**The Fix**

With this CL, we signal the conclusion of the analysis of a key immediately once it's failed.

---
Alternative considered: add a post-processing step in `AbstractParallelEvaluator` if a `BuildDriverKey` is found to have a child error in `--keep_going` mode.

- Pros: by default covers every type of Exceptions thrown during analysis or otherwise that would lead to a failed BuildDriverKey eval.
- Cons: More intrusive: need to redefine some of the events, need to break some assumptions about posting events (e.g. posting even after the evaluation is already inactive)

PiperOrigin-RevId: 616821894
Change-Id: I5b8047ee191d2beaf510c213c448ffe5d3371615

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

diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/BuildDriverKey.java b/src/main/java/com/google/devtools/build/lib/skyframe/BuildDriverKey.java
index ff737bb..75c3a36 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/BuildDriverKey.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/BuildDriverKey.java
@@ -32,12 +32,16 @@
 
   private final boolean extraActionTopLevelOnly;
 
+  // This key is created anew each build, so it's fine to carry this information here.
+  private final boolean keepGoing;
+
   private BuildDriverKey(
       ActionLookupKey actionLookupKey,
       TopLevelArtifactContext topLevelArtifactContext,
       boolean explicitlyRequested,
       boolean skipIncompatibleExplicitTargets,
       boolean extraActionTopLevelOnly,
+      boolean keepGoing,
       boolean isTopLevelAspectDriver) {
     this.actionLookupKey = actionLookupKey;
     this.topLevelArtifactContext = topLevelArtifactContext;
@@ -45,6 +49,7 @@
     this.skipIncompatibleExplicitTargets = skipIncompatibleExplicitTargets;
     this.isTopLevelAspectDriver = isTopLevelAspectDriver;
     this.extraActionTopLevelOnly = extraActionTopLevelOnly;
+    this.keepGoing = keepGoing;
   }
 
   public static BuildDriverKey ofTopLevelAspect(
@@ -52,13 +57,15 @@
       TopLevelArtifactContext topLevelArtifactContext,
       boolean explicitlyRequested,
       boolean skipIncompatibleExplicitTargets,
-      boolean extraActionTopLevelOnly) {
+      boolean extraActionTopLevelOnly,
+      boolean keepGoing) {
     return new BuildDriverKey(
         actionLookupKey,
         topLevelArtifactContext,
         explicitlyRequested,
         skipIncompatibleExplicitTargets,
         extraActionTopLevelOnly,
+        keepGoing,
         /* isTopLevelAspectDriver= */ true);
   }
 
@@ -67,13 +74,15 @@
       TopLevelArtifactContext topLevelArtifactContext,
       boolean explicitlyRequested,
       boolean skipIncompatibleExplicitTargets,
-      boolean extraActionTopLevelOnly) {
+      boolean extraActionTopLevelOnly,
+      boolean keepGoing) {
     return new BuildDriverKey(
         actionLookupKey,
         topLevelArtifactContext,
         explicitlyRequested,
         skipIncompatibleExplicitTargets,
         extraActionTopLevelOnly,
+        keepGoing,
         /* isTopLevelAspectDriver= */ false);
   }
 
@@ -101,6 +110,10 @@
     return extraActionTopLevelOnly;
   }
 
+  public boolean keepGoing() {
+    return keepGoing;
+  }
+
   @Override
   public SkyFunctionName functionName() {
     return SkyFunctions.BUILD_DRIVER;