Skip to content

Commit

Permalink
[7.1.0] Cherry-picks for elimination of repo rule restarts (#21082)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
Wyverald authored Jan 26, 2024
1 parent 5202a05 commit 5fcbb85
Show file tree
Hide file tree
Showing 4 changed files with 33 additions and 55 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -340,10 +340,27 @@ public void beforeCommand(CommandEnvironment env) throws AbruptExitException {
starlarkRepositoryFunction.setWorkerExecutorService(repoFetchingWorkerThreadPool);
break;
case VIRTUAL:
throw new AbruptExitException(
detailedExitCode(
"using a virtual worker thread for repo fetching is not yet supported",
Code.BAD_DOWNLOADER_CONFIG));
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
// anyone... right? (OSS Bazel already ships with a bundled JDK 21)
starlarkRepositoryFunction.setWorkerExecutorService(
(ExecutorService)
Executors.class
.getDeclaredMethod("newVirtualThreadPerTaskExecutor")
.invoke(null));
} catch (ReflectiveOperationException 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);
if (repoOptions.repositoryDownloaderRetries >= 0) {
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
6 changes: 6 additions & 0 deletions src/test/shell/bazel/starlark_repository_test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -2384,6 +2384,12 @@ EOF
bazel build @foo//:bar --experimental_worker_for_repo_fetching=platform >& $TEST_log \
|| fail "Expected build to succeed"
expect_log_n "hello world!" 1

# virtual worker thread, never restarts
bazel shutdown
bazel build @foo//:bar --experimental_worker_for_repo_fetching=virtual >& $TEST_log \
|| fail "Expected build to succeed"
expect_log_n "hello world!" 1
}

function test_duplicate_value_in_environ() {
Expand Down

0 comments on commit 5fcbb85

Please sign in to comment.