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 native image of turbine to the prebuilt Java tools #19361

Merged
merged 1 commit into from
Nov 13, 2023

Conversation

fmeum
Copy link
Collaborator

@fmeum fmeum commented Aug 29, 2023

A simple local experiment shows that the time for all Java header compilation actions required for //src:bazel-dev decreases by a factor of 4.5 when using a native image of turbine instead of jar. The time taken for an incremental build of //src/main/java/com/google/devtools/build/lib/bazel:BazelServer after adding a public method to Label decreases by a factor of 2 with --experimental_java_classpath=bazel.

As a first step towards using the native image in Java toolchains, ship it as part of the prebuilt Java tools by using rules_graalvm.

@fmeum
Copy link
Collaborator Author

fmeum commented Aug 29, 2023

FYI @sgammon

Based on the docs of rules_graalvm, I assume I will have to wait for version 0.10.0 to get Windows support?

@sgammon
Copy link

sgammon commented Aug 29, 2023

@fmeum windows support is shipped on main and potentially will work in 0.9.2, which shipped early to fix some compat issues with the previous rules nevermind, it probably needs your path fix and I can issue a release shortly.

there are now integration tests for Bazel 4-7, and each type of GVM variant, etc. Windows runs most of these, it just skips the bazel4, bazel5 and legacy tests; in each case this is because support is missing for Windows upstream.

Sticking with the latest GVM version and rule version should provide good Windows support against MSVC, modulo any bugs you might encounter in the newer hermeticity advice. Keep in mind that native-image uses various env vars like INCLUDE and LIB, which may need to be made available in stricter environments.

@sgammon
Copy link

sgammon commented Aug 29, 2023

ah, it looks like it's complaining about the toolchain:

Error: On Windows, GraalVM Native Image for JDK 20 requires Visual Studio 2022 version 17.1.0 or later (C/C++ Optimizing Compiler Version 19.31 or later).
--
  | Compiler info detected: cl.exe (microsoft, x64, 19.16.27050)
  | Error: To prevent native-toolchain checking provide command-line option -H:-CheckToolchain
  | com.oracle.svm.core.util.UserError$UserException: On Windows, GraalVM Native Image for JDK 20 requires Visual Studio 2022 version 17.1.0 or later (C/C++ Optimizing Compiler Version 19.31 or later).
  | Compiler info detected: cl.exe (microsoft, x64, 19.16.27050)
  | To prevent native-toolchain checking provide command-line option -H:-CheckToolchain
  | at org.graalvm.nativeimage.builder/com.oracle.svm.core.util.UserError.abort(UserError.java:126)
  | at org.graalvm.nativeimage.builder/com.oracle.svm.hosted.c.codegen.CCompilerInvoker.addSkipCheckingInfo(CCompilerInvoker.java:106)
  | at org.graalvm.nativeimage.builder/com.oracle.svm.hosted.c.codegen.CCompilerInvoker.verifyCompiler(CCompilerInvoker.java:97)
  | at org.graalvm.nativeimage.builder/com.oracle.svm.hosted.NativeImageGenerator.setupNativeImage(NativeImageGenerator.java:930)
  | at org.graalvm.nativeimage.builder/com.oracle.svm.hosted.NativeImageGenerator.doRun(NativeImageGenerator.java:579)
  | at org.graalvm.nativeimage.builder/com.oracle.svm.hosted.NativeImageGenerator.run(NativeImageGenerator.java:539)
  | at org.graalvm.nativeimage.builder/com.oracle.svm.hosted.NativeImageGeneratorRunner.buildImage(NativeImageGeneratorRunner.java:408)
  | at org.graalvm.nativeimage.builder/com.oracle.svm.hosted.NativeImageGeneratorRunner.build(NativeImageGeneratorRunner.java:612)
  | at org.graalvm.nativeimage.builder/com.oracle.svm.hosted.NativeImageGeneratorRunner.start(NativeImageGeneratorRunner.java:134)
  | at org.graalvm.nativeimage.builder/com.oracle.svm.hosted.NativeImageGeneratorRunner.main(NativeImageGeneratorRunner.java:94)

-H:-CheckToolchain can be passed with the check_toolchain attr on the native_image target, which defaults to True.

@fmeum fmeum force-pushed the native-turbine branch 14 times, most recently from e82360b to de64a2e Compare August 31, 2023 09:41
@fmeum
Copy link
Collaborator Author

fmeum commented Aug 31, 2023

With sgammon/rules_graalvm#70, sgammon/rules_graalvm#71, sgammon/rules_graalvm#72, and sgammon/rules_graalvm#73 I can get the native_image to build in CI on all platforms.

@sgowroji
Copy link
Member

sgowroji commented Nov 8, 2023

Hi @fmeum, Could you please resolve the conflicts?

@fmeum fmeum force-pushed the native-turbine branch 2 times, most recently from 4630ff5 to fb71ab0 Compare November 8, 2023 13:26
@fmeum
Copy link
Collaborator Author

fmeum commented Nov 8, 2023

@sgowroji Done

@sgowroji
Copy link
Member

sgowroji commented Nov 8, 2023

Hi @fmeum, Could you please take a look at the failing checks. @hvadehra Can you please approve the new changes.

@fmeum
Copy link
Collaborator Author

fmeum commented Nov 8, 2023

@sgowroji Also done!

@fmeum fmeum requested a review from hvadehra November 8, 2023 18:06
@sgowroji sgowroji added awaiting-review PR is awaiting review from an assigned reviewer awaiting-PR-merge PR has been approved by a reviewer and is ready to be merge internally and removed awaiting-PR-merge PR has been approved by a reviewer and is ready to be merge internally awaiting-review PR is awaiting review from an assigned reviewer labels Nov 9, 2023
A simple local experiment shows that the time for all Java header
compilation actions required for `//src:bazel-dev` decreases by a factor
of 4.5 when using a native image of turbine instead of jar. The time
taken for an incremental build of
`//src/main/java/com/google/devtools/build/lib/bazel:BazelServer` after
adding a public method to `Label` decreases by a factor of 2 with
`--experimental_java_classpath=bazel`.

As a first step towards using the native image in Java toolchains, ship
it as part of the prebuilt Java tools by using rules_graalvm.
@fmeum
Copy link
Collaborator Author

fmeum commented Nov 11, 2023

@sgowroji I fixed the new merge conflict.

@meteorcloudy meteorcloudy merged commit 4113fd5 into bazelbuild:master Nov 13, 2023
31 checks passed
@github-actions github-actions bot removed the awaiting-PR-merge PR has been approved by a reviewer and is ready to be merge internally label Nov 13, 2023
@meteorcloudy
Copy link
Member

I'm terribly sorry that I thought this was a cherry-pick and merged this directly. I had to force push to master to fix the history and this PR should be merged by importing to google first, which is work in process.

@meteorcloudy meteorcloudy added the awaiting-PR-merge PR has been approved by a reviewer and is ready to be merge internally label Nov 13, 2023
@fmeum
Copy link
Collaborator Author

fmeum commented Nov 14, 2023

I resolved the conflict in the lockfile. @sgowroji

copybara-service bot pushed a commit that referenced this pull request Nov 15, 2023
A simple local experiment shows that the time for all Java header compilation actions required for `//src:bazel-dev` decreases by a factor of 4.5 when using a native image of turbine instead of jar. The time taken for an incremental build of `//src/main/java/com/google/devtools/build/lib/bazel:BazelServer` after adding a public method to `Label` decreases by a factor of 2 with `--experimental_java_classpath=bazel`.

As a first step towards using the native image in Java toolchains, ship it as part of the prebuilt Java tools by using rules_graalvm.

Closes #19361.

PiperOrigin-RevId: 582531539
Change-Id: If71d8ec86fecbc1363a748d197f88962bd489fb4
@fmeum
Copy link
Collaborator Author

fmeum commented Nov 15, 2023

@keertk Could I ask you to create a new java_tools release now that this has been merged? I could then use it in a rules_java PR.

@sgowroji sgowroji removed the awaiting-PR-merge PR has been approved by a reviewer and is ready to be merge internally label Nov 15, 2023
github-merge-queue bot pushed a commit that referenced this pull request Feb 21, 2024
)

A simple local experiment shows that the time for all Java header
compilation actions required for `//src:bazel-dev` decreases by a factor
of 4.5 when using a native image of turbine instead of jar. The time
taken for an incremental build of
`//src/main/java/com/google/devtools/build/lib/bazel:BazelServer` after
adding a public method to `Label` decreases by a factor of 2 with
`--experimental_java_classpath=bazel`.

As a first step towards using the native image in Java toolchains, ship
it as part of the prebuilt Java tools by using rules_graalvm.

Closes #19361.
Commit
5f79684

PiperOrigin-RevId: 582531539
Change-Id: If71d8ec86fecbc1363a748d197f88962bd489fb4

---------

Co-authored-by: Fabian Meumertzheim <fabian@meumertzhe.im>
Co-authored-by: Xùdōng Yáng <wyverald@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
team-Rules-Java Issues for Java rules
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants