skyfocus: support --nouse_action_cache, fixes NPE with `ActionCache…

…#size` calls. PiperOrigin-RevId: 642840333 Change-Id: Ia381036a9aad6ffa4899a97a470ad157a84d1389
bazelbuild · Jun 13, 2024 · d095e2e · d095e2e
1 parent e73dca7
commit d095e2e
Show file tree

Hide file tree

Showing 5 changed files with 39 additions and 16 deletions.
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/BUILD b/src/main/java/com/google/devtools/build/lib/skyframe/BUILD
@@ -2582,6 +2582,7 @@ java_library(
         "//src/main/java/com/google/devtools/build/skyframe",
         "//src/main/java/com/google/devtools/build/skyframe:skyframe-objects",
         "//third_party:guava",
+        "//third_party:jsr305",
     ],
 )
 

diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkyfocusExecutor.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkyfocusExecutor.java
@@ -46,6 +46,7 @@
 import java.util.Optional;
 import java.util.Set;
 import java.util.stream.Stream;
+import javax.annotation.Nullable;
 
 /** A class that prepares the working set to run the core SkyframeFocuser algorithm. */
 public class SkyfocusExecutor {
@@ -237,7 +238,7 @@ public static FocusResult execute(
       ImmutableSet<FileStateKey> workingSet,
       InMemoryMemoizingEvaluator evaluator,
       ExtendedEventHandler eventHandler,
-      ActionCache actionCache)
+      @Nullable ActionCache actionCache)
       throws InterruptedException {
 
     Set<SkyKey> roots = evaluator.getLatestTopLevelEvaluations();

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
@@ -4046,21 +4046,21 @@ public final void runSkyfocus(
       ImmutableSet<Label> topLevelTargets,
       ImmutableSet<PathFragment> projectDirectories,
       Reporter reporter,
-      ActionCache actionCache)
+      @Nullable ActionCache actionCache)
       throws InterruptedException {
     if (!skyfocusState.enabled() || topLevelTargets.isEmpty()) {
       return;
     }
 
     int beforeNodeCount = this.getEvaluator().getValues().size();
     long beforeHeap = 0;
-    long beforeActionCacheEntries = actionCache.size();
     if (skyfocusState.options().dumpPostGcStats) {
       // we have to gc once here to get an accurate reading on the exact work Skyfocus is
       // doing.
       System.gc();
       beforeHeap = getHeapSize();
     }
+    long beforeActionCacheEntries = actionCache == null ? 0 : actionCache.size();
 
     ImmutableMultiset<SkyFunctionName> skyFunctionCountBefore = ImmutableMultiset.of();
     InMemoryGraph graph = memoizingEvaluator.getInMemoryGraph();
@@ -4123,12 +4123,14 @@ public final void runSkyfocus(
           memoizingEvaluator.getValues().size(),
           Long::toString);
 
-      reportMetricChange(
-          reporter,
-          "Action cache count",
-          beforeActionCacheEntries,
-          actionCache.size(),
-          Long::toString);
+      if (actionCache != null) {
+        reportMetricChange(
+            reporter,
+            "Action cache count",
+            beforeActionCacheEntries,
+            actionCache.size(),
+            Long::toString);
+      }
     }
 
     if (skyfocusState.options().dumpPostGcStats) {

diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeFocuser.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeFocuser.java
@@ -36,6 +36,7 @@
 import java.util.Collection;
 import java.util.Set;
 import java.util.concurrent.atomic.AtomicLong;
+import javax.annotation.Nullable;
 
 /**
  * SkyframeFocuser is a minimizing optimizer (i.e. garbage collector) for the Skyframe graph, based
@@ -53,9 +54,10 @@ private static boolean isVerificationSetKeyType(SkyKey k) {
   // The in-memory Skyframe graph
   private final InMemoryGraph graph;
 
-  private final ActionCache actionCache;
+  // Can be null with --nouse_action_cache.
+  @Nullable private final ActionCache actionCache;
 
-  private SkyframeFocuser(InMemoryGraph graph, ActionCache actionCache) {
+  private SkyframeFocuser(InMemoryGraph graph, @Nullable ActionCache actionCache) {
     super(
         /* parallelism= */ Runtime.getRuntime().availableProcessors(),
         /* keepAliveTime= */ 2,
@@ -86,10 +88,7 @@ private SkyframeFocuser(InMemoryGraph graph, ActionCache actionCache) {
    * @return the set of kept SkyKeys in the in-memory graph, categorized by deps and rdeps.
    */
   public static FocusResult focus(
-      InMemoryGraph graph,
-      ActionCache actionCache,
-      Set<SkyKey> roots,
-      Set<SkyKey> leafs)
+      InMemoryGraph graph, @Nullable ActionCache actionCache, Set<SkyKey> roots, Set<SkyKey> leafs)
       throws InterruptedException {
     SkyframeFocuser focuser = new SkyframeFocuser(graph, actionCache);
     return focuser.run(roots, leafs);
@@ -399,7 +398,8 @@ private FocusResult run(Set<SkyKey> roots, Set<SkyKey> leafs) throws Interrupted
               return;
             }
 
-            if (inMemoryNodeEntry.getValue() instanceof ActionLookupValue alv) {
+            if (actionCache != null
+                && inMemoryNodeEntry.getValue() instanceof ActionLookupValue alv) {
               for (ActionAnalysisMetadata a : alv.getActions()) {
                 for (Artifact output : a.getOutputs()) {
                   actionCache.remove(output.getExecPathString());

diff --git a/src/test/java/com/google/devtools/build/lib/buildtool/SkyfocusIntegrationTest.java b/src/test/java/com/google/devtools/build/lib/buildtool/SkyfocusIntegrationTest.java
@@ -1092,4 +1092,23 @@ public void workingSet_expanded_withReanalysis() throws Exception {
     assertThat(getSkyframeExecutor().getSkyfocusState().workingSet()).hasSize(2);
   }
 
+  @Test
+  public void actionCache_canBeNull() throws Exception {
+    addOptions("--nouse_action_cache");
+    write("hello/x.txt", "x");
+    write(
+        "hello/BUILD",
+        """
+        genrule(
+            name = "target",
+            srcs = ["x.txt"],
+            outs = ["out"],
+            cmd = "cat $< > $@",
+        )
+        """);
+
+    buildTarget("//hello/..."); // does not crash.
+    assertThat(getSkyframeExecutor().getSkyfocusState().workingSetStrings())
+        .containsExactly("hello", "hello/BUILD", "hello/x.txt");
+  }
 }