Skip to content
This repository has been archived by the owner on Nov 27, 2023. It is now read-only.

chore: Update Graalvm version in GitHub workflows #731

Merged
merged 1 commit into from
Jul 11, 2023

Conversation

@codecov
Copy link

codecov bot commented Jun 28, 2023

Codecov Report

Merging #731 (4e73488) into main (f7a0e3f) will decrease coverage by 0.07%.
The diff coverage is n/a.

@@             Coverage Diff              @@
##               main     #731      +/-   ##
============================================
- Coverage     78.66%   78.60%   -0.07%     
  Complexity      160      160              
============================================
  Files            54       54              
  Lines          2958     2958              
  Branches        500      500              
============================================
- Hits           2327     2325       -2     
- Misses          423      425       +2     
  Partials        208      208              

see 1 file with indirect coverage changes

@mkralik3
Copy link
Contributor Author

@Delawen
Do we need to specify the Graalvm version explicitly? Was there any problem with the latest one? ( for the particular Java version )

@Delawen
Copy link
Member

Delawen commented Jun 28, 2023

@apupier

I think it makes sense to have a fixed version to prevent random occurring bugs due to graalvm.

But also, at the same time, we are not looking at minor versions of java.

@apupier
Copy link
Member

apupier commented Jun 28, 2023

I think it makes sense to have a fixed version to prevent random occurring bugs due to graalvm.

agree but it seems that they are pushing to use the "graalvm" distriution based on https://github.com/graalvm/setup-graalvm#migrating-from-graalvm-223-or-earlier-to-the-new-graalvm-for-jdk-17-and-later

If I understand well, it means that it is using Oracle Graalvm JDK instead of the community edition. the license is different https://www.oracle.com/java/technologies/javase/jdk-faqs.html#GraalVM-licensing we will need to read it carefully. I think we should also ask what Quarkus is recommending and internally what Red Hat is recommending.

But also, at the same time, we are not looking at minor versions of java.

End-users will pick the latest version of Java on an update of their system. There are very low chance of regression for minor Java version for the not that extensive use of Java API we are using in this project.

The graalvm has an high impact for the native binary that is generated, the end-users won't be able to update it.

@mkralik3
Copy link
Contributor Author

mkralik3 commented Jun 30, 2023

Yes, it looks that with new releases, they don't use graalvm versioning as before (e.g. 22.3.2), but they use the same version as Java, e.g. 17.0.7 , https://github.com/graalvm/graalvm-ce-builds/releases/tag/jdk-17.0.7 so I would say the version parameter in workflows will not be applicable for next Graalvm versions.
https://github.com/graalvm/setup-graalvm#options ( up to 22.3.2 )

X.Y.Z (e.g., 22.3.0) for a specific GraalVM release up to 22.3.2

tldr, versioning of Graalvm was changed https://www.graalvm.org/release-notes/

GraalVM for JDK 20
GraalVM for JDK 17
GraalVM 22.3.x
GraalVM 22.2.x
GraalVM 22.1.x
GraalVM 22.0.x
GraalVM 21.3.x
GraalVM 21.2.x
GraalVM 21.1.x

