-
Notifications
You must be signed in to change notification settings - Fork 216
Theta, Tuple convert to Panama FFM #668
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
Currently the only usage of this is in filters.bloomfilter. The original dependency on net.openhft.hashing.LongHashFunction has been removed as it is obsolete and uses sun.misc.unsafe.
filters.BloomFilter, common.Util uses XxHash now in o.a.datasketches.hash POM aligned with ds-memory pom
ds-memory-6.1.0-SNAPSHOT.
Make use of MemorySegmentStatus where required
freakyzoidberg
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.
❤️ Love the move to native Java for memory bocks/segment handling.
Small nit as I got confused with with commented CI configs - maybe worth adding an explanation along the comment.
Otherwise did not see anything wrong in the modified code - Thanks
| paths-ignore: [ '**/*.html', '**/*.md', '**/*.txt', '**/*.xml', '**/*.yaml', '**/*.yml', '**/LICENSE', '**/NOTICE' ] | ||
| # The branches below must be a subset of the branches above | ||
| branches: [ 'main', '[0-9]+.[0-9]+.[Xx]' ] | ||
| # push: |
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.
Are those commented config intended ?
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.
Is that related to that statement maybe
We will not be able to run the GHA workflows until we have either a Java24 or Java25 release.
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.
Yes. The current state of FFM is a problem wrt LTS releases and Eclipse (and VScode). FFM in Java 21 is still in "Preview", and Eclipse ECJ compiler only supports preview with the latest Java release (24). FFM graduated to full Java integration with Java 22. The next LTS is Java 25, which comes out in September, 2025. Although Java 25ea is available, few tools support ea. So I did this development in Java 24. From examining Java 25ea I know that FFM doesn't change in any ways that would affect my use of FFM. I don't really want to release the library on Java 24 as it would only be good for a few months.
Since there is no Maven Central artifact of this code, the GHA runners cannot access it -- thus I have to disable the GHA workflows until I can actually release it, which will be after September. The commented GHA workflow "on:" configs are only until I can actually do a release, when I can enable the CI again. All of this work, and there will be more upcoming merges, is in preparation for Java 25.
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.
Thank you, thank you for your willingness to review this code!!
I am doing the conversion of the Java library in stages, as the review of such large scale changes would be nearly impossible. It will not be easy as it is.
This PR completes the conversion of the following source directories from datasketches.memory to java.lang.foreign.*:
main/..
test/..
A few other minor changes had to be made
The other sketch directories have not been touched, with a few exceptions. As a result, except for the above sketches, all the rest still may depend on DS.memory.
All the tests pass from Eclipse and from Maven.
This is all compiled with Java 24 and code coverage is still around 97%.
We will not be able to run the GHA workflows until we have either a Java24 or Java25 release.
It will be interesting to do some characterization studies of the FFM version of Theta to the current DS.memory version ... any volunteers?