RFR: 8349146: [REDO] Implement a better allocator for downcalls [v6]

Jorn Vernee jvernee at openjdk.org
Wed Apr 30 18:03:52 UTC 2025


On Wed, 30 Apr 2025 15:33:52 GMT, Per Minborg <pminborg at openjdk.org> wrote:

>> This PR is based on the work of @mernst-github and aims to implement an _internal_ thread-local 'stack' allocator, which works like a dynamically sized arena, but with reset functionality to reset the allocated size back to a certain level. The underlying memory could stay around between calls, which could improve performance.
>> 
>> Re-allocated segments are not zeroed between allocations.
>
> Per Minborg has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Improve on comments

src/java.base/share/classes/jdk/internal/foreign/BufferStack.java line 185:

> 183:         long size = layout.byteAlignment() > JAVA_LONG.byteSize()
> 184:                 ? Utils.alignUp(layout.byteSize(), layout.byteAlignment())
> 185:                 : layout.byteSize();

We already do this kind of conditional alignment in `SegmentFactories` (see the use of `MAX_MALLOC_ALIGN`). I suggest re-using that logic, by passing the alignment to `PerThread::of`, and then forwarding it to the `arena.allocate` call. The other overload can pass `1` as the alignment.

test/jdk/java/foreign/TestBufferStack.java line 124:

> 122:                 outOfStack = hugeFrame.allocate(4);
> 123:                 assertEquals(hugeFrame.scope(), outOfStack.scope());
> 124:                 assertTrue(outOfStack.asOverlappingSlice(stackSegment).isEmpty());

Pre-existing but: I think `segment11` can be used here, and `stackSegment` is not actually needed?

test/jdk/java/foreign/TestBufferStackStress.java line 44:

> 42: import static org.junit.jupiter.api.Assertions.*;
> 43: 
> 44: public class TestBufferStackStress extends NativeTestHelper {

I don't think this test strictly needs to extend `NativeTestHelper`.

test/jdk/java/foreign/TestBufferStackStress2.java line 98:

> 96:                         System.gc();
> 97:                     }
> 98:                     segment.get(ValueLayout.JAVA_BYTE, i);

Won't this read out of bounds? `SMALL_ALLOC_SIZE` is only 8.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/24829#discussion_r2068999305
PR Review Comment: https://git.openjdk.org/jdk/pull/24829#discussion_r2068978694
PR Review Comment: https://git.openjdk.org/jdk/pull/24829#discussion_r2069007343
PR Review Comment: https://git.openjdk.org/jdk/pull/24829#discussion_r2069198088


More information about the core-libs-dev mailing list