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

Maurizio Cimadamore mcimadamore at openjdk.org
Thu May 1 10:24:49 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

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

> 190:                 assertEquals(firstAddress, segment.address());
> 191:                 var segmentTwo = arena.allocate(JAVA_INT);
> 192:                 assertNotEquals(firstAddress, segmentTwo.address());

Should this check be stronger -- e.g. that the address of the second segment should be the address of the first + the size of JAVA_INT?

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

> 201: 
> 202:     @Test
> 203:     void allocationCaptureStateLayout() {

This test is not really testing much re. capture state layout. It feels like the only added thing w.r.t. `allocationSameAsPoolSize` is that it also checks that additional allocation will fail with IOOBE -- maybe these two tests should be merged?

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

> 239:     @Test
> 240:     void closeConfinement() {
> 241:         var pool = BufferStack.of(POOL_SIZE);

Nice confinement tests!

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

> 26:  * @modules java.base/jdk.internal.foreign
> 27:  * @build NativeTestHelper TestBufferStackStress
> 28:  * @run junit/othervm --enable-native-access=ALL-UNNAMED TestBufferStackStress

is `--enable-native-access` needed here (and also in the other stress test) ?

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

> 43: import java.util.concurrent.atomic.AtomicBoolean;
> 44: 
> 45: final class TestBufferStackStress2 extends NativeTestHelper {

This also extends `NativeTestHelper` -- does it need to?

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

PR Review Comment: https://git.openjdk.org/jdk/pull/24829#discussion_r2070124607
PR Review Comment: https://git.openjdk.org/jdk/pull/24829#discussion_r2070126160
PR Review Comment: https://git.openjdk.org/jdk/pull/24829#discussion_r2070126843
PR Review Comment: https://git.openjdk.org/jdk/pull/24829#discussion_r2070129639
PR Review Comment: https://git.openjdk.org/jdk/pull/24829#discussion_r2070129263


More information about the core-libs-dev mailing list