-
Notifications
You must be signed in to change notification settings - Fork 34
Frank grimes java 21 ffm #242
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
Conversation
The project's minimum Java release requirements moved from Java 17 to Java 21. Package imports changed from `jdk.incubator.foreign` -> `java.lang.foreign` and a number of API changes were required. Notably, the new `java.lang.foreign.Arena` class needs to be exposed when allocating native memory regions as it controls the scope and lifecycle of allocations. Respective API docs are as follows: - https://docs.oracle.com/en/java/javase/17/docs/api/jdk.incubator.foreign/jdk/incubator/foreign/package-summary.html - https://docs.oracle.com/en/java/javase/21/docs/api/java.base/java/lang/foreign/package-summary.html
This is to try to address file permission errors seen in some GitHub Action Workflow runs on Windows: "[INFO] Error: Failures: Error: LeafImplTest.checkMapLeafs:118 » AccessDenied TestFile2.bin Error: SpecificLeafTest.checkMapLeafs:108 » AccessDenied TestFile2.bin"
This is to try to address following errors seen in some GitHub Action Workflow runs on Windows: "Warning: Files with unapproved licenses: force_original.txt"
Migrate to Java 21 FFM API
Mostly Checkstyle issues
Had to upgrade my Java 21 JDK to Temurin 21.0.5+11 due to a discovered bug in Java.
README.md
Outdated
| ### *NOTE: The DataSketches Java Memory Component is not thread-safe.* | ||
| ### *NOTES:* | ||
| * The DataSketches Java Memory Component is not thread-safe. | ||
| * I recommend Eclipse Adoptium/Temurin 21.0.5+11 or later as earlier releases of 21 have bugs that affect this product. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
first person recommendation looks strange
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed.
| long v = wbuf.getLong(); | ||
| assertEquals(v, i); | ||
| } | ||
| for (int i = 0; i < n; i++) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what happened to alignment here and below? tabs used?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wow! this is weird. No tabs used. Not sure how this happened. We have had two folks editing these files recently, perhaps they used a different indention scheme.
| wbuf.getLongArray(arr2, 0, n); | ||
| for (int i = 0; i < n; i++) { | ||
| assertEquals(arr2[i], i); | ||
| wbuf.putLongArray(arr, 0, n); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
seems like some problem with alignment all over the place
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This occurred in a bunch of files. I went through all of them and I think it is OK now.
AlexanderSaydakov
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see some formatting problems, but I don't think they should stop this effort
Upgrade much of the API that was based on Java 17 to use new Java 21 FFM API, which is in "preview" mode.
Also was able to remove some duplicated code.
Updated POM, as well as the GH Actions workflows.
This PR upgrades the Main branch to Java 21.
Note: I found a bug in Temurin Java 21.0.2 JDK and had to upgrade to Temurin Java 21.0.2+11 release.