Fix the --loading_phase_threads to work during "blaze build". Previously and confusingly it only worked on "blaze query".

--
MOS_MIGRATED_REVID=115338436

3 files changed, 37 insertions, 7 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/pkgcache/LoadingOptions.java b/src/main/java/com/google/devtools/build/lib/pkgcache/LoadingOptions.java
index 2f40193fd0..9ed11f3d68 100644
--- a/src/main/java/com/google/devtools/build/lib/pkgcache/LoadingOptions.java
+++ b/src/main/java/com/google/devtools/build/lib/pkgcache/LoadingOptions.java
@@ -15,9 +15,11 @@ package com.google.devtools.build.lib.pkgcache;
 
 import com.google.devtools.build.lib.packages.TestSize;
 import com.google.devtools.build.lib.packages.TestTimeout;
+import com.google.devtools.common.options.Converter;
+import com.google.devtools.common.options.Converters.CommaSeparatedOptionListConverter;
 import com.google.devtools.common.options.Option;
 import com.google.devtools.common.options.OptionsBase;
-import com.google.devtools.common.options.Converters.CommaSeparatedOptionListConverter;
+import com.google.devtools.common.options.OptionsParsingException;
 
 import java.util.List;
 import java.util.Set;
@@ -28,8 +30,9 @@ import java.util.Set;
 public class LoadingOptions extends OptionsBase {
 
   @Option(name = "loading_phase_threads",
-      defaultValue = "200",
+      defaultValue = "-1",
       category = "undocumented",
+      converter = LoadingPhaseThreadCountConverter.class,
       help = "Number of parallel threads to use for the loading phase.")
   public int loadingPhaseThreads;
 
@@ -112,4 +115,31 @@ public class LoadingOptions extends OptionsBase {
       help = "Use the Skyframe-based target pattern evaluator; implies "
           + "--experimental_interleave_loading_and_analysis.")
   public boolean useSkyframeTargetPatternEvaluator;
-}
\ No newline at end of file
+
+  /**
+   * 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> {
+    @Override
+    public Integer convert(String input) throws OptionsParsingException {
+      if ("-1".equals(input)) {
+        // 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.
+        return System.getenv("TEST_TMPDIR") == null ? 200 : 5;
+      }
+
+      try {
+        return Integer.decode(input);
+      } 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/skyframe/SkyframeExecutor.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java
index 470e562902..4be534db58 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
@@ -1398,7 +1398,8 @@ public abstract class SkyframeExecutor implements WalkableGraphFactory {
      * Loads the specified {@link TransitiveTargetValue}s.
      */
     EvaluationResult<TransitiveTargetValue> loadTransitiveTargets(EventHandler eventHandler,
-        Iterable<Target> targetsToVisit, Iterable<Label> labelsToVisit, boolean keepGoing)
+            Iterable<Target> targetsToVisit, Iterable<Label> labelsToVisit, boolean keepGoing,
+            int parallelThreads)
         throws InterruptedException {
       List<SkyKey> valueNames = new ArrayList<>();
       for (Target target : targetsToVisit) {
@@ -1408,8 +1409,7 @@ public abstract class SkyframeExecutor implements WalkableGraphFactory {
         valueNames.add(TransitiveTargetValue.key(label));
       }
 
-      return buildDriver.evaluate(valueNames, keepGoing, DEFAULT_THREAD_COUNT,
-          eventHandler);
+      return buildDriver.evaluate(valueNames, keepGoing, parallelThreads, eventHandler);
     }
 
     public Set<Package> retrievePackages(
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeLabelVisitor.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeLabelVisitor.java
index 6cfb49adc3..f23edea999 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeLabelVisitor.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeLabelVisitor.java
@@ -69,7 +69,7 @@ final class SkyframeLabelVisitor implements TransitivePackageLoader {
     rootCauses.clear();
     lastBuildKeepGoing = false;
     EvaluationResult<TransitiveTargetValue> result = transitivePackageLoader.loadTransitiveTargets(
-        eventHandler, targetsToVisit, labelsToVisit, keepGoing);
+        eventHandler, targetsToVisit, labelsToVisit, keepGoing, parallelThreads);
     updateVisitedValues(result.values());
     lastBuildKeepGoing = keepGoing;