and due to that, the version parameter is not needed anymore ( since the specific version is set in java-version: '17.0.7'
e.g. Mandrel continue with the same versioning so Mandrel 23.0.0 is from GraalVM for JDK 17 Community

====

If I understand well, it means that it is using Oracle Graalvm JDK instead of the community edition. the license is different https://www.oracle.com/java/technologies/javase/jdk-faqs.html#GraalVM-licensing we will need to read it carefully. I think we should also ask what Quarkus is recommending and internally what Red Hat is recommending.

We can use different distribution, graalvm-community or mandrel. If I understand correctly, mandrel is primarily intended to use with Quarkus.
https://github.com/graalvm/mandrel#mandrel
https://developers.redhat.com/blog/2021/04/14/mandrel-a-specialized-distribution-of-graalvm-for-quarkus

@mkralik3
Copy link
Contributor Author

mkralik3 commented Jun 30, 2023

Unfortunately, ubuntu and windows native build constantly fail on this error

Image heap writing found a class not seen during static analysis. Did a static field or an object referenced from a static field change during native image generation? For example, a lazily initialized cache could have been initialized during image generation, in which case you need to force eager initialization of the cache before static analysis or reset the cache using a field value recomputation.

Maybe similar? (oracle/graal#6793)

However, it passes on macOS which has more powerful hardware ( +1CPU core, +7GB ram )
https://docs.github.com/en/actions/using-github-hosted-runners/about-github-hosted-runners#supported-runners-and-hardware-resources
which can be a root cause because on macOS, the peek for memory in the log shows more then 7GB of ram. ( which is maximum on ubuntu/windows)

building api package with Oracle GraalVM 17.0.7+8.1 on:
macOS: 88.5s (10.4% of total time) in 254 GCs | Peak RSS: 7.25GB | CPU load: 2.72
Windows: failed
Ubuntu: failed

example (workflows from some merged PR) of a building api package with Graalvm GraalVM 22.3.0 Java 17 CE on:
macOS: 36.7s (6.9% of total time) in 92 GCs | Peak RSS: 7.13GB | CPU load: 2.20
Windows: 67.7s (14.6% of total time) in 183 GCs | Peak RSS: 5.37GB | CPU load: 1.68
Ubuntu: 60.0s (12.0% of total time) in 178 GCs | Peak RSS: 6.04GB | CPU load: 1.77

Locally
GraalVM 22.3.0 Java 17 CE 15.2s (6.2% of total time) in 84 GCs | Peak RSS: 7.77GB | CPU load: 6.32
Oracle GraalVM 17.0.7+8.1 10.2s (3.5% of total time) in 60 GCs | Peak RSS: 12.57GB | CPU load: 8.69

@mkralik3
Copy link
Contributor Author

mkralik3 commented Jun 30, 2023

I found a workaround. By quarkus.native.native-image-xmx, we can instruct GraalVM to use 5GB of maximum heap during building a native image
https://quarkus.io/guides/building-native-image#quarkus-native-pkg-native-config_quarkus.native.native-image-xmx

@mkralik3 mkralik3 mentioned this pull request Jun 30, 2023
@mkralik3 mkralik3 linked an issue Jun 30, 2023 that may be closed by this pull request
@mkralik3
Copy link
Contributor Author

I think it makes sense to have a fixed version to prevent random occurring bugs due to graalvm.

If I understand this graalvm/setup-graalvm#47 correctly, we can specify a full version, e.g. 17.0.7, and the new version should not break anything since it will have a different full version of java.

Also Madrel is not supported on macOS graalvm/mandrel#114, so I have switched to graalvm-community

@mkralik3 mkralik3 marked this pull request as ready for review June 30, 2023 17:36
@mkralik3 mkralik3 requested review from a team, Delawen and apupier June 30, 2023 17:36
@Delawen
Copy link
Member

Delawen commented Jul 7, 2023

We need the companion PR for https://github.com/KaotoIO/kaoto-ui/blob/main/.github/workflows/add-standalone-jar-to-release.yml#L114-L118

(Waiting to have it to merge or we will be using different graalvm versions depending on who builds what)

* Used community version instead of Oracle version
* Set maximum heap size for building native image
@sonarcloud
Copy link

sonarcloud bot commented Jul 10, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

@mkralik3
Copy link
Contributor Author

@Delawen sure, I have created a PR: KaotoIO/kaoto-ui#2118

@Delawen Delawen linked an issue Jul 11, 2023 that may be closed by this pull request
@Delawen Delawen merged commit 6917e45 into KaotoIO:main Jul 11, 2023
17 of 18 checks passed
@mkralik3 mkralik3 deleted the updateGraalvmWorkflows branch August 3, 2023 10:08
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Upgrade to the latest Graalvm Upgrade GraalVM to 22.3.1
3 participants