Consolidate instances of the --loading_phase_threads flag.

RELNOTES: None.
PiperOrigin-RevId: 179838936

11 files changed, 108 insertions, 37 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/BUILD b/src/main/java/com/google/devtools/build/lib/BUILD
index 27d97e173c..736c3278fb 100644
--- a/src/main/java/com/google/devtools/build/lib/BUILD
+++ b/src/main/java/com/google/devtools/build/lib/BUILD
@@ -989,6 +989,12 @@ java_library(
 )
 
 java_library(
+    name = "loading-phase-threads-option",
+    srcs = ["runtime/LoadingPhaseThreadsOption.java"],
+    deps = ["//src/main/java/com/google/devtools/common/options"],
+)
+
+java_library(
     name = "runtime",
     srcs = glob(
         [
@@ -1002,11 +1008,13 @@ java_library(
         exclude = [
             "buildtool/BuildRequestOptions.java",
             "runtime/KeepGoingOption.java",
+            "runtime/LoadingPhaseThreadsOption.java",
         ],
     ),
     deps = [
         ":build-request-options",
         ":keep-going-option",
+        ":loading-phase-threads-option",
         "//src/main/java/com/google/devtools/build/docgen:docgen_javalib",
         "//src/main/java/com/google/devtools/build/lib:build-base",
         "//src/main/java/com/google/devtools/build/lib:events",
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/BuildView.java b/src/main/java/com/google/devtools/build/lib/analysis/BuildView.java
index 8261c7c7ac..816dba9d43 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/BuildView.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/BuildView.java
@@ -167,17 +167,6 @@ public class BuildView {
    */
   public static class Options extends OptionsBase {
     @Option(
-      name = "loading_phase_threads",
-      defaultValue = "-1",
-      category = "what",
-      converter = LoadingPhaseThreadCountConverter.class,
-      documentationCategory = OptionDocumentationCategory.UNCATEGORIZED,
-      effectTags = {OptionEffectTag.UNKNOWN},
-      help = "Number of parallel threads to use for the loading/analysis phase."
-    )
-    public int loadingPhaseThreads;
-
-    @Option(
       name = "analysis_warnings_as_errors",
       deprecationWarning =
           "analysis_warnings_as_errors is now a no-op and will be removed in"
@@ -465,6 +454,7 @@ public class BuildView {
       List<String> aspects,
       Options viewOptions,
       boolean keepGoing,
+      int loadingPhaseThreads,
       TopLevelArtifactContext topLevelOptions,
       ExtendedEventHandler eventHandler,
       EventBus eventBus)
@@ -579,12 +569,7 @@ public class BuildView {
     try {
       skyframeAnalysisResult =
           skyframeBuildView.configureTargets(
-              eventHandler,
-              topLevelCtKeys,
-              aspectKeys,
-              eventBus,
-              keepGoing,
-              viewOptions.loadingPhaseThreads);
+              eventHandler, topLevelCtKeys, aspectKeys, eventBus, keepGoing, loadingPhaseThreads);
       setArtifactRoots(skyframeAnalysisResult.getPackageRoots());
     } finally {
       skyframeBuildView.clearInvalidatedConfiguredTargets();
diff --git a/src/main/java/com/google/devtools/build/lib/buildtool/BuildRequest.java b/src/main/java/com/google/devtools/build/lib/buildtool/BuildRequest.java
index 9553731fac..35328c1908 100644
--- a/src/main/java/com/google/devtools/build/lib/buildtool/BuildRequest.java
+++ b/src/main/java/com/google/devtools/build/lib/buildtool/BuildRequest.java
@@ -30,6 +30,7 @@ import com.google.devtools.build.lib.pkgcache.LoadingOptions;
 import com.google.devtools.build.lib.pkgcache.PackageCacheOptions;
 import com.google.devtools.build.lib.runtime.BlazeCommandEventHandler;
 import com.google.devtools.build.lib.runtime.KeepGoingOption;
+import com.google.devtools.build.lib.runtime.LoadingPhaseThreadsOption;
 import com.google.devtools.build.lib.util.OptionsUtils;
 import com.google.devtools.build.lib.util.io.OutErr;
 import com.google.devtools.common.options.OptionsBase;
@@ -76,7 +77,8 @@ public class BuildRequest implements OptionsClassProvider {
           LoadingOptions.class,
           BuildView.Options.class,
           ExecutionOptions.class,
-          KeepGoingOption.class);
+          KeepGoingOption.class,
+          LoadingPhaseThreadsOption.class);
 
   private BuildRequest(String commandName,
                        final OptionsProvider options,
@@ -209,6 +211,10 @@ public class BuildRequest implements OptionsClassProvider {
     return getOptions(KeepGoingOption.class).keepGoing;
   }
 
+  /** Returns the value of the --loading_phase_threads option. */
+  int getLoadingPhaseThreadCount() {
+    return getOptions(LoadingPhaseThreadsOption.class).threads;
+  }
   /**
    * Returns the set of execution options specified for this request.
    */
diff --git a/src/main/java/com/google/devtools/build/lib/buildtool/BuildTool.java b/src/main/java/com/google/devtools/build/lib/buildtool/BuildTool.java
index a16f3d0942..42f53d6e9a 100644
--- a/src/main/java/com/google/devtools/build/lib/buildtool/BuildTool.java
+++ b/src/main/java/com/google/devtools/build/lib/buildtool/BuildTool.java
@@ -561,6 +561,7 @@ public final class BuildTool {
             request.getAspects(),
             request.getViewOptions(),
             request.getKeepGoing(),
+            request.getLoadingPhaseThreadCount(),
             request.getTopLevelArtifactContext(),
             env.getReporter(),
             env.getEventBus());
diff --git a/src/main/java/com/google/devtools/build/lib/query2/output/QueryOptions.java b/src/main/java/com/google/devtools/build/lib/query2/output/QueryOptions.java
index 0ce1ab14ea..328c8e71b8 100644
--- a/src/main/java/com/google/devtools/build/lib/query2/output/QueryOptions.java
+++ b/src/main/java/com/google/devtools/build/lib/query2/output/QueryOptions.java
@@ -131,15 +131,6 @@ public class QueryOptions extends OptionsBase {
   public OrderOutput orderOutput;
 
   @Option(
-    name = "loading_phase_threads",
-    defaultValue = "200",
-    documentationCategory = OptionDocumentationCategory.EXECUTION_STRATEGY,
-    effectTags = {OptionEffectTag.BAZEL_INTERNAL_CONFIGURATION},
-    help = "Number of parallel threads to use for the loading phase."
-  )
-  public int loadingPhaseThreads;
-
-  @Option(
     name = "host_deps",
     defaultValue = "true",
     category = "query",
diff --git a/src/main/java/com/google/devtools/build/lib/runtime/LoadingPhaseThreadsOption.java b/src/main/java/com/google/devtools/build/lib/runtime/LoadingPhaseThreadsOption.java
new file mode 100644
index 0000000000..75f10006f4
--- /dev/null
+++ b/src/main/java/com/google/devtools/build/lib/runtime/LoadingPhaseThreadsOption.java
@@ -0,0 +1,70 @@
+// Copyright 2017 The Bazel Authors. All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//    http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+package com.google.devtools.build.lib.runtime;
+
+import com.google.devtools.common.options.Converter;
+import com.google.devtools.common.options.Option;
+import com.google.devtools.common.options.OptionDocumentationCategory;
+import com.google.devtools.common.options.OptionEffectTag;
+import com.google.devtools.common.options.OptionsBase;
+import com.google.devtools.common.options.OptionsParsingException;
+
+/** Defines the --loading_phase_threads option which is used by multiple commands. */
+public class LoadingPhaseThreadsOption extends OptionsBase {
+  @Option(
+    name = "loading_phase_threads",
+    defaultValue = LoadingPhaseThreadCountConverter.DEFAULT_VALUE,
+    documentationCategory = OptionDocumentationCategory.EXECUTION_STRATEGY,
+    effectTags = {OptionEffectTag.BAZEL_INTERNAL_CONFIGURATION},
+    converter = LoadingPhaseThreadCountConverter.class,
+    help = "Number of parallel threads to use for the loading/analysis phase."
+  )
+  public int threads;
+
+  /**
+   * A converter for loading phase thread count. Since the default is not a true constant, we create
+   * a converter here to implement the default logic.
+   */
+  public static final class LoadingPhaseThreadCountConverter implements Converter<Integer> {
+
+    private static final String DEFAULT_VALUE = "default";
+    // Reduce thread count while running tests. Test cases are typically small, and large thread
+    // pools vying for a relatively small number of CPU cores may induce non-optimal
+    // performance.
+    private static final int NON_TEST_THREADS = 200;
+    private static final int TEST_THREADS = 20;
+
+    @Override
+    public Integer convert(String input) throws OptionsParsingException {
+      if (DEFAULT_VALUE.equals(input)) {
+        return System.getenv("TEST_TMPDIR") == null ? NON_TEST_THREADS : TEST_THREADS;
+      }
+
+      try {
+        int result = Integer.decode(input);
+        if (result < 1) {
+          throw new OptionsParsingException("'" + input + "' must be at least 1");
+        }
+        return result;
+      } catch (NumberFormatException e) {
+        throw new OptionsParsingException("'" + input + "' is not an int");
+      }
+    }
+
+    @Override
+    public String getTypeDescription() {
+      return "an integer";
+    }
+  }
+}
diff --git a/src/main/java/com/google/devtools/build/lib/runtime/commands/BuildCommand.java b/src/main/java/com/google/devtools/build/lib/runtime/commands/BuildCommand.java
index d5315eaf38..a02c991839 100644
--- a/src/main/java/com/google/devtools/build/lib/runtime/commands/BuildCommand.java
+++ b/src/main/java/com/google/devtools/build/lib/runtime/commands/BuildCommand.java
@@ -26,6 +26,7 @@ import com.google.devtools.build.lib.runtime.BlazeRuntime;
 import com.google.devtools.build.lib.runtime.Command;
 import com.google.devtools.build.lib.runtime.CommandEnvironment;
 import com.google.devtools.build.lib.runtime.KeepGoingOption;
+import com.google.devtools.build.lib.runtime.LoadingPhaseThreadsOption;
 import com.google.devtools.build.lib.util.ExitCode;
 import com.google.devtools.common.options.OptionsParser;
 import com.google.devtools.common.options.OptionsProvider;
@@ -45,7 +46,8 @@ import java.util.List;
     PackageCacheOptions.class,
     BuildView.Options.class,
     LoadingOptions.class,
-    KeepGoingOption.class
+    KeepGoingOption.class,
+    LoadingPhaseThreadsOption.class
   },
   usesConfigurationOptions = true,
   shortDescription = "Builds the specified targets.",
diff --git a/src/main/java/com/google/devtools/build/lib/runtime/commands/QueryCommand.java b/src/main/java/com/google/devtools/build/lib/runtime/commands/QueryCommand.java
index dde4da7800..d34ae7b4a5 100644
--- a/src/main/java/com/google/devtools/build/lib/runtime/commands/QueryCommand.java
+++ b/src/main/java/com/google/devtools/build/lib/runtime/commands/QueryCommand.java
@@ -40,6 +40,7 @@ import com.google.devtools.build.lib.runtime.BlazeRuntime;
 import com.google.devtools.build.lib.runtime.Command;
 import com.google.devtools.build.lib.runtime.CommandEnvironment;
 import com.google.devtools.build.lib.runtime.KeepGoingOption;
+import com.google.devtools.build.lib.runtime.LoadingPhaseThreadsOption;
 import com.google.devtools.build.lib.util.AbruptExitException;
 import com.google.devtools.build.lib.util.ExitCode;
 import com.google.devtools.build.lib.vfs.FileSystemUtils;
@@ -57,7 +58,12 @@ import java.util.Set;
 /** Command line wrapper for executing a query with blaze. */
 @Command(
   name = "query",
-  options = {PackageCacheOptions.class, QueryOptions.class, KeepGoingOption.class},
+  options = {
+    PackageCacheOptions.class,
+    QueryOptions.class,
+    KeepGoingOption.class,
+    LoadingPhaseThreadsOption.class
+  },
   help = "resource:query.txt",
   shortDescription = "Executes a dependency graph query.",
   allowResidue = true,
@@ -137,7 +143,7 @@ public final class QueryCommand implements BlazeCommand {
             options.getOptions(KeepGoingOption.class).keepGoing,
             !streamResults,
             queryOptions.universeScope,
-            queryOptions.loadingPhaseThreads,
+            options.getOptions(LoadingPhaseThreadsOption.class).threads,
             settings);
     QueryExpression expr;
     try {
diff --git a/src/test/java/com/google/devtools/build/lib/BUILD b/src/test/java/com/google/devtools/build/lib/BUILD
index d9b1af55bc..1309e1951c 100644
--- a/src/test/java/com/google/devtools/build/lib/BUILD
+++ b/src/test/java/com/google/devtools/build/lib/BUILD
@@ -383,6 +383,7 @@ java_library(
         "//src/main/java/com/google/devtools/build/lib:java-compilation",
         "//src/main/java/com/google/devtools/build/lib:java-rules",
         "//src/main/java/com/google/devtools/build/lib:keep-going-option",
+        "//src/main/java/com/google/devtools/build/lib:loading-phase-threads-option",
         "//src/main/java/com/google/devtools/build/lib:packages",
         "//src/main/java/com/google/devtools/build/lib:proto-rules",
         "//src/main/java/com/google/devtools/build/lib:python-rules",
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/util/AnalysisTestCase.java b/src/test/java/com/google/devtools/build/lib/analysis/util/AnalysisTestCase.java
index f00e30f2c5..93e88ea1df 100644
--- a/src/test/java/com/google/devtools/build/lib/analysis/util/AnalysisTestCase.java
+++ b/src/test/java/com/google/devtools/build/lib/analysis/util/AnalysisTestCase.java
@@ -56,6 +56,7 @@ import com.google.devtools.build.lib.pkgcache.PackageManager;
 import com.google.devtools.build.lib.pkgcache.PathPackageLocator;
 import com.google.devtools.build.lib.rules.repository.RepositoryDelegatorFunction;
 import com.google.devtools.build.lib.runtime.KeepGoingOption;
+import com.google.devtools.build.lib.runtime.LoadingPhaseThreadsOption;
 import com.google.devtools.build.lib.skyframe.BazelSkyframeExecutorConstants;
 import com.google.devtools.build.lib.skyframe.ConfiguredTargetKey;
 import com.google.devtools.build.lib.skyframe.PrecomputedValue;
@@ -242,7 +243,8 @@ public abstract class AnalysisTestCase extends FoundationTestCase {
                     SkylarkSemanticsOptions.class,
                     BuildRequestOptions.class,
                     BuildView.Options.class,
-                    KeepGoingOption.class),
+                    KeepGoingOption.class,
+                    LoadingPhaseThreadsOption.class),
                 ruleClassProvider.getConfigurationOptions()));
     optionsParser.parse(new String[] {"--default_visibility=public" });
     optionsParser.parse(args);
@@ -305,10 +307,8 @@ public abstract class AnalysisTestCase extends FoundationTestCase {
     LoadingOptions loadingOptions = Options.getDefaults(LoadingOptions.class);
 
     BuildView.Options viewOptions = optionsParser.getOptions(BuildView.Options.class);
-    KeepGoingOption keepGoingOption = optionsParser.getOptions(KeepGoingOption.class);
     // update --keep_going option if test requested it.
-    keepGoingOption.keepGoing = flags.contains(Flag.KEEP_GOING);
-    viewOptions.loadingPhaseThreads = LOADING_PHASE_THREADS;
+    boolean keepGoing = flags.contains(Flag.KEEP_GOING);
 
     PackageCacheOptions packageCacheOptions = optionsParser.getOptions(PackageCacheOptions.class);
     PathPackageLocator pathPackageLocator =
@@ -344,7 +344,7 @@ public abstract class AnalysisTestCase extends FoundationTestCase {
             ImmutableList.copyOf(labels),
             PathFragment.EMPTY_FRAGMENT,
             loadingOptions,
-            keepGoingOption.keepGoing,
+            keepGoing,
             /*determineTests=*/ false,
             /*callback=*/ null);
 
@@ -359,7 +359,8 @@ public abstract class AnalysisTestCase extends FoundationTestCase {
             masterConfig,
             aspects,
             viewOptions,
-            keepGoingOption.keepGoing,
+            keepGoing,
+            LOADING_PHASE_THREADS,
             AnalysisTestUtil.TOP_LEVEL_ARTIFACT_CONTEXT,
             reporter,
             eventBus);
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewTestCase.java b/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewTestCase.java
index aae20bbde0..fa36b7ff0e 100644
--- a/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewTestCase.java
+++ b/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewTestCase.java
@@ -1547,7 +1547,6 @@ public abstract class BuildViewTestCase extends FoundationTestCase {
             : customLoadingOptions;
 
     BuildView.Options viewOptions = Options.getDefaults(BuildView.Options.class);
-    viewOptions.loadingPhaseThreads = loadingPhaseThreads;
 
     LoadingPhaseRunner runner = new LegacyLoadingPhaseRunner(getPackageManager(),
         Collections.unmodifiableSet(ruleClassProvider.getRuleClassMap().keySet()));
@@ -1570,6 +1569,7 @@ public abstract class BuildViewTestCase extends FoundationTestCase {
         aspects,
         viewOptions,
         keepGoing,
+        loadingPhaseThreads,
         AnalysisTestUtil.TOP_LEVEL_ARTIFACT_CONTEXT,
         reporter,
         eventBus);