You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@datasketches.apache.org by GitBox <gi...@apache.org> on 2023/01/14 01:40:58 UTC
[GitHub] [datasketches-memory] leerho opened a new pull request, #170: Master tempfix
leerho opened a new pull request, #170:
URL: https://github.com/apache/datasketches-memory/pull/170
While working on the Java 17 migration, I discovered a bug in memory in how regions were being handled. It only occurs in rare cases but it was a bug nonetheless. And in digging in to figure it out I decided I did not like the way the transitions were being handled between Native Order and Non-native Order, Memory to Buffer, Buffer to Memory, to duplicates and to regions. It was more complicated than it needed to be and the logic was spread all over the place making it really difficult to understand and debug. So I decided to byte-the-bullet and rewrite that portion of the code. Which, unfortunately affects nearly all the classes :(
The only changes to the API was to the base interface class (currently called BaseState), which held some common methods that applied to all the memory variations (heap, off-heap, m-maps and ByteBuffer resources, plus Regions, Buffers and Duplicates.). The central API classes: Memory, WritableMemory, Buffer and WritableBuffer were not affected at all. Within this BaseState class are the following API changes.
- getCumulativeOffset() and getCumulativeOffset(offset). I removed these two methods because they should never have been in the public API in the first place. These are used internally for accessing off-heap memory and contain actual off-heap memory addresses, which is never a good idea to expose. They are not useful at all to the user. I don't see any way they could be of any use.
- getTypeByteOrder() was changed to getByteOrder(). The concept of "types" is used internally to track the some 127 different variations of how the Memory can be constructed. Having the word "type" is confusing to the user as it isn't used anywhere else in the API and begs the question "type of what?".
- getRegionOffset() was changed to getTotalOffset(). This is a useful concept: If the user has constructed a Region within a Region within a Region, say, the getTotalOffset() provides the total offset of the queried region to the start of the underlying resource. The name getRegionOffset is misleading as it doesn't describe "to what?"
I don't want to do a major release on this, but if there is pushback, I am willing to add back getTypeByteOrder() and getRegionOffset() as redirects to the new names.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@datasketches.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@datasketches.apache.org
For additional commands, e-mail: commits-help@datasketches.apache.org
[GitHub] [datasketches-memory] leerho merged pull request #170: Master tempfix
Posted by "leerho (via GitHub)" <gi...@apache.org>.
leerho merged PR #170:
URL: https://github.com/apache/datasketches-memory/pull/170
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@datasketches.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@datasketches.apache.org
For additional commands, e-mail: commits-help@datasketches.apache.org
[GitHub] [datasketches-memory] leerho commented on pull request #170: Master tempfix
Posted by GitBox <gi...@apache.org>.
leerho commented on PR #170:
URL: https://github.com/apache/datasketches-memory/pull/170#issuecomment-1382632142
And "mvn clean test" runs OK locally.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@datasketches.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@datasketches.apache.org
For additional commands, e-mail: commits-help@datasketches.apache.org
[GitHub] [datasketches-memory] leerho commented on pull request #170: Master tempfix
Posted by GitBox <gi...@apache.org>.
leerho commented on PR #170:
URL: https://github.com/apache/datasketches-memory/pull/170#issuecomment-1382631591
This is complaining that Java 11 is an invalid version. Not sure yet how to fix.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@datasketches.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@datasketches.apache.org
For additional commands, e-mail: commits-help@datasketches.apache.org