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 JDK/JRE + Gradle #1237

Merged
merged 4 commits into from
Feb 17, 2024
Merged

Update JDK/JRE + Gradle #1237

merged 4 commits into from
Feb 17, 2024

Conversation

skovati
Copy link
Contributor

@skovati skovati commented Nov 8, 2023

Description

Updates JDK/JRE to 21, as well as Gradle to 8.4 8.6.

Updates sim engine to only use virtual thread per-task executor. Removes a test that checks thread count in the sim driver since that fails with virtual threads. @Twisol

Verification

Just 👀'ing at e2e-tests

Future work

A few TODOs are now unblocked by this JDK update. (e.g. rg "TODO" | rg "21")

@skovati skovati added the build Changes that affect the build system or external dependencies label Nov 8, 2023
@skovati skovati self-assigned this Nov 8, 2023
@skovati skovati requested a review from Twisol November 8, 2023 18:33
@dandelany dandelany mentioned this pull request Nov 9, 2023
@skovati
Copy link
Contributor Author

skovati commented Nov 29, 2023

Manual testing has shown:

  • JDK 19 built models run fine on Aerie w/ JRE 19
  • JDK 19 built models run fine on Aerie w/ JRE 21
  • JDK 21 built models DO NOT run on Aerie w/ JRE 19
  • JDK 21 built models run fine on Aerie w/ JRE 21

So, upgrading the JRE of aerie-* container images should be backwards compatible, but mission modelers will need to hold off on upgrading to JDK 21 until their target venues are also running JRE 21.

@skovati skovati marked this pull request as ready for review November 29, 2023 00:34
@skovati skovati requested a review from a team as a code owner November 29, 2023 00:34
@skovati skovati requested review from dandelany and jmdelfa November 29, 2023 00:34
@skovati skovati force-pushed the build/update-jdk-21 branch from 09021de to db46a8c Compare November 29, 2023 00:35
@camargo
Copy link
Member

camargo commented Dec 6, 2023

We should also update https://github.com/NASA-AMMOS/aerie-mission-model-template as well :)

@dandelany
Copy link
Collaborator

Reviewers of this PR should:

  • checkout the PR
  • try to build Aerie on your local machine
  • see if you run into any issues with gradle trying to automatically update the toolchain
  • document any additional steps needed to get it working

@dandelany
Copy link
Collaborator

Pending a couple questions that @skovati will look into:

  • Can/should we replace the thread release test with something new supported by Loom?
  • Can we upgrade to latest gradle (now >8.4)

@dandelany
Copy link
Collaborator

@Mythicaeda will test this with building the Clipper model in JDK 21

@Mythicaeda
Copy link
Contributor

Mythicaeda commented Feb 16, 2024

I've tested building the Clipper model against this branch. Once I was able to get the Clipper model to accept Java 21 artifacts, it compiled on J21 and simulated, although I do not do a deep dive into whether the results were the same as the version compiled on J19 using J19 Aerie artifacts (I did check a couple views, though, and didn't see any differences).

However, in order to get Clipper to build I had to change the Clipper build process, as they specifically look for Java 19 artifacts and reject JARs compiled on other Java versions. This means that the version of Aerie that includes the J21 upgrade will be breaking to Clipper.

Details for how to update Clipper model to support JDK 21:

  • Update Grade wrapper to one that supports J21 (I chose 8.6)
  • Update the Java sourceCompatibilty/targetCompatability statements in gov.nasa.jpl.europa.clipper.aerie.adaptations.java-common-conventions.gradle (I chose to remove these for the sake of getting a build working, but a more delicate change should work)

Copy link
Contributor

@Mythicaeda Mythicaeda left a comment

Choose a reason for hiding this comment

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

I've tested locally and it LGTM!

@dandelany
Copy link
Collaborator

Merging to include with 2.4.0 release - thanks!

@dandelany dandelany merged commit b82dd7d into develop Feb 17, 2024
6 checks passed
@dandelany dandelany deleted the build/update-jdk-21 branch February 17, 2024 00:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build Changes that affect the build system or external dependencies clipper-launch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Upgrade to JRE 21
6 participants