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

Update GraalVM to 17.0.7 (23.0.0 JDK17) #7176

Merged
merged 56 commits into from
Jul 20, 2023

Conversation

Akirathan
Copy link
Member

@Akirathan Akirathan commented Jun 30, 2023

Closes #5300

Update GraalVM to 17.0.7+7.1

Pull Request Description

Removed warnings:

  • Remove deprecated ConditionProfile.createCountingProfile().
  • Add @Shared to some @Cached parameters (Truffle now emits warnings about potential @Share usage).
  • Specialization method names should not start with execute
  • Add limit attribute to some specialization methods
  • Add @NeverDefault for some cached initializer expressions
  • Add @Idempotent or @NonIdempotent where appropriate

BigInteger and potential Node inlining are tracked in follow-up issues.

Important Notes

For SDKMan users:

sdk install java 17.0.7-graalce
sdk use java 17.0.7-graalce

For other users - download link can be found at https://github.com/graalvm/graalvm-ce-builds/releases/tag/jdk-17.0.7

Release notes: https://www.graalvm.org/release-notes/JDK_17/

R component was dropped from the release 23.0.0, only python is available to install via gu install python.

Checklist

Please ensure that the following checklist has been satisfied before submitting the PR:

  • Do QA on Windows - make sure that the new GraalVM runtime is downloaded, installed, and found.
  • Ensure that python and R interop tests are successful.
  • Run and compare benchmarks.
  • The documentation has been updated, if necessary.
  • All code follows the
    Scala,
    Java,
    and
    Rust
    style guides. In case you are using a language not listed above, follow the Rust style guide.
  • All code has been tested:
    • Unit tests have been written where possible.
    • Make sure benchmarks are OK

@Akirathan Akirathan linked an issue Jun 30, 2023 that may be closed by this pull request
1 task
@Akirathan Akirathan self-assigned this Jun 30, 2023
@Akirathan Akirathan added the CI: Clean build required CI runners will be cleaned before and after this PR is built. label Jun 30, 2023
build.sbt Outdated Show resolved Hide resolved
@Akirathan Akirathan added --breaking Important: a change that will break a public API or user-facing behaviour and removed --breaking Important: a change that will break a public API or user-facing behaviour labels Jul 4, 2023
Akirathan added 3 commits July 5, 2023 11:30
- Remove deprecated `ConditionProfile.createCountingProfile()`.
- Add `@Shared` to some `@Cached` parameters.
- Specialization method names should not start with execute
- Add limit attribute to some specialization methods
- Add `@NeverDefault` for some cached initializer expressions
- Add `@Idempotent` or `@NonIdempotent` where appropriate
@Akirathan
Copy link
Member Author

Windows build fails on cannot find package graalvm-ce-java11-windows-amd64-1.0.0.zip.

At first sight, this seems like another instance of dirty-caches-issue. In the last few commits, I renamed a lot of files inside runtime-version-manager-test/src/main/resources to satisfy the new versioning requirements. This changed might not been propagated to the windows runners. There was a recent clean action on the CI, so I am restarting the job now and hopefully, it will work.

@Akirathan
Copy link
Member Author

I have just checked the benchmarks, and there are no big regressions, neither improvements anywhere. Seems like the performance stays roughly the same. Which is precisely the result that we wished for.

Comment on lines -657 to +658
val regex = """graalvm-ce-java(\d+)-(.+)""".r
val regex = """graalvm-ce-java(.+)-(.+)""".r
Copy link
Member

Choose a reason for hiding this comment

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

This change is itself backwards compatible, but that means newer engine versions will be accepting Java versions (like this one we are upgrading into) that were not supported by the old runtime-version-manager.

This means that the launcher/project-manager will fail when it sees an engine with such a problematic runtime:

[error] [2023-07-19T14:13:35.009Z] [org.enso.launcher.cli.Main] A fatal error has occurred: The runtime GraalVM 23.0.0 Java 17.0.7 is already installed, but cannot be loaded due to Invalid runtime component name `graalvm-ce-java17.0.7-23.0.0`.. Until the launcher gets an auto-repair feature, please try reinstalling the runtime by uninstalling all engines that use it and installing them again, or manually removing `C:\Users\progr\AppData\Local\enso\runtime\graalvm-ce-java17.0.7-23.0.0`. (Caused by: Invalid runtime component name `graalvm-ce-java17.0.7-23.0.0`.)

