Skip to content

Commit

Permalink
Enable worker thread for repo fetching by default
Browse files Browse the repository at this point in the history
We use virtual threads if available (JDK 21+); if not, we fall back to not using worker threads at all. This approach is slightly safer than straight up setting the default to `virtual` as that would break certain obscure platforms without an available JDK 21. The plan is to cherry-pick this into 7.1.0.

Also removed test_reexecute in bazel_workspaces_test since, well, that's the whole point!

Fixes #10515

RELNOTES: The flag `--experimental_worker_for_repo_fetching` now defaults to `auto`, which uses virtual threads from JDK 21 if it's available. This eliminates restarts during repo fetching.
PiperOrigin-RevId: 601810629
Change-Id: I9d2ec59688586356d90604fdd328cc985e75d1d4
  • Loading branch information
Wyverald authored and copybara-github committed Jan 26, 2024
1 parent 999a26a commit 57c1801
Show file tree
Hide file tree
Showing 3 changed files with 16 additions and 56 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -344,6 +344,7 @@ public void beforeCommand(CommandEnvironment env) throws AbruptExitException {
starlarkRepositoryFunction.setWorkerExecutorService(repoFetchingWorkerThreadPool);
break;
case VIRTUAL:
case AUTO:
try {
// Since Google hasn't migrated to JDK 21 yet, we can't directly call
// Executors.newVirtualThreadPerTaskExecutor here. But a bit of reflection never hurt
Expand All @@ -354,11 +355,15 @@ public void beforeCommand(CommandEnvironment env) throws AbruptExitException {
.getDeclaredMethod("newVirtualThreadPerTaskExecutor")
.invoke(null));
} catch (ReflectiveOperationException e) {
throw new AbruptExitException(
detailedExitCode(
"couldn't create virtual worker thread executor for repo fetching",
Code.BAD_DOWNLOADER_CONFIG),
e);
if (repoOptions.workerForRepoFetching == RepositoryOptions.WorkerForRepoFetching.AUTO) {
starlarkRepositoryFunction.setWorkerExecutorService(null);
} else {
throw new AbruptExitException(
detailedExitCode(
"couldn't create virtual worker thread executor for repo fetching",
Code.BAD_DOWNLOADER_CONFIG),
e);
}
}
}
downloadManager.setDisableDownload(repoOptions.disableDownload);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -216,7 +216,8 @@ public class RepositoryOptions extends OptionsBase {
public enum WorkerForRepoFetching {
OFF,
PLATFORM,
VIRTUAL;
VIRTUAL,
AUTO;

static class Converter extends EnumConverter<WorkerForRepoFetching> {
public Converter() {
Expand All @@ -227,14 +228,16 @@ public Converter() {

@Option(
name = "experimental_worker_for_repo_fetching",
defaultValue = "off",
defaultValue = "auto",
converter = WorkerForRepoFetching.Converter.class,
documentationCategory = OptionDocumentationCategory.REMOTE,
effectTags = {OptionEffectTag.UNKNOWN},
help =
"The threading mode to use for repo fetching. If set to 'off', no worker thread is used,"
+ " and the repo fetching is subject to restarts. Otherwise, uses a platform thread"
+ " (i.e. OS thread) if set to 'platform' or a virtual thread if set to 'virtual'.")
+ " (i.e. OS thread) if set to 'platform' or a virtual thread if set to 'virtual'. If"
+ " set to 'auto', virtual threads are used if available (i.e. running on JDK 21+),"
+ " otherwise no worker thread is used.")
public WorkerForRepoFetching workerForRepoFetching;

@Option(
Expand Down
48 changes: 0 additions & 48 deletions src/test/shell/bazel/bazel_workspaces_test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -96,54 +96,6 @@ function test_execute() {
ensure_contains_exactly "location: .*repos.bzl:2:25" 0
}

# The workspace is set up so that the function is interrupted and re-executed.
# The log should contain both instances.
function test_reexecute() {
create_new_workspace
cat > BUILD <<'EOF'
genrule(
name="test",
srcs=["@repo//:t.txt"],
outs=["out.txt"],
cmd="echo Result > $(location out.txt)"
)
EOF
cat >> repos.bzl <<EOF
def _executeMe(repository_ctx):
repository_ctx.execute(["echo", "testing!"])
build_contents = "package(default_visibility = ['//visibility:public'])\n\n"
build_contents += "exports_files([\"t.txt\"])\n"
repository_ctx.file("BUILD", build_contents, False)
repository_ctx.symlink(Label("@another//:dummy.txt"), "t.txt")
ex_repo = repository_rule(
implementation = _executeMe,
local = True,
)
def _another(repository_ctx):
build_contents = "exports_files([\"dummy.txt\"])\n"
repository_ctx.file("BUILD", build_contents, False)
repository_ctx.file("dummy.txt", "dummy\n", False)
a_repo = repository_rule(
implementation = _another,
local = True,
)
EOF
cat >> WORKSPACE <<EOF
load("//:repos.bzl", "ex_repo")
load("//:repos.bzl", "a_repo")
ex_repo(name = "repo")
a_repo(name = "another")
EOF

build_and_process_log

ensure_contains_atleast "location: .*repos.bzl:2:" 2
}


# Ensure details of the specific functions are present
function test_execute2() {
set_workspace_command 'repository_ctx.execute(["echo", "test_contents"], 21, {"Arg1": "Val1"}, True)'
Expand Down

0 comments on commit 57c1801

Please sign in to comment.