[Skymeld] Make --notrack_incremental_state work with Skymeld.

These 2 facts were incompatible:

* --notrack_incremental_state does not keep any edge in the graph.
* Skymeld relies exclusively on graph traversal of the transitive closure of a top level ActionLookupValue to collect the ALVs for conflict checking.

This CL allows Skymeld to utilize the collection of ALVs present in the graph at the point of conflict checking, similar to how it's done in regular blaze. However, it introduces an optimization to keep the performance reasonable:

* [Brute force] For each conflict check, iterate through all the nodes in the graph, return those with ALVs. This has O(num_top_level_alv x num_nodes_in_graph) time complexity.
* [Optimized] For each conflict check, return only the evaluated ALVs since the last conflict check. This has O(num_top_level_alv + num_nodes_in_graph) time complexity.

PiperOrigin-RevId: 502849785
Change-Id: I8b99845a23999a34afc84041bd509a6cc57affd2

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

@@ -177,6 +177,7 @@ public void buildTargets(BuildRequest request, BuildResult result, TargetValidat
         return;
       }
 
+      env.getSkyframeExecutor().setMergedSkyframeAnalysisExecution(false);
       AnalysisResult analysisResult =
           AnalysisPhaseRunner.execute(env, request, buildOptions, validator);
 
@@ -309,7 +310,7 @@ private void buildTargetsWithMergedAnalysisExecution(
       throws InterruptedException, TargetParsingException, LoadingFailedException,
           AbruptExitException, ViewCreationFailedException, BuildFailedException, TestExecException,
           InvalidConfigurationException, RepositoryMappingResolutionException {
-
+    env.getSkyframeExecutor().setMergedSkyframeAnalysisExecution(true);
     // Target pattern evaluation.
     TargetPatternPhaseValue loadingResult;
     Profiler.instance().markPhase(ProfilePhase.TARGET_PATTERN_EVAL);

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

@@ -228,6 +228,7 @@
 import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Map;
+import java.util.Map.Entry;
 import java.util.Optional;
 import java.util.Set;
 import java.util.UUID;
@@ -243,6 +244,7 @@
 import java.util.function.Consumer;
 import java.util.function.Supplier;
 import javax.annotation.Nullable;
+import javax.annotation.concurrent.GuardedBy;
 import net.starlark.java.eval.StarlarkSemantics;
 
 /**
@@ -379,6 +381,8 @@ public abstract class SkyframeExecutor implements WalkableGraphFactory, Configur
   private RemoteOutputsMode lastRemoteOutputsMode;
   private Boolean lastRemoteCacheEnabled;
 
+  private boolean mergedSkyframeAnalysisExecution = false;
+
   // Reset after each build.
   private IncrementalArtifactConflictFinder incrementalArtifactConflictFinder;
   // Reset after each build.
@@ -1406,6 +1410,11 @@ public SkyframeBuildView getSkyframeBuildView() {
     return skyframeBuildView;
   }
 
+  /** Sets whether this build is done with --experimental_merged_skyframe_analysis_execution. */
+  public void setMergedSkyframeAnalysisExecution(boolean mergedSkyframeAnalysisExecution) {
+    this.mergedSkyframeAnalysisExecution = mergedSkyframeAnalysisExecution;
+  }
+
   /** Sets the eventBus to use for posting events. */
   public void setEventBus(@Nullable EventBus eventBus) {
     this.eventBus.set(eventBus);
@@ -3049,6 +3058,15 @@ protected class SkyframeProgressReceiver implements EvaluationProgressReceiver {
     /** This receiver is only needed for execution, so it is null otherwise. */
     @Nullable EvaluationProgressReceiver executionProgressReceiver = null;
 
+    /**
+     * As the ActionLookupValues are marked done in the graph, put it in the map. This map will be
+     * "swapped out" for a new one each time it's requested via {@link
+     * #getBatchedActionLookupValuesForConflictChecking()}
+     */
+    @GuardedBy("this")
+    private Map<SkyKey, SkyValue> batchedActionLookupValuesForConflictChecking =
+        Maps.newConcurrentMap();
+
     @Override
     public void invalidated(SkyKey skyKey, InvalidationState state) {
       if (ignoreInvalidations) {
@@ -3109,6 +3127,15 @@ public void evaluated(
         }
       }
 
+      if (mergedSkyframeAnalysisExecution
+          && !tracksStateForIncrementality()
+          && skyKey instanceof ActionLookupKey
+          && newValue != null) {
+        synchronized (this) {
+          batchedActionLookupValuesForConflictChecking.put(skyKey, newValue);
+        }
+      }
+
       if (EvaluationState.BUILT.equals(state)) {
         skyKeyStateReceiver.evaluated(skyKey);
       }
@@ -3123,6 +3150,23 @@ public void evaluated(
             skyKey, newValue, newError, evaluationSuccessState, state);
       }
     }
+
+    /**
+     * Returns the SkyKey -> SkyValue map of done ActionLookupValues since the last time this method
+     * was called. Replaces {@link #batchedActionLookupValuesForConflictChecking} with a new empty
+     * map.
+     *
+     * <p>This is only used in skymeld mode AND when we don't keep the incremental state.
+     */
+    public ImmutableMap<SkyKey, SkyValue> getBatchedActionLookupValuesForConflictChecking() {
+      Preconditions.checkState(mergedSkyframeAnalysisExecution && !tracksStateForIncrementality());
+      synchronized (this) {
+        ImmutableMap<SkyKey, SkyValue> result =
+            ImmutableMap.copyOf(batchedActionLookupValuesForConflictChecking);
+        batchedActionLookupValuesForConflictChecking = Maps.newConcurrentMap();
+        return result;
+      }
+    }
   }
 
   public final ExecutionFinishedEvent createExecutionFinishedEvent() {
@@ -3174,12 +3218,23 @@ private ActionLookupValuesCollectionResult collectTransitiveActionLookupValuesOf
     try (SilentCloseable c =
         Profiler.instance().profile("SkyframeExecutor.collectTransitiveActionLookupValuesOfKey")) {
       ActionLookupValuesTraversal result = new ActionLookupValuesTraversal();
-      Map<ActionLookupKey, SkyValue> foundTransitiveActionLookupEntities =
-          incrementalTransitiveActionLookupKeysCollector.collect(key);
-      foundTransitiveActionLookupEntities.forEach(result::accumulate);
+      if (tracksStateForIncrementality()) {
+        Map<ActionLookupKey, SkyValue> foundTransitiveActionLookupEntities =
+            incrementalTransitiveActionLookupKeysCollector.collect(key);
+        foundTransitiveActionLookupEntities.forEach(result::accumulate);
+        return ActionLookupValuesCollectionResult.create(
+            result.getActionLookupValueShards(),
+            ImmutableSet.copyOf(foundTransitiveActionLookupEntities.keySet()));
+      }
+      // No graph edges available when there's no incrementality. We get the ALVs collected
+      // since the last time this method was called.
+      ImmutableMap<SkyKey, SkyValue> batchOfActionLookupValues =
+          progressReceiver.getBatchedActionLookupValuesForConflictChecking();
+      for (Entry<SkyKey, SkyValue> entry : batchOfActionLookupValues.entrySet()) {
+        result.accumulate((ActionLookupKey) entry.getKey(), entry.getValue());
+      }
       return ActionLookupValuesCollectionResult.create(
-          result.getActionLookupValueShards(),
-          ImmutableSet.copyOf(foundTransitiveActionLookupEntities.keySet()));
+          result.getActionLookupValueShards(), batchOfActionLookupValues.keySet());
     }
   }
 

src/test/java/com/google/devtools/build/lib/buildtool/OutputArtifactConflictTest.java

@@ -734,4 +734,22 @@ public void dependencyHasConflict_keepGoing_bothTopLevelTargetsFail(
     assertThat(eventListener.failedTargetNames)
         .containsExactly("//foo:top_level_a", "//foo:top_level_b");
   }
+
+  @Test
+  public void conflict_noTrackIncrementalState_detected(
+      @TestParameter boolean mergedAnalysisExecution) throws Exception {
+    addOptions("--experimental_merged_skyframe_analysis_execution=" + mergedAnalysisExecution);
+    addOptions("--notrack_incremental_state");
+    writeConflictBzl();
+    write(
+        "foo/BUILD",
+        "load('//foo:conflict.bzl', 'my_rule')",
+        "my_rule(name = 'first')",
+        "my_rule(name = 'second')");
+
+    assertThrows(
+        ViewCreationFailedException.class, () -> buildTarget("//foo:first", "//foo:second"));
+    events.assertContainsError(
+        "file 'foo/conflict_output' is generated by these conflicting actions:");
+  }
 }