Skip to content

Conversation

@leerho
Copy link
Contributor

@leerho leerho commented Jun 28, 2025

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/..

  • fdt
  • theta
  • thetacommon
  • tuple/*

test/..

  • fdt
  • theta
  • thetacommon
  • tuple/*

A few other minor changes had to be made

  • we had some duplicate "common" methods between the ../thetacommon/ThetaUil and ../common/Util classes. These were consolidated into ../common/Util.
  • Some minor API changes had to be made, but they are pretty obvious. (e.g., getMemory() -> getMemorySegment(),...)
  • Internal variable names, such as "mem" for Memory were changed to "seg" for MemorySegment, etc.

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%.

  • FFM in Java 21 is still in Preview, and doing dev work with a Preview mode that is not suppored by Eclipse is really messy.
  • Java 25 will be out in September. I expect little to no changes in my use of FFM in Java 24 to Java 25.
  • Note that FFM will enable more optimizations in the code (e.g., entire classes removed, ...), but we can reserve that for later.

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?

@leerho leerho requested a review from nikunjbhartia June 28, 2025 01:16
Copy link
Member

@freakyzoidberg freakyzoidberg left a 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:
Copy link
Member

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 ?

Copy link
Member

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.

Copy link
Contributor Author

@leerho leerho Jun 28, 2025

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.

Copy link
Contributor Author

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!!

@leerho leerho merged commit 882eb45 into main Jun 28, 2025
@leerho leerho deleted the thetaRework branch June 28, 2025 18:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants