Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add a flag to use Turbine only if we can use the GraalVM native image #21869

Open
timothyg-stripe opened this issue Apr 1, 2024 · 2 comments
Labels
P4 This is either out of scope or we don't have bandwidth to review a PR. (No assignee) team-Rules-Java Issues for Java rules type: feature request

Comments

@timothyg-stripe
Copy link
Contributor

timothyg-stripe commented Apr 1, 2024

Description of the feature request:

In experimenting with --java_header_compilation, we have found that Turbine is only worth using if we can use the GraalVM native image added in #19361. If we cannot (such as if the java_library uses an annotation processor not supported by the native image), then we would rather fall back to Javac+JavaIjar, than using non–native image Turbine.

As such, we would like to have a flag in Bazel to only use Turbine if we will end up using the native image, tentatively called --[no]experimental_java_only_use_direct_header_compiler. We expect that this flag to be used in something like:

+def _would_use_header_compiler_direct(toolchain, plugin_info):
+    builtin_processors = {
+        proc: True
+        for proc in toolchain._header_compiler_builtin_processors.to_list()
+    }
+    return all([proc in builtin_processors for proc in plugin_info.plugins.processor_classes.to_list()])

-def _should_use_header_compilation(ctx, toolchain):
+def _should_use_header_compilation(ctx, toolchain, plugin_info):
     if not ctx.fragments.java.use_header_compilation():
         return False
     if toolchain._forcibly_disable_header_compilation:
         return False
     if not toolchain._header_compiler:
         fail(
             "header compilation was requested but it is not supported by the " +
             "current Java toolchain '" + str(toolchain.label) +
             "'; see the java_toolchain.header_compiler attribute",
         )
     if not toolchain._header_compiler_direct:
         fail(
             "header compilation was requested but it is not supported by the " +
             "current Java toolchain '" + str(toolchain.label) +
             "'; see the java_toolchain.header_compiler_direct attribute",
         )
+    if ctx.fragments.java.experimental_java_only_use_direct_header_compiler() and \
+       not _would_use_header_compiler_direct(toolchain, plkugin_info):
+        return False
     return True

(with equivalent change done to the Java version of the function)

Specifically, _would_use_header_compiler_direct is intended to mirror the logic in JavaHeaderCompilationAction.Builder.build.

cc @fmeum

Which category does this issue belong to?

Java Rules

What underlying problem are you trying to solve with this feature?

Making --java_header_compilation faster out of the box.

Which operating system are you running Bazel on?

macOS

What is the output of bazel info release?

7.1.1

If bazel info release returns development version or (@non-git), tell us how you built Bazel.

No response

What's the output of git remote get-url origin; git rev-parse HEAD ?

No response

Have you found anything relevant by searching the web?

No response

Any other information, logs, or outputs that you want to share?

No response

@timothyg-stripe timothyg-stripe changed the title Add a flag to use Turbine _only if_ we can use the GraalVM native image Add a flag to use Turbine only if we can use the GraalVM native image Apr 1, 2024
@github-actions github-actions bot added the team-Rules-Java Issues for Java rules label Apr 1, 2024
@fmeum
Copy link
Collaborator

fmeum commented Apr 1, 2024

Are you using --experimental_java_classpath=bazel? My understanding (and experience so far) is that Turbine isn't just beneficial when it's actually faster, but also because it provides the jdeps information needed for classpath pruning with this flag, which in turn reduces the amount of targets rebuilt during a typical incremental build. This information isn't available with ijar running after javac.

Cc @cushon

@cushon
Copy link
Contributor

cushon commented Apr 1, 2024

.jdeps are produced by both JavaBuilder and turbine, and if turbine is disabled the build should use the .jdeps output from JavaBuilder instead. There are some other things that could be get broken if turbine is only enabled for some actions in the build, including the 'direct classpath' stuff in JavaHeaderCompileAction which relies on information about transitive dependencies turbine repackages to allow compiling against direct dependencies only.

@hvadehra hvadehra added P4 This is either out of scope or we don't have bandwidth to review a PR. (No assignee) and removed untriaged labels Apr 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P4 This is either out of scope or we don't have bandwidth to review a PR. (No assignee) team-Rules-Java Issues for Java rules type: feature request
Projects
None yet
Development

No branches or pull requests

7 participants