Rename --keep_incrementality_data to --track_incremental_state.

New name clears the namespace a 2nd flag that will wipe the build graph after the build. The old name would be confusing as it could easily apply to that, and so needs to be more specifically just about tracking state in the first place. The new flag can be clearly separate and about keeping state after the build.

Partial roll forward of https://github.com/bazelbuild/bazel/commit/9321316b34767b06c3071b2cf2a4de189874fcce, with fixes to documentation that are still relevant.

RELNOTES: Rename --keep_incrementality_data to --track_incremental_state
PiperOrigin-RevId: 179078292

1 files changed, 36 insertions, 29 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SequencedSkyframeExecutor.java b/src/main/java/com/google/devtools/build/lib/skyframe/SequencedSkyframeExecutor.java
index f3e4bf4da0..3aa1ce8da1 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/SequencedSkyframeExecutor.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/SequencedSkyframeExecutor.java
@@ -107,17 +107,13 @@ public final class SequencedSkyframeExecutor extends SkyframeExecutor {
 
   private boolean lastAnalysisDiscarded = false;
 
-  private enum IncrementalState {
-    NORMAL,
-    CLEAR_EDGES_AND_ACTIONS
-  }
-
   /**
-   * If {@link IncrementalState#CLEAR_EDGES_AND_ACTIONS}, the graph will not store edges, saving
-   * memory but making subsequent builds not incremental. Also, each action will be dereferenced
-   * once it is executed, saving memory.
+   * If false, the graph will not store state useful for incremental builds, saving memory but
+   * leaving the graph un-reusable. Subsequent builds will therefore not be incremental.
+   *
+   * <p>Avoids storing edges entirely and dereferences each action after execution.
    */
-  private IncrementalState incrementalState = IncrementalState.NORMAL;
+  private boolean trackIncrementalState = true;
 
   private boolean evaluatorNeedsReset = false;
 
@@ -504,52 +500,63 @@ public final class SequencedSkyframeExecutor extends SkyframeExecutor {
         SkyFunctionName.functionIs(SkyFunctions.FILE_STATE)));
   }
 
+  /**
+   * {@inheritDoc}
+   *
+   * <p>Necessary conditions to not store graph edges are either
+   *
+   * <ol>
+   *   <li>batch (since incremental builds are not possible) and discard_analysis_cache (since
+   *       otherwise user isn't concerned about saving memory this way).
+   *   <li>track_incremental_state set to false.
+   * </ol>
+   */
   @Override
   public void decideKeepIncrementalState(
       boolean batch, OptionsProvider options, EventHandler eventHandler) {
     Preconditions.checkState(!active);
     BuildView.Options viewOptions = options.getOptions(BuildView.Options.class);
     BuildRequestOptions requestOptions = options.getOptions(BuildRequestOptions.class);
+    boolean oldState = trackIncrementalState;
+
+    // First check if the incrementality state should be kept around during the build.
     boolean explicitlyRequestedNoIncrementalData =
-        requestOptions != null && !requestOptions.keepIncrementalityData;
+        requestOptions != null && !requestOptions.trackIncrementalState;
     boolean implicitlyRequestedNoIncrementalData =
         batch && viewOptions != null && viewOptions.discardAnalysisCache;
-    boolean discardingEdges =
-        explicitlyRequestedNoIncrementalData || implicitlyRequestedNoIncrementalData;
+    trackIncrementalState =
+        !explicitlyRequestedNoIncrementalData && !implicitlyRequestedNoIncrementalData;
     if (explicitlyRequestedNoIncrementalData != implicitlyRequestedNoIncrementalData) {
       if (requestOptions != null && !explicitlyRequestedNoIncrementalData) {
         eventHandler.handle(
             Event.warn(
-                "--batch and --discard_analysis_cache specified, but --nokeep_incrementality_data "
+                "--batch and --discard_analysis_cache specified, but --notrack_incremental_state "
                     + "not specified: incrementality data is implicitly discarded, but you may need"
-                    + " to specify --nokeep_incrementality_data in the future if you want to "
+                    + " to specify --notrack_incremental_state in the future if you want to "
                     + "maximize memory savings."));
       }
       if (!batch) {
         eventHandler.handle(
             Event.warn(
-                "--batch not specified with --nokeep_incrementality_data: the server will "
+                "--batch not specified with --notrack_incremental_state: the server will "
                     + "remain running, but the next build will not be incremental on this one."));
       }
     }
-    IncrementalState oldState = incrementalState;
-    incrementalState =
-        discardingEdges ? IncrementalState.CLEAR_EDGES_AND_ACTIONS : IncrementalState.NORMAL;
-    if (oldState != incrementalState) {
-      logger.info("Set incremental state to " + incrementalState);
+
+    // Now check if it is necessary to wipe the previous state. We do this if either the previous
+    // or current incrementalStateRetentionStrategy requires the build to have been isolated.
+    if (oldState != trackIncrementalState) {
+      logger.info("Set incremental state to " + trackIncrementalState);
       evaluatorNeedsReset = true;
-      removeActionsAfterEvaluation.set(
-          incrementalState == IncrementalState.CLEAR_EDGES_AND_ACTIONS);
-    } else if (incrementalState == IncrementalState.CLEAR_EDGES_AND_ACTIONS) {
+      removeActionsAfterEvaluation.set(!trackIncrementalState);
+    } else if (!trackIncrementalState) {
       evaluatorNeedsReset = true;
     }
   }
 
   @Override
-  public boolean hasIncrementalState() {
-    // TODO(bazel-team): Combine this method with clearSkyframeRelevantCaches() once legacy
-    // execution is removed [skyframe-execution].
-    return incrementalState == IncrementalState.NORMAL;
+  public boolean tracksStateForIncrementality() {
+    return trackIncrementalState;
   }
 
   @Override
@@ -618,7 +625,7 @@ public final class SequencedSkyframeExecutor extends SkyframeExecutor {
    * phase. Instead, their analysis-time data is cleared while preserving the generating action info
    * needed for execution. The next build will delete the nodes (and recreate them if necessary).
    *
-   * <p>If {@link #hasIncrementalState} is false, then also delete loading-phase nodes (as
+   * <p>If {@link #tracksStateForIncrementality} is false, then also delete loading-phase nodes (as
    * determined by {@link #LOADING_TYPES}) from the graph, since there will be no future builds to
    * use them for.
    */
@@ -638,7 +645,7 @@ public final class SequencedSkyframeExecutor extends SkyframeExecutor {
         }
         SkyKey key = keyAndEntry.getKey();
         SkyFunctionName functionName = key.functionName();
-        if (!hasIncrementalState() && LOADING_TYPES.contains(functionName)) {
+        if (!tracksStateForIncrementality() && LOADING_TYPES.contains(functionName)) {
           it.remove();
           continue;
         }