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.

Sign up for GitHub

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

Jump to bottom

Update to GraalVM / Mandrel 22.3.0 #28861

Merged
merged 3 commits into from
Nov 3, 2022
Merged

Update to GraalVM / Mandrel 22.3.0 #28861

merged 3 commits into from
Nov 3, 2022

Conversation

cescoffier
Copy link
Member

@cescoffier cescoffier commented Oct 26, 2022 •
edited
Loading

DO NOT MERGE!

@quarkus-bot quarkus-bot bot added area/core area/dependencies Pull requests that update a dependency file area/devtools Issues/PR related to maven, gradle, platform and cli tooling/plugins area/documentation area/infra-automation anything related to CI, bots, etc. that are used to automated our infrastructure labels Oct 26, 2022
@geoand geoand changed the title WIP - Update to GraalVM 2.3.0 WIP - Update to GraalVM 22.3.0 Oct 26, 2022
@cescoffier cescoffier marked this pull request as ready for review October 26, 2022 15:17
zakkak
zakkak previously requested changes Oct 26, 2022
View reviewed changes
Copy link
Contributor

@zakkak zakkak left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Needs rebasing now that #28843 got merged.

@gsmet
Copy link
Member

gsmet commented Oct 26, 2022

Let me rebase it.

@gsmet
Copy link
Member

gsmet commented Oct 26, 2022

Done! We still need to adjust the Mandrel image version when available but I updated the doc version as it won't break anything.

