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 22.3.1 JDK17 #6750

Merged
merged 17 commits into from
May 24, 2023

Conversation

Akirathan
Copy link
Member

@Akirathan Akirathan commented May 18, 2023

Fixes #6274

Pull Request Description

Upgrade GraalVM to 22.3.2 based on JDK17.

How to Upgrade:
For SDKMan users:

sdk install java 22.3.1.r17-grl
sdk default java 22.3.1.r17-grl

For anybody else, just download the GraalVM distribution manually from https://github.com/graalvm/graalvm-ce-builds/releases/tag/vm-22.3.1 and set JAVA_HOME accordingly.

Important Notes

Checklist

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

  • Run benchmarks and ensure no regressions.
  • Try to install GraalPy and FastR and run interop tests
  • 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.

@Akirathan Akirathan linked an issue May 18, 2023 that may be closed by this pull request
@Akirathan Akirathan added the CI: Clean build required CI runners will be cleaned before and after this PR is built. label May 18, 2023
@Akirathan
Copy link
Member Author

Akirathan commented May 19, 2023

I have tried:

  • Building native image of project-manager and launcher, and running some Enso code.
  • Yesterday's book club without any issues - AKKA appears to work on JDK17.
  • Dry-run benchmarks on CI work and normal benchmarks work on my machine.

Failing tests on the CI:

GitHub
Hybrid visual and textual functional programming. Contribute to enso-org/enso development by creating an account on GitHub.

@Akirathan Akirathan self-assigned this May 19, 2023
@Akirathan Akirathan marked this pull request as ready for review May 19, 2023 07:46
@4e6
Copy link
Contributor

4e6 commented May 19, 2023

How is the benchmarks?

Copy link
Contributor

@4e6 4e6 left a comment

Choose a reason for hiding this comment

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

🎉

@Akirathan
Copy link
Member Author

How is the benchmarks?

Do you think it is necessary to check benchmarks before integration? I guess that I should probably run them, at least from curiosity ..

@JaroslavTulach
Copy link
Member

JaroslavTulach commented May 19, 2023

There are two groups of failing tests:

@Akirathan
Copy link
Member Author

Akirathan commented May 22, 2023

Apparently, there are no FastR, neither GraalPy releases for 22.3.2. I am reverting to 22.3.1 so that we can get interoperability with FastR and GraalPy.

GitHub
A high-performance implementation of the R programming language, built on GraalVM. - oracle/fastr

@Akirathan
Copy link
Member Author

Reverted to GraalVM JDK17 22.3.1 in 84a9e65

@Akirathan
Copy link
Member Author

Akirathan commented May 22, 2023

Raised a question on GraalVM Slack polyglot channel "Why are FastR and GraalPy components not available on GraalVM JDK17 22.3.2" in https://graalvm.slack.com/archives/CP6RTC4LT/p1684762240737959

@Akirathan Akirathan changed the title Update GraalVM to 22.3.2 JDK17 Update GraalVM to 22.3.1 JDK17 May 22, 2023
@Akirathan
Copy link
Member Author

Package IDE (macos) job fails in signing the .dmg package with an output given here

The executable does not have the hardened runtime enabled.

@Akirathan
Copy link
Member Author

Akirathan commented May 23, 2023

Scheduled benchmarks in https://github.com/enso-org/enso/actions/runs/5056857474

GitHub
Hybrid visual and textual functional programming. Contribute to enso-org/enso development by creating an account on GitHub.

@Akirathan
Copy link
Member Author

Akirathan commented May 23, 2023

Scheduled benchmarks in https://github.com/enso-org/enso/actions/runs/5056857474

Benchmark results are fine. I will get the MacOS signature working and merge this PR.

GitHub
Hybrid visual and textual functional programming. Contribute to enso-org/enso development by creating an account on GitHub.

Copy link
Contributor

@somebody1234 somebody1234 left a comment

Choose a reason for hiding this comment

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

just reviewing the single line of TS code. it's too long (131 chars > 100), but it might be easier if i run the formatter on it

(edit: formatter has been run.)

@somebody1234
Copy link
Contributor

somebody1234 commented May 23, 2023

weird, prettier is fine with that line

edit: oh, just forgot to pull

@Akirathan
Copy link
Member Author

weird, prettier is fine with that line

edit: oh, just forgot to pull

Thanks!

@@ -307,7 +307,7 @@ spec_with_numeric_type name numeric_type =

Test.specify "Column with locale" <|
input = Column.from_vector "values" ["100000000", "2222", "3"] . parse numeric_type
expected = Column.from_vector "values" ["100 000 000,00", "2 222,00", "3,00"]
expected = Column.from_vector "values" ["100000000,00", "2222,00", "3,00"]
Copy link
Member Author

Choose a reason for hiding this comment

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

This is a change of a Unicode symbol for a whitespace.

Copy link
Member Author

Choose a reason for hiding this comment

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

The old Unicode character was U+A0 and the new one is U+202F

Comment on lines +17 to +18
val optRComponent =
if (os.hasRSupport) Seq(GraalVMComponent.R) else Seq()
Copy link
Member Author

Choose a reason for hiding this comment

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

This is a separate fix that I noticed recently from @xvcgreg's logs from another issue. Apparently, we try to install FastR on Windows, but that is not possible - it is not supported on Windows. Same case as with GraalPy, but FastR will, most probably, never be supported on Windows.

Copy link
Member

@jdunkerley jdunkerley left a comment

Choose a reason for hiding this comment

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

LGTM

@Akirathan Akirathan merged commit bd70ed6 into develop May 24, 2023
@Akirathan Akirathan deleted the wip/akirathan/upgrade-graal-jdk17-6274 branch May 24, 2023 08:39
Procrat added a commit that referenced this pull request May 30, 2023
…le-6756-6804

* develop: (22 commits)
  Coalesce graph editor view invalidations (#6786)
  Append warnings extracted before tail call execution (#6849)
  Detect and override hooks of the same kind (#6842)
  Dynamic app resampling and better performance measurements. (#6595)
  Show spinner when opening/creating a project, take #2 (#6827)
  Infrastructure for testing inter project imports and exports (#6840)
  Only initialise visualisation chooser if it is used. (#6758)
  Allow casting a Mixed column into a concrete type (#6777)
  Stop graph editing when in full-screen visualization mode (#6844)
  Handle `show-dashboard` event (#6837)
  Fix some dashboard issues (#6668)
  Fix JWT leak (#6815)
  Fix "set username" screen (#6824)
  Fallback to opened date when ordering projects (#6814)
  Various test improvements to increase coverage and speed things up (#6820)
  do not activate nested dropdowns together (#6830)
  Clearly select single specialization with enum dispatch pattern (#6819)
  Prevent incorrect application of list widget on incompatible expressions (#6771)
  Update GraalVM to 22.3.1 JDK17 (#6750)
  Import/export syntax error have more specific messages (#6808)
  ...
mergify bot pushed a commit that referenced this pull request Jul 20, 2023
- Previous GraalVM update: #6750

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`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI: Clean build required CI runners will be cleaned before and after this PR is built.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Upgrade to GraalVM based on JDK17
7 participants