Skip to content

Commit

Permalink
Make --loading_phase_threads default to the number of CPUs.
Browse files Browse the repository at this point in the history
Change the behavior of the --loading_phase_threads to match that of --jobs:
default it to a new "auto" magic value that causes Bazel to determine a
reasonable setting (currently the number of CPUs) but allow users to override
it with an explicit count.

This should alleviate threading contention during analysis and reduce the
unresponsiveness of macOS when this happens.  Some tests on large apps show
a reduction of up to 40% (!) on a MacBook Pro with 4 physical cores and a
more modest 15% on a Mac Pro with 6 physical cores.

Note that the previous hack to cap the number of threads to 20 for unit and
integration tests remains.  I'll tackle this separately.

RELNOTES: --loading_phase_threads now defaults to "auto" (not 200, as was
previously the case), which at the moment corresponds to the number of CPUs.
If your sources are on a slow file system, increasing this value may yield
better analysis-time performance when disk caches are cold.
PiperOrigin-RevId: 215151994
  • Loading branch information
jmmv authored and Copybara-Service committed Oct 1, 2018
1 parent 050e0ff commit ac88041
Show file tree
Hide file tree
Showing 3 changed files with 61 additions and 25 deletions.
5 changes: 4 additions & 1 deletion src/main/java/com/google/devtools/build/lib/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -38,33 +45,44 @@ public class LoadingPhaseThreadsOption extends OptionsBase {
*/
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;
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";
}
}
}
25 changes: 20 additions & 5 deletions src/test/shell/integration/execution_phase_tests.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down

0 comments on commit ac88041

Please sign in to comment.