-
Enhancement
-
Resolution: Fixed
-
P4
-
None
When reviewing the changes by Panama in the methods of FileChannelImpl for mmapping files to MemorySegemnt or MappedByteBuffer, I found some inconsistencies that should be cleaned up. The implementation details on Windows or Linux require to have aligned offsets into the file, so the code mmaps larger slices than requested to fit alignment constraints.
Currently it is confusing:
- New code returning MemorySegment uses unmapper.address() which handles also adds the offset alignment.
- Old code returning MappedByteBuffer uses unmapper.address + unmapper.pagePosition (private fields in unmapper) directly. It ias also not easy to understand how the aligned size vs. user-requested size if used.
This causes confusion for somebody reviewing or trying to understand the code: See the mailing list posts:
https://mail.openjdk.java.net/pipermail/panama-dev/2022-May/016981.html
https://mail.openjdk.java.net/pipermail/panama-dev/2022-May/016990.html
I propose to make the code of both methods align to each other and use the accessor methods in the Unmapper class. Maybe the Unmapper class should be renamed, as the name does not follow function. Unmapping is just one usecase of it.
Currently it is confusing:
- New code returning MemorySegment uses unmapper.address() which handles also adds the offset alignment.
- Old code returning MappedByteBuffer uses unmapper.address + unmapper.pagePosition (private fields in unmapper) directly. It ias also not easy to understand how the aligned size vs. user-requested size if used.
This causes confusion for somebody reviewing or trying to understand the code: See the mailing list posts:
https://mail.openjdk.java.net/pipermail/panama-dev/2022-May/016981.html
https://mail.openjdk.java.net/pipermail/panama-dev/2022-May/016990.html
I propose to make the code of both methods align to each other and use the accessor methods in the Unmapper class. Maybe the Unmapper class should be renamed, as the name does not follow function. Unmapping is just one usecase of it.