panama memory mapping #1678
panama memory mapping #1678
Conversation
SteffenHeu
commented
Feb 29, 2024
- drop in replacement, using project panama's memory segments.
- memory segment for now converted to bytebuffer to serve as drop-in
# Conflicts: # mzmine-community/src/main/java/io/github/mzmine/main/TmpFileCleanup.java # mzmine-community/src/main/java/io/github/mzmine/util/MemoryMapStorage.java
create sparse temp file memory map method
| @NotNull final double[] values) { | ||
| if (values.length == 0) { | ||
| return AbstractStorableSpectrum.EMPTY_BUFFER; | ||
| } |
There was a problem hiding this comment.
this is removed for DoubleBuffer but not in storeValuesToFloatBuffer. Harmonize?
| } | ||
|
|
||
| @NotNull | ||
| private MemorySegment __storeData(@NotNull Object data, ValueLayout layout, int length) { |
There was a problem hiding this comment.
since when do we do __ for private methods? Mostly know it from Python
There was a problem hiding this comment.
it's also a pattern in some jdk methods to emphasize that it is meant for internal use only. The thing is that i would not want any other class to directly invoke this method. I'll also make it final in the next commit
| // success, store in actual mapped file | ||
| final MemorySegment memorySegment = allocator.allocate(layout, length); | ||
| MemorySegment.copy(data, 0, memorySegment, layout, 0, length); | ||
| return memorySegment; |
There was a problem hiding this comment.
all this code is duplicated from above and makes it complicated.
Just do allocator = null and recall the same method and it will create a new allocator and run the same logic.
There was a problem hiding this comment.
I wanted to achieve two things here: 1. lazy initialisation (first call) and 2. no recursive calls to createMappedFile to not try to create more files than necessary if the creation does not work. This way there is exactly one retry, otherwise the data is stored into ram. If you have a better pattern for that I'm happy to implement it. Recursive calls are elegant, but they also complicate the code sometimes. In such fundamental things 8 duplicate lines are fine imo if they serve the purpose of avoiding some complexity/nested code.
One thing that might be debatable could be using the same Arena.ofAuto for the whole storage, so we create less Arenas if this method fails.
There was a problem hiding this comment.
I think it is ok. My only idea was to do recursive but its true one would have to be sure that all the other error handling catches issues that may create infinite loops.
Creating and keeping a single Arena might be good