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

svm: fix for JDK 21.0.4 after the downport of "8315373: Change VirtualThread to unmount after freezing, re-mount before thawing" and dependnet changes #1

Closed
wants to merge 5 commits into from

Conversation

simonis
Copy link
Contributor

@simonis simonis commented Mar 13, 2024

This change fixes svm after the downport of JDK-8315373 (which included JDK-8312498, JDK-8312777, JDK-8321270, JDK-8322818, JDK-8323002, JDK-8323296, JDK-8316924) to OpenJDK 21.0.4. It does this by downporting

and adopting them to work on JDK 21.

I'm not sure if this fix should be done in this repository or in the release/graal-vm/23.1 branch of the oracle/graal repository which currently matches the master branch of this repository. I don't understand how these two branches/repositories are related, if one is synced into the other automatically or if one of them is abandoned. For any case I've also submitted this PR to oracle/graal as well (see PR). Feel free to close one of them but please be so kind and explain the relationship of the two branches/repositories :)

zapster and others added 4 commits March 12, 2024 18:45
@fniephaus
Copy link
Member

Thanks for the contribution, @simonis! This is the first backport PR, so please give us some time to discuss and communicate the community backporting process. In the meantime, I ran the gates and a style gate is failing with:

The import org.graalvm.compiler.serviceprovider.JavaVersionUtil is never used

Could you please fix this?

Feel free to close one of them but please be so kind and explain the relationship of the two branches/repositories :)

I have closed the other PR that you opened against the JDK 21 release branch of oracle/graal. I believe we will keep it around but it is not maintained anymore. Instead, we use this repository to allow the community to request GraalVM for JDK 21 backports, as @ezzarghili mentioned in oracle/graal#8562 (comment).

I see you also asked more questions in oracle/graal#8562 (comment). Let's discuss those and more general questions about the community backporting process on Slack (I see you've posted there as well).

@simonis
Copy link
Contributor Author

simonis commented Mar 18, 2024

Hi @fniephaus,

Thanks a lot for taking care of this PR and the also of the more general question about GraalVM community support for JDK 21. I've fixed the style issue because of the unused import statement but I'm unsure about the other five failing tests related to com.oracle.truffle.api.test.polyglot.FileSystemsTest#testSetAttribute. Is this an error introduced by my change or a more general problem? I've now also enabled GitHub Actions on my personal fork of this repository to see such issues earlier in the future.

@fniephaus
Copy link
Member

Hi @simonis, thanks for fixing the style gate. I've already reached out to the Truffle team regarding the failing FileSystemsTest#testSetAttribute test. It currently seems unrelated to your change. If it is related, I'll let you know.

@jerboaa
Copy link
Contributor

jerboaa commented Mar 18, 2024

@simonis FWIW, 21.0.4 is the July update. We should merge PRs like this one only once we have all the work for the 21.0.3 update completed. This also gives us opportunity to actually produce builds and test with 21.0.4 EA JDK builds as well.

@jerboaa
Copy link
Contributor

jerboaa commented Mar 18, 2024

@simonis FWIW, 21.0.4 is the July update. We should merge PRs like this one only once we have all the work for the 21.0.3 update completed. This also gives us opportunity to actually produce builds and test with 21.0.4 EA JDK builds as well.

Or at least some verification that both releases keep working (which we currently don't have for development releases overlapping with the unreleased update one cycle earlier).

@fniephaus
Copy link
Member

Regarding the FileSystemsTest: JDK-8316304, which happens to be created by @jerboaa 😉, required this Truffle change and it was backported to JDK 21 and earlier. So it seems you have found another backport candidate :)

@fniephaus
Copy link
Member

We should merge PRs like this one only once we have all the work for the 21.0.3 update completed.

Actually, I'd say it makes sense to integrate this when the JDK version in the common.json is bumped to 21.0.4. That's also when the fix is really needed and put to work. Currently, it's behind a JDK21u3OrEarlier predicate, which works on the update level. We typically do not use update-level predicates due to the increased amount of maintenance and testing work all of this requires. When the version is bumped to 21.0.4, the predicate is no longer needed.

@jerboaa
Copy link
Contributor

jerboaa commented May 16, 2024

@ezzarghili What's your thought on this backport? OK to merge? We are in the 21.0.4 dev cycle now. Are you still going to update this repo for GraalVM for JDK 21 community?

@simonis Please fix the style issues:

2 files were modified
 - src/com.oracle.svm.core/src/com/oracle/svm/core/jdk/JDK21u3OrEarlier.java
Changes:
--- a/src/com.oracle.svm.core/src/com/oracle/svm/core/jdk/JDK21u3OrEarlier.java
+++ b/src/com.oracle.svm.core/src/com/oracle/svm/core/jdk/JDK21u3OrEarlier.java
@@ -31,7 +31,7 @@
 public class JDK21u3OrEarlier implements BooleanSupplier {
 
     public static final boolean jdk21u3OrEarlier = JavaVersionUtil.JAVA_SPEC < 21 ||
Formatter modified files - run "mx eclipseformat", check in changes and repush
-        (JavaVersionUtil.JAVA_SPEC == 21 && Runtime.version().update() <= 3);
+                    (JavaVersionUtil.JAVA_SPEC == 21 && Runtime.version().update() <= 3);
 
     @Override
     public boolean getAsBoolean() {

 - src/com.oracle.svm.core/src/com/oracle/svm/core/jdk/JDK21u4OrLater.java
Changes:
--- a/src/com.oracle.svm.core/src/com/oracle/svm/core/jdk/JDK21u4OrLater.java
+++ b/src/com.oracle.svm.core/src/com/oracle/svm/core/jdk/JDK21u4OrLater.java
@@ -31,7 +31,7 @@
 public class JDK21u4OrLater implements BooleanSupplier {
 
     public static final boolean jdk21u4OrLater = JavaVersionUtil.JAVA_SPEC > 21 ||
-        (JavaVersionUtil.JAVA_SPEC == 21 && Runtime.version().update() >= 4);
+                    (JavaVersionUtil.JAVA_SPEC == 21 && Runtime.version().update() >= 4);
 
     @Override
     public boolean getAsBoolean() {

@simonis
Copy link
Contributor Author

simonis commented Jun 4, 2024

Closing this PR in favor of #4

@simonis simonis closed this Jun 4, 2024
@simonis simonis mentioned this pull request Aug 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
OCA Verified All contributors have signed the Oracle Contributor Agreement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants