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

Changes to correct issues in 5.0.0 #249

Merged
merged 6 commits into from
Jan 28, 2025
Merged

Changes to correct issues in 5.0.0 #249

merged 6 commits into from
Jan 28, 2025

Conversation

leerho
Copy link
Contributor

@leerho leerho commented Jan 24, 2025

The Java 21 FFM API changed considerably from the Java 17 FFM API.

  • Allocation of off-heap memory is no longer performed from MemorySegment, it is now in Arena.
  • Closing of a segment is no longer performed from the MemorySegment, it must be done from the owning Arena.
  • An Arena can manage multiple segments and Arena::close() closes everything that an Arena owns.

Problems with the 5.0.0 release:

  • Although version 5.0.0 could work, the Resource class (the base of the Memory hierarchy) extended AutoCloseable, which was a subtle mistake, because one cannot close just one Memory, which wraps a MemorySegment. This could confuse the user by implying that a single Memory could be closed when it actually depends on what else the Arena is managing.
  • In Java 21, Arena extends AutoCloseable, not MemorySegment.ResourceScope of Java 17. And the Arena that is used to create a MemorySegment is not recoverable from the MemorySegment. This means that a MemorySegment plays no role in its off-heap creation and no role in closing the off-heap collection of resources under an Arena, unlike in Java 17.
  • To fix this required making sure that every path in Memory that could create off-heap memory also required providing an Arena (as well as the alignment bytes).
  • As a result, the user must choose the type of Arena appropriate for their use-case (danger: don't use Arena types that allow multi-threading, because the rest of the code is not thread-safe). This Arena is passed to Memory, and, when done, the user must close the Arena. There is no way that Memory can take on those roles. Once given an Arena to use, Memory can use it to create the underlying MemorySegment, and in certain cases, facilitate the closing.
  • Creating a memory-mapped MemorySegment is a bit more involved and Memory deals with all of that for the user.
  • Most of the changes in this PR are in the /test/ tree. Every test TWR block had to be updated to be based on an Arena, and not on a segment. A simple change, but there are a lot of them. Also where and how Arenas are closed had to be done carefully, with suitable warnings.

@leerho leerho marked this pull request as ready for review January 25, 2025 17:39
@leerho leerho changed the title Changes in main tree Changes to correct issues in 5.0.0 Jan 25, 2025
@@ -34,7 +34,7 @@ under the License.

<groupId>org.apache.datasketches</groupId>
<artifactId>datasketches-memory</artifactId>
<version>5.0.0-SNAPSHOT</version>
Copy link
Contributor

Choose a reason for hiding this comment

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

the main branch should be 5.1.0-SNAPSHOT

@leerho leerho merged commit fe21452 into main Jan 28, 2025
@leerho leerho deleted the changes_in_main_tree branch January 28, 2025 00:43
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