Move --track_incremental_state and --keep_state_after_build to CommonCommandOptions to better facilitate certain kinds of benchmarking.

PiperOrigin-RevId: 196695342

6 files changed, 48 insertions, 51 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/buildtool/BuildRequestOptions.java b/src/main/java/com/google/devtools/build/lib/buildtool/BuildRequestOptions.java
index cd4e86ddad..f5701cb5d0 100644
--- a/src/main/java/com/google/devtools/build/lib/buildtool/BuildRequestOptions.java
+++ b/src/main/java/com/google/devtools/build/lib/buildtool/BuildRequestOptions.java
@@ -379,31 +379,6 @@ public class BuildRequestOptions extends OptionsBase {
   public boolean useActionCache;
 
   @Option(
-    name = "track_incremental_state",
-    oldName = "keep_incrementality_data",
-    defaultValue = "true",
-    documentationCategory = OptionDocumentationCategory.BUILD_TIME_OPTIMIZATION,
-    effectTags = {OptionEffectTag.LOSES_INCREMENTAL_STATE},
-    help =
-        "If false, Blaze will not persist data that allows for invalidation and re-evaluation "
-            + "on incremental builds in order to save memory on this build. Subsequent builds "
-            + "will not have any incrementality with respect to this one. Usually you will want "
-            + "to specify --batch when setting this to false."
-  )
-  public boolean trackIncrementalState;
-
-  @Option(
-    name = "keep_state_after_build",
-    defaultValue = "true",
-    documentationCategory = OptionDocumentationCategory.BUILD_TIME_OPTIMIZATION,
-    effectTags = {OptionEffectTag.LOSES_INCREMENTAL_STATE},
-    help =
-        "If false, Blaze will discard the inmemory state from this build when the build finishes. "
-            + "Subsequent builds will not have any incrementality with respect to this one."
-  )
-  public boolean keepStateAfterBuild;
-
-  @Option(
       name = "discard_actions_after_execution",
       defaultValue = "true",
       documentationCategory = OptionDocumentationCategory.UNDOCUMENTED,
diff --git a/src/main/java/com/google/devtools/build/lib/runtime/BlazeRuntime.java b/src/main/java/com/google/devtools/build/lib/runtime/BlazeRuntime.java
index cdad1e53b4..9ccef65167 100644
--- a/src/main/java/com/google/devtools/build/lib/runtime/BlazeRuntime.java
+++ b/src/main/java/com/google/devtools/build/lib/runtime/BlazeRuntime.java
@@ -32,7 +32,6 @@ import com.google.devtools.build.lib.analysis.config.BuildOptions;
 import com.google.devtools.build.lib.analysis.config.ConfigurationFragmentFactory;
 import com.google.devtools.build.lib.analysis.test.CoverageReportActionFactory;
 import com.google.devtools.build.lib.buildeventstream.PathConverter;
-import com.google.devtools.build.lib.buildtool.BuildRequestOptions;
 import com.google.devtools.build.lib.clock.BlazeClock;
 import com.google.devtools.build.lib.clock.Clock;
 import com.google.devtools.build.lib.events.Event;
@@ -473,12 +472,12 @@ public final class BlazeRuntime {
       module.afterCommand();
     }
 
-    // If the command just completed was or inherits from Build, wipe the dependency graph if
-    // requested. This is sufficient, as this method is always run at the end of commands unless
-    // the server crashes, in which case no inmemory state will linger for the next build anyway.
-    BuildRequestOptions buildRequestOptions =
-        env.getOptions().getOptions(BuildRequestOptions.class);
-    if (buildRequestOptions != null && !buildRequestOptions.keepStateAfterBuild) {
+    // Wipe the dependency graph if requested. Note that this method always runs at the end of
+    // a commands unless the server crashes, in which case no inmemory state will linger for the
+    // next build anyway.
+    CommonCommandOptions commonOptions =
+        Preconditions.checkNotNull(env.getOptions().getOptions(CommonCommandOptions.class));
+    if (!commonOptions.keepStateAfterBuild) {
       workspace.getSkyframeExecutor().resetEvaluator();
     }
 
diff --git a/src/main/java/com/google/devtools/build/lib/runtime/CommandEnvironment.java b/src/main/java/com/google/devtools/build/lib/runtime/CommandEnvironment.java
index f75f06a0b4..3e6343bcb0 100644
--- a/src/main/java/com/google/devtools/build/lib/runtime/CommandEnvironment.java
+++ b/src/main/java/com/google/devtools/build/lib/runtime/CommandEnvironment.java
@@ -21,9 +21,11 @@ import com.google.common.eventbus.EventBus;
 import com.google.devtools.build.lib.actions.PackageRootResolver;
 import com.google.devtools.build.lib.actions.cache.ActionCache;
 import com.google.devtools.build.lib.analysis.BlazeDirectories;
+import com.google.devtools.build.lib.analysis.BuildView.Options;
 import com.google.devtools.build.lib.analysis.SkyframePackageRootResolver;
 import com.google.devtools.build.lib.analysis.config.BuildConfiguration;
 import com.google.devtools.build.lib.analysis.config.DefaultsPackage;
+import com.google.devtools.build.lib.buildtool.BuildRequestOptions;
 import com.google.devtools.build.lib.cmdline.Label;
 import com.google.devtools.build.lib.events.Reporter;
 import com.google.devtools.build.lib.packages.NoSuchThingException;
@@ -615,9 +617,13 @@ public final class CommandEnvironment {
     // Fail fast in the case where a Blaze command forgets to install the package path correctly.
     skyframeExecutor.setActive(false);
     // Let skyframe figure out how much incremental state it will be keeping.
+    Options viewOptions = options.getOptions(Options.class);
+    BuildRequestOptions requestOptions = options.getOptions(BuildRequestOptions.class);
     skyframeExecutor.decideKeepIncrementalState(
         runtime.getStartupOptionsProvider().getOptions(BlazeServerStartupOptions.class).batch,
-        options,
+        commonOptions.keepStateAfterBuild, commonOptions.trackIncrementalState,
+        viewOptions != null && viewOptions.discardAnalysisCache,
+        requestOptions != null && requestOptions.discardActionsAfterExecution,
         reporter);
 
     // Start the performance and memory profilers.
diff --git a/src/main/java/com/google/devtools/build/lib/runtime/CommonCommandOptions.java b/src/main/java/com/google/devtools/build/lib/runtime/CommonCommandOptions.java
index 8d520e90cf..6e9ba86492 100644
--- a/src/main/java/com/google/devtools/build/lib/runtime/CommonCommandOptions.java
+++ b/src/main/java/com/google/devtools/build/lib/runtime/CommonCommandOptions.java
@@ -372,4 +372,29 @@ public class CommonCommandOptions extends OptionsBase {
   )
   public List<String> deprecationWarnings;
 
+  @Option(
+      name = "track_incremental_state",
+      oldName = "keep_incrementality_data",
+      defaultValue = "true",
+      documentationCategory = OptionDocumentationCategory.BUILD_TIME_OPTIMIZATION,
+      effectTags = {OptionEffectTag.LOSES_INCREMENTAL_STATE},
+      help =
+          "If false, Blaze will not persist data that allows for invalidation and re-evaluation "
+              + "on incremental builds in order to save memory on this build. Subsequent builds "
+              + "will not have any incrementality with respect to this one. Usually you will want "
+              + "to specify --batch when setting this to false."
+  )
+  public boolean trackIncrementalState;
+
+  @Option(
+      name = "keep_state_after_build",
+      defaultValue = "true",
+      documentationCategory = OptionDocumentationCategory.BUILD_TIME_OPTIMIZATION,
+      effectTags = {OptionEffectTag.LOSES_INCREMENTAL_STATE},
+      help =
+          "If false, Blaze will discard the inmemory state from this build when the build "
+              + "finishes. Subsequent builds will not have any incrementality with respect to this "
+              + "one."
+  )
+  public boolean keepStateAfterBuild;
 }
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 aaeed3f73f..0b371780ca 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
@@ -31,13 +31,11 @@ import com.google.devtools.build.lib.actions.ArtifactRoot;
 import com.google.devtools.build.lib.actions.CommandLineExpansionException;
 import com.google.devtools.build.lib.analysis.AnalysisProtos.ActionGraphContainer;
 import com.google.devtools.build.lib.analysis.BlazeDirectories;
-import com.google.devtools.build.lib.analysis.BuildView;
 import com.google.devtools.build.lib.analysis.ConfiguredTarget;
 import com.google.devtools.build.lib.analysis.WorkspaceStatusAction.Factory;
 import com.google.devtools.build.lib.analysis.buildinfo.BuildInfoFactory;
 import com.google.devtools.build.lib.analysis.config.BuildOptions;
 import com.google.devtools.build.lib.analysis.configuredtargets.RuleConfiguredTarget;
-import com.google.devtools.build.lib.buildtool.BuildRequestOptions;
 import com.google.devtools.build.lib.cmdline.PackageIdentifier;
 import com.google.devtools.build.lib.concurrent.Uninterruptibles;
 import com.google.devtools.build.lib.events.Event;
@@ -90,7 +88,6 @@ import com.google.devtools.build.skyframe.SkyFunctionName;
 import com.google.devtools.build.skyframe.SkyKey;
 import com.google.devtools.build.skyframe.SkyValue;
 import com.google.devtools.common.options.OptionsClassProvider;
-import com.google.devtools.common.options.OptionsProvider;
 import java.io.PrintStream;
 import java.util.ArrayList;
 import java.util.Collection;
@@ -579,22 +576,19 @@ public final class SequencedSkyframeExecutor extends SkyframeExecutor {
    */
   @Override
   public void decideKeepIncrementalState(
-      boolean batch, OptionsProvider options, EventHandler eventHandler) {
+      boolean batch, boolean keepStateAfterBuild, boolean shouldTrackIncrementalState,
+      boolean discardAnalysisCache, boolean discardActionsAfterExecution,
+      EventHandler eventHandler) {
     Preconditions.checkState(!active);
-    BuildView.Options viewOptions = options.getOptions(BuildView.Options.class);
-    BuildRequestOptions requestOptions = options.getOptions(BuildRequestOptions.class);
     boolean oldValueOfTrackIncrementalState = trackIncrementalState;
 
     // First check if the incrementality state should be kept around during the build.
-    boolean explicitlyRequestedNoIncrementalData =
-        requestOptions != null && !requestOptions.trackIncrementalState;
-    boolean implicitlyRequestedNoIncrementalData =
-        batch && viewOptions != null && viewOptions.discardAnalysisCache;
+    boolean explicitlyRequestedNoIncrementalData = !shouldTrackIncrementalState;
+    boolean implicitlyRequestedNoIncrementalData = (batch && discardAnalysisCache);
     trackIncrementalState =
         !explicitlyRequestedNoIncrementalData && !implicitlyRequestedNoIncrementalData;
-    boolean keepStateAfterBuild = requestOptions != null && requestOptions.keepStateAfterBuild;
     if (explicitlyRequestedNoIncrementalData != implicitlyRequestedNoIncrementalData) {
-      if (requestOptions != null && !explicitlyRequestedNoIncrementalData) {
+      if (!explicitlyRequestedNoIncrementalData) {
         eventHandler.handle(
             Event.warn(
                 "--batch and --discard_analysis_cache specified, but --notrack_incremental_state "
@@ -612,10 +606,7 @@ public final class SequencedSkyframeExecutor extends SkyframeExecutor {
       }
     }
 
-    removeActionsAfterEvaluation.set(
-        !trackIncrementalState
-            && requestOptions != null
-            && requestOptions.discardActionsAfterExecution);
+    removeActionsAfterEvaluation.set(!trackIncrementalState && discardActionsAfterExecution);
     // 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 (oldValueOfTrackIncrementalState != trackIncrementalState) {
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
index 47f51a0e60..d21b239197 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java
@@ -161,7 +161,6 @@ import com.google.devtools.build.skyframe.SkyKey;
 import com.google.devtools.build.skyframe.SkyValue;
 import com.google.devtools.build.skyframe.WalkableGraph.WalkableGraphFactory;
 import com.google.devtools.common.options.OptionsClassProvider;
-import com.google.devtools.common.options.OptionsProvider;
 import java.io.PrintStream;
 import java.util.ArrayList;
 import java.util.Collection;
@@ -757,7 +756,9 @@ public abstract class SkyframeExecutor implements WalkableGraphFactory {
    * longer.
    */
   public void decideKeepIncrementalState(
-      boolean batch, OptionsProvider viewOptions, EventHandler eventHandler) {
+      boolean batch, boolean keepStateAfterBuild, boolean trackIncrementalState,
+      boolean discardAnalysisCache, boolean discardActionsAfterExecution,
+      EventHandler eventHandler) {
     // Assume incrementality.
   }