RFR: 8345687: Improve the implementation of SegmentFactories::allocateSegment

[Jorn Vernee]

On Fri, 6 Dec 2024 16:30:47 GMT, Quan Anh Mai <qamai at openjdk.org> wrote:

> Hi,
> 
> This patch improves the performance of a typical `Arena::allocate` in several ways:
> 
> - Delay the creation of the NativeMemorySegmentImpl. This avoids the merge of the instance with the one obtained from the call in the uncommon path, increasing the chance the object being scalar replaced.
> - Split the allocation of over-aligned memory to a slow-path method.
> - Align the memory to 8 bytes, allowing faster zeroing.
> - Use a dedicated method to zero the just-allocated native memory, reduce code size and make it more straightforward.
> - Make `VM.pageAlignDirectMemory` a `Boolean` instead of a `boolean` so that `false` value can be constant folded.
> 
> Please take a look and leave your reviews, thanks a lot.

src/java.base/share/classes/jdk/internal/foreign/ArenaImpl.java line 53:

> 51:         Utils.checkAllocationSizeAndAlign(byteSize, byteAlignment);
> 52:         long address = SegmentFactories.allocateNative(byteSize, byteAlignment, session, shouldReserveMemory, false);
> 53:         return new NativeMemorySegmentImpl(address, byteSize, false, session);

Could you move this constructor call (and the one below) to `allocateNative`? All segment construction calls are currently in `SegmentFactories` as a measure to avoid bootstrap cycles, which we had problems with in the past.

src/java.base/share/classes/jdk/internal/foreign/SegmentFactories.java line 213:

> 211: 
> 212:     @DontInline
> 213:     private static long allocateNativeOveraligned(long byteSize, long byteAlignment, MemorySessionImpl sessionImpl,

This would always make the session escape in the case of over aligned allocation. Maybe it's possible to move the call to `addCleanupIfFail` to `allocateNative` instead? (that seems to be the only use of `sessionImpl` here).

test/micro/org/openjdk/bench/java/lang/foreign/AllocTest.java line 87:

> 85:     }
> 86: 
> 87:     public static class CallocArena implements Arena {

The following changes seem to be here to support 32-bit platforms? Could you please do that in a separate PR?

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

PR Review Comment: https://git.openjdk.org/jdk/pull/22610#discussion_r1873822362
PR Review Comment: https://git.openjdk.org/jdk/pull/22610#discussion_r1873826657
PR Review Comment: https://git.openjdk.org/jdk/pull/22610#discussion_r1873831139