While I know the launcher project is not used too much (sadly, it's really useful), I'm not sure how big of a priority it is to make sure this work. However, this also affects the project-manager - it will have a similar error when old IDE encounters such a runtime.

If we change the manifest.template.yaml:

-minimum-launcher-version: 0.2.13
-minimum-project-manager-version: 0.2.13
+minimum-launcher-version: 2023.2.1-nightly.2023.7.20
+minimum-project-manager-version: 2023.2.1-nightly.2023.7.20

It will fix that issue.

The launcher will then propose to upgrade to the tomorrows nightly (won't work today but that's a short time period):

[warn] [2023-07-19T14:40:38.253Z] [org.enso.launcher.upgrade.LauncherUpgrader.auto-upgrade] A more recent launcher version (at least 2023.2.1-nightly.2023.7.20) is required to continue.
Do you want to upgrade the launcher and continue? [Y/n] 

Similarly, the project-manager should display a slightly more friendly error message in the logs saying that to open this project a more recent version of project-manager is needed.

I think we should update the manifest to reflect these breaking changes and ensure that old launcher/project-manager don't crash and burn but display nice error messages.

Ideally, we should set the version to be some full release, not a nightly - we can bump this with a PR once we have such a release.

Copy link
Member Author

Choose a reason for hiding this comment

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

I bumped the version in 881c983. Let's see if something fails now.

@enso-bot enso-bot bot mentioned this pull request Jul 20, 2023
2 tasks
build/ci_utils/src/cache/goodie/graalvm.rs Outdated Show resolved Hide resolved
build/ci_utils/src/cache/goodie/graalvm.rs Outdated Show resolved Hide resolved
build/ci_utils/src/cache/goodie/graalvm.rs Outdated Show resolved Hide resolved
build/ci_utils/src/cache/goodie/graalvm.rs Outdated Show resolved Hide resolved
build/ci_utils/src/cache/goodie/graalvm.rs Outdated Show resolved Hide resolved
build/ci_utils/src/cache/goodie/graalvm.rs Outdated Show resolved Hide resolved
build/ci_utils/src/cache/goodie/graalvm.rs Outdated Show resolved Hide resolved
@Akirathan Akirathan added CI: Ready to merge This PR is eligible for automatic merge and removed CI: Clean build required CI runners will be cleaned before and after this PR is built. labels Jul 20, 2023
@mergify mergify bot merged commit cab6968 into develop Jul 20, 2023
@mergify mergify bot deleted the wip/akirathan/5300-graalvm-update-23-00 branch July 20, 2023 15:11
mergify bot pushed a commit that referenced this pull request Jul 23, 2023
Follow-up of recent GraalVM update #7176 that fixes downloading of GraalVM for Mac - instead of "darwin", the releases are now named "macos"

# Important Notes
Also re-enables the JDK/GraalVM version check as onLoad hook to the `sbt` process. We used to have that check a long time ago. Provides errors like this one if the `sbt` is run with a different JVM version:
```
[error] GraalVM version mismatch - you are running Oracle GraalVM 20.0.1+9.1 but GraalVM 17.0.7 is expected.
[error] GraalVM version check failed.
```
hubertp added a commit that referenced this pull request Sep 4, 2023
While looking into #7698 discovered that the `buildGraalDistribution`
task would fail to use the old (pre-23.0) GraalVM naming of artifacts.
This has changed since #7176.

The fix does not attempt to fix a problem of packaged GraalVM name in
`runtime`. That, as the ticket mentiones it, is the role of rust build
script that creates the correct bundle.
hubertp added a commit that referenced this pull request Sep 6, 2023
While looking into #7698 discovered that the `buildGraalDistribution`
task would fail to use the old (pre-23.0) GraalVM naming of artifacts.
This has changed since #7176.

The fix does not attempt to fix a problem of packaged GraalVM name in
`runtime`. That, as the ticket mentiones it, is the role of rust build
script that creates the correct bundle.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI: Ready to merge This PR is eligible for automatic merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

GraalVM Bump 23.0 to resolve BigInteger issues
8 participants