diff --git a/src/main/java/com/google/devtools/build/lib/BUILD b/src/main/java/com/google/devtools/build/lib/BUILD index 54714bc7bbb8ce..a5fb612693b8f8 100644 --- a/src/main/java/com/google/devtools/build/lib/BUILD +++ b/src/main/java/com/google/devtools/build/lib/BUILD @@ -1284,7 +1284,10 @@ java_library( java_library( name = "loading-phase-threads-option", srcs = ["runtime/LoadingPhaseThreadsOption.java"], - deps = ["//src/main/java/com/google/devtools/common/options"], + deps = [ + "//src/main/java/com/google/devtools/build/lib/actions:localhost_capacity", + "//src/main/java/com/google/devtools/common/options", + ], ) java_library( 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 index 75f10006f40343..812f571e1f70a9 100644 --- a/src/main/java/com/google/devtools/build/lib/runtime/LoadingPhaseThreadsOption.java +++ b/src/main/java/com/google/devtools/build/lib/runtime/LoadingPhaseThreadsOption.java @@ -13,22 +13,29 @@ // limitations under the License. package com.google.devtools.build.lib.runtime; +import com.google.devtools.build.lib.actions.LocalHostCapacity; 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; +import java.util.logging.Logger; /** Defines the --loading_phase_threads option which is used by multiple commands. */ public class LoadingPhaseThreadsOption extends OptionsBase { + + private static final Logger logger = Logger.getLogger(LoadingPhaseThreadsOption.class.getName()); + @Option( name = "loading_phase_threads", - defaultValue = LoadingPhaseThreadCountConverter.DEFAULT_VALUE, + defaultValue = "auto", documentationCategory = OptionDocumentationCategory.EXECUTION_STRATEGY, effectTags = {OptionEffectTag.BAZEL_INTERNAL_CONFIGURATION}, converter = LoadingPhaseThreadCountConverter.class, help = "Number of parallel threads to use for the loading/analysis phase." + + " \"auto\" means to use a reasonable value derived from the machine's hardware profile" + + " (e.g. the number of processors)." ) public int threads; @@ -38,33 +45,44 @@ public class LoadingPhaseThreadsOption extends OptionsBase { */ public static final class LoadingPhaseThreadCountConverter implements Converter { - 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; + int threads; + if (input.equals("auto")) { + threads = (int) Math.ceil(LocalHostCapacity.getLocalHostCapacity().getCpuUsage()); + logger.info("Flag \"loading_phase_threads\" was set to \"auto\"; using " + threads + + " threads"); + } else { + try { + threads = Integer.decode(input); + if (threads < 1) { + throw new OptionsParsingException("'" + input + "' must be at least 1"); + } + } catch (NumberFormatException e) { + throw new OptionsParsingException("'" + input + "' is not an int"); + } } - 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"); + if (System.getenv("TEST_TMPDIR") != null) { + // Cap 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. + // + // TODO(jmmv): If tests care about this, it's them who should be setting a cap. + threads = Math.min(20, threads); + logger.info("Running under a test; loading_phase_threads capped at " + threads); } + + // TODO(jmmv): Using the number of cores has proven to yield reasonable analysis times on + // Mac Pros and MacBook Pros but we should probably do better than this. (We haven't made + // any guarantees that "auto" means number of cores precisely to leave us room to tune this + // further in the future.) + return threads; } @Override public String getTypeDescription() { - return "an integer"; + return "\"auto\" or an integer"; } } } diff --git a/src/test/shell/integration/execution_phase_tests.sh b/src/test/shell/integration/execution_phase_tests.sh index 71951a21f55f21..fe5e3bf6087920 100755 --- a/src/test/shell/integration/execution_phase_tests.sh +++ b/src/test/shell/integration/execution_phase_tests.sh @@ -249,24 +249,39 @@ function test_cache_computed_file_digests_ui() { "Digests cache not reenabled" } -function test_jobs_default_auto() { +function do_threading_default_auto_test() { + local context="${1}"; shift + local flag_name="${1}"; shift + local -r pkg="${FUNCNAME}" mkdir -p "$pkg" || fail "could not create \"$pkg\"" - # The default flag value is only read if --jobs is not set explicitly. - # Do not use a bazelrc here, this would break the test. mkdir -p $pkg/package || fail "mkdir failed" echo "cc_library(name = 'foo', srcs = ['foo.cc'])" >$pkg/package/BUILD echo "int foo(void) { return 0; }" >$pkg/package/foo.cc + # The default value of the flag under test is only read if it is not set + # explicitly. Do not use a bazelrc here as it would break the test. local output_base="$(bazel --nomaster_bazelrc --bazelrc=/dev/null info \ output_base 2>/dev/null)" || fail "bazel info should work" local java_log="${output_base}/java.log" bazel --nomaster_bazelrc --bazelrc=/dev/null build $pkg/package:foo \ >>"${TEST_log}" 2>&1 || fail "Should build" - assert_last_log "BuildRequest" 'Flag "jobs" was set to "auto"' "${java_log}" \ - "--jobs was not set to auto by default" + assert_last_log \ + "${context}" \ + "Flag \"${flag_name}\" was set to \"auto\"" \ + "${java_log}" \ + "--${flag_name} was not set to auto by default" +} + +function test_jobs_default_auto() { + do_threading_default_auto_test "BuildRequest" "jobs" +} + +function test_loading_phase_threads_default_auto() { + do_threading_default_auto_test "LoadingPhaseThreadsOption" \ + "loading_phase_threads" } function test_analysis_warning_cached() {