gsmet
gsmet requested changes Oct 26, 2022
View reviewed changes
core/deployment/src/main/java/io/quarkus/deployment/pkg/NativeConfig.java Outdated
@@ -16,7 +16,7 @@
@ConfigRoot(phase = ConfigPhase.BUILD_TIME)
public class NativeConfig {

public static final String DEFAULT_GRAALVM_BUILDER_IMAGE = "quay.io/quarkus/ubi-quarkus-graalvmce-builder-image:22.2-java17";
public static final String DEFAULT_GRAALVM_BUILDER_IMAGE = "quay.io/quarkus/ubi-quarkus-graalvmce-builder-image:22.3-java17";
public static final String DEFAULT_MANDREL_BUILDER_IMAGE = "quay.io/quarkus/ubi-quarkus-mandrel-builder-image:22.2-java17";
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To make sure we won't forget: we need to update that one when ready.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

quay.io/quarkus/ubi-quarkus-mandrel-builder-image:22.3-java17 is there now, fwiw.

@gsmet gsmet dismissed zakkak’s stale review October 26, 2022 17:18

Addressed.

@gsmet
Copy link
Member

gsmet commented Oct 26, 2022

@zakkak I see the build failing with:

Error:  Failed to execute goal org.apache.maven.plugins:maven-compiler-plugin:3.8.1:compile (default-compile) on project quarkus-core: Compilation failure: Compilation failure: 
Error:  /home/runner/work/quarkus/quarkus/core/runtime/src/main/java/io/quarkus/runtime/graal/ConfigurationSubstitutions.java:[6,36] cannot find symbol
Error:    symbol:   class AlwaysInline
Error:    location: package com.oracle.svm.core.annotate
Error:  /home/runner/work/quarkus/quarkus/core/runtime/src/main/java/io/quarkus/runtime/graal/ConfigurationSubstitutions.java:[26,6] cannot find symbol
Error:    symbol:   class AlwaysInline
Error:    location: class io.quarkus.runtime.graal.Target_io_smallrye_config_SmallRyeConfigProviderResolver
Error:  -> [Help 1]

Did we miss something when ensuring 22.3 compatibility?

@zakkak
Copy link
Contributor

zakkak commented Oct 26, 2022 •
edited
Loading

Did we miss something when ensuring 22.3 compatibility?

Hmmm, indeed. Unfortunately Quarkus uses the defined graalvm-version for the GraalVM artifacts no matter which GraalVM version we use. So in our tests javac was building with the 22.2 GraalVM jars despite using 22.3 to build the actual native image. IIRC @Sanne experienced this at some point as well.

Regarding this specific failure, it's because com.oracle.svm.core.annotate.AlwaysInline (which is not public API) was moved to com.oracle.svm.core.AlwaysInline.

So it looks like we will need to patch the source code accordingly and bump the minimum version to 22.3 or remove the non public API annotations.

@gsmet
Copy link
Member

gsmet commented Oct 27, 2022

I pushed an update to move to the new package. Rationale explained in the commit comment.

@cescoffier
Copy link
Member Author

@zakkak unfortunately, it cannot be backported, at least not directly.
These images use the new multi-archs and jdock models. We need to use the former model for the backport.

@zakkak
Copy link
Contributor

zakkak commented Oct 27, 2022

@zakkak unfortunately, it cannot be backported, at least not directly. These images use the new multi-archs and jdock models. We need to use the former model for the backport.

Oh, I see. Shall we keep the label though to indicate that we need this in 2.13 as well?

@gsmet
Copy link
Member

gsmet commented Oct 27, 2022

Yes, let's keep the label so that we don't miss it.

@gsmet
Copy link
Member

gsmet commented Oct 28, 2022

@cescoffier what's the status of this build in your fork? Did you see other problems?

@cescoffier
Copy link
Member Author

Nope, looks good.
The 2.13 images have been produced. Waiting for mandrel.

@zakkak
Copy link
Contributor

zakkak commented Oct 31, 2022

Waiting for mandrel.

FYI, due to some infrastructure related issues this might take a bit longer than usual.

@zakkak
Copy link
Contributor

zakkak commented Nov 2, 2022

Mandrel 22.3 is now released. I have also openned quarkusio/quarkus-images#215 to generate the quarkus images. @cescoffier are you going to update this PR to include the Mandrel images as well?

@zakkak zakkak mentioned this pull request Nov 2, 2022
Closed
@cescoffier cescoffier changed the title WIP - Update to GraalVM 22.3.0 Update to GraalVM / Mandrel 22.3.0 Nov 2, 2022
zakkak
zakkak approved these changes Nov 2, 2022
View reviewed changes
Copy link
Contributor

@zakkak zakkak left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks @cescoffier

cescoffier and others added 3 commits November 3, 2022 10:51
@cescoffier @gsmet
Update GraalVM 22.3.0 images
258ea39
@gsmet
Use the GraalVM 22.3 AlwaysInline annotation
3d85651 
This annotation has moved to a new package and we need to use the 22.3
version to make sure our code compiles.
As for using 22.2, it still works but the annotation is ignored, which
looks OK as the annotation is not used in places where it is strictly
necessary.
If people use the recommended GraalVM version, everything will be fine.
If not, things will mostly work anyway.
@cescoffier @gsmet
Update to Mandrel 22.3 images
f2bf43d
@gsmet
Copy link
Member

gsmet commented Nov 3, 2022

I rebased to trigger CI as it was in limbo.

@quarkus-bot
Copy link

quarkus-bot bot commented Nov 3, 2022

Failing Jobs - Building f2bf43d

Status Name Step Failures Logs Raw logs
✔️ JVM Tests - JDK 11
✔️ JVM Tests - JDK 17
✖ JVM Tests - JDK 17 MacOS M1 Set up runner ⚠️ Check → Logs Raw logs
✔️ JVM Tests - JDK 18

@gsmet gsmet merged commit b39d1f6 into quarkusio:main Nov 3, 2022
@quarkus-bot quarkus-bot bot added this to the 2.15 - main milestone Nov 3, 2022
@rsvoboda
Copy link
Member

rsvoboda commented Nov 4, 2022 •
edited
Loading

I see errors in our daily native runs, it's apparently because of oracle/graal#5303 bug
I'm running DB tests from integration-tests module using -Dquarkus.native.container-build=true.

Shouldn't we set mandrel image as default for now? Peter has commented on the issue that Mandrel 22.3 works for him.

@zakkak
Copy link
Contributor

zakkak commented Nov 4, 2022

@gsmet I am marking this as a "noteworthy" feature for better visibility.

(inspired by discussion in https://quarkusio.zulipchat.com/#narrow/stream/187030-users/topic/Quarkus.20won't.20build.20native/near/307716675)

@rsvoboda
Copy link
Member

rsvoboda commented Nov 4, 2022

@galderz / @zakkak do you think oracle/graal#5303 is gonna be fixed soon?
Or should be the community default builder image switched to mandrel till it's fixed? cc @cescoffier

@zakkak
Copy link
Contributor

zakkak commented Nov 4, 2022

@rsvoboda I really don't know. I personally have not seen this issue happen in any of the machines I have access to.

I would suggest commenting on the upstream issue and possibly providing more info about the context under which the issue is triggered.

Regarding switching to Mandrel by default I am +0. People getting hit by the issue can easily switch to Mandrel by passing -Dquarkus.native.builder-image=mandrel. If it turns out to be a pretty common issue I expect upstream to make a new release soonish.

@rsvoboda
Copy link
Member

rsvoboda commented Nov 4, 2022

ok, I will comment upstream

@cescoffier cescoffier deleted the graalvm-22.3.0 branch November 4, 2022 12:28
@gsmet gsmet modified the milestones: 2.15 - main, 2.14.1.Final Nov 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jerboaa jerboaa jerboaa left review comments

@zakkak zakkak zakkak approved these changes

@gsmet gsmet gsmet approved these changes

Assignees
No one assigned
Labels
area/core area/dependencies Pull requests that update a dependency file area/devtools Issues/PR related to maven, gradle, platform and cli tooling/plugins area/documentation area/infra-automation anything related to CI, bots, etc. that are used to automated our infrastructure release/noteworthy-feature
Projects
None yet
Milestone
2.14.1.Final
Development

Successfully merging this pull request may close these issues.
5 participants
@cescoffier @gsmet @zakkak @rsvoboda @jerboaa