RFR: 8287788: reuse intermediate segments allocated during FFM stub invocations [v9]

[Maurizio Cimadamore]

On Wed, 22 Jan 2025 10:43:27 GMT, Maurizio Cimadamore <mcimadamore at openjdk.org> wrote:

>> Matthias Ernst has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   --unnecessary annotations
>
> src/java.base/share/classes/jdk/internal/foreign/abi/BufferStack.java line 38:
> 
>> 36:         @SuppressWarnings("restricted")
>> 37:         public MemorySegment allocate(long byteSize, long byteAlignment) {
>> 38:             MemorySegment slice = backingSegment.asSlice(offset, byteSize, byteAlignment);
> 
> You have re-implemented a slicing allocator (`SegmentAllocator::slicingAllocator`). I think that's probably ok, but I'll point out that slicing allocator will try to deal with alignment, and will also throw exceptions when called with non-sensical arguments.
> 
> A more robust way to do this would be to:
> 1. have `reserve` pass the reserved size to `Frame`
> 2. `Frame` will slice the segment according to offset/size
> 3. then create a slicing allocator based on that segment
> 4. use the slicing allocator to implement `allocate`
> 
> In our tests, something like this did not add any extra overhead (the allocation of the slicing allocator is escape-analyzed away)

On another note: in principle if a Frame is not the latest returned in a given thread, it is not safe to allow its allocation method (and probably close too) to succeed. Consider this case:


Arena arenaOuter = bufferStack.reserve(100);
arenaOuter.allocate(16); // now offset is 16
...
Arena arenaInner = bufferStack.reserve(100);
arenaInner.allocate(16); // now offset is 32
arenaOuter.allocate(16); // now offset is 48 // whooops
...
arenaInner.close(); // now offset is 16
arenaOuter.allocate(16); // now offset is 32
arenaOuter.allocate(16); // now offset is 48 (overwriting!!)


The last statement in this snippet effectively overwrites the memory that has been previously allocated _by the same frame_.  This is because we allow `arenaOuter::allocate` to work even after `arenaInner` has been obtained (see statement with `whooops` comment).

Now, I believe the linker implementation is "correct" and never attempts something like this - other clients might try something similar and get surprising result. So, in the general case, I believe we should think about this (although it's fine if you do not want to tackle that in this PR)

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/23142#discussion_r1925132267