From 0d3f62705aedb08590f7b73b405b317957aae52b Mon Sep 17 00:00:00 2001 From: Googler Date: Wed, 24 Jan 2024 11:47:33 -0800 Subject: [PATCH 1/2] Use reflection to support `--experimental_worker_for_repo_fetching=virtual` Bazel already ships JDK 21, but the codebase can't use JDK 21 features (language OR runtime) because Google hasn't migrated to JDK 21 yet (soon TM). But we can cheat a bit using reflection! Work towards https://github.com/bazelbuild/bazel/issues/10515 PiperOrigin-RevId: 601188027 Change-Id: I65c1712da0347bef40fc4af94bad4c211687a8b7 --- .../lib/bazel/BazelRepositoryModule.java | 20 +++++++++++++++---- .../shell/bazel/starlark_repository_test.sh | 6 ++++++ 2 files changed, 22 insertions(+), 4 deletions(-) diff --git a/src/main/java/com/google/devtools/build/lib/bazel/BazelRepositoryModule.java b/src/main/java/com/google/devtools/build/lib/bazel/BazelRepositoryModule.java index 0a2661fde95dc7..51162dd1491951 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/BazelRepositoryModule.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/BazelRepositoryModule.java @@ -340,10 +340,22 @@ 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)); + 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) { + 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) { diff --git a/src/test/shell/bazel/starlark_repository_test.sh b/src/test/shell/bazel/starlark_repository_test.sh index d5008c47cd5dd4..8493fe92f86ca0 100755 --- a/src/test/shell/bazel/starlark_repository_test.sh +++ b/src/test/shell/bazel/starlark_repository_test.sh @@ -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() { From 7bf7df889240f3277e0cc3b5237e3b1f02a3da04 Mon Sep 17 00:00:00 2001 From: Googler Date: Fri, 26 Jan 2024 10:55:56 -0800 Subject: [PATCH 2/2] Enable worker thread for repo fetching by default 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 https://github.com/bazelbuild/bazel/issues/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 --- .../lib/bazel/BazelRepositoryModule.java | 15 ++++-- .../bazel/repository/RepositoryOptions.java | 9 ++-- src/test/shell/bazel/bazel_workspaces_test.sh | 48 ------------------- 3 files changed, 16 insertions(+), 56 deletions(-) diff --git a/src/main/java/com/google/devtools/build/lib/bazel/BazelRepositoryModule.java b/src/main/java/com/google/devtools/build/lib/bazel/BazelRepositoryModule.java index 51162dd1491951..56de39965b24f1 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/BazelRepositoryModule.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/BazelRepositoryModule.java @@ -340,6 +340,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 @@ -350,11 +351,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); diff --git a/src/main/java/com/google/devtools/build/lib/bazel/repository/RepositoryOptions.java b/src/main/java/com/google/devtools/build/lib/bazel/repository/RepositoryOptions.java index e984bad9e35c94..e2183ed3957ad4 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/repository/RepositoryOptions.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/repository/RepositoryOptions.java @@ -216,7 +216,8 @@ public class RepositoryOptions extends OptionsBase { public enum WorkerForRepoFetching { OFF, PLATFORM, - VIRTUAL; + VIRTUAL, + AUTO; static class Converter extends EnumConverter { public Converter() { @@ -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( diff --git a/src/test/shell/bazel/bazel_workspaces_test.sh b/src/test/shell/bazel/bazel_workspaces_test.sh index e65a60377e8aa6..b600f6a3eb8337 100755 --- a/src/test/shell/bazel/bazel_workspaces_test.sh +++ b/src/test/shell/bazel/bazel_workspaces_test.sh @@ -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 <> WORKSPACE <