On Fri, 6 Dec 2024 18:33:33 GMT, Jorn Vernee <jver...@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/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). In fact, except for the `alignUp` call, I'm not sure I see enough difference between the two code paths to warrant splitting out the code to a new method? AFAIU, the split was made to avoid playing with slicing in the hot path - but if we create the segment as the very last thing we do in `allocateNative`, do we care? ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/22610#discussion_r1875929133
