On Wed, 11 Dec 2024 19:29:13 GMT, Quan Anh Mai <[email protected]> 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.
>
> Quan Anh Mai has updated the pull request incrementally with two additional
> commits since the last revision:
>
> - wrong init
> - move segment instance creation to SegmentFactories
test/micro/org/openjdk/bench/java/lang/foreign/AllocTest.java line 77:
> 75: var freeAddr = lookup.findOrThrow("free");
> 76: CALLOC = linker.downcallHandle(callocAddr,
> FunctionDescriptor.of(ValueLayout.JAVA_LONG, ValueLayout.JAVA_LONG,
> ValueLayout.JAVA_LONG));
> 77: FREE = linker.downcallHandle(freeAddr,
> FunctionDescriptor.ofVoid(ValueLayout.JAVA_LONG));
I'm not sure these changes are needed. It seems the main goal here is to avoid
the cost of the capture associated with `CLayouts::freeMemory` ? If so, can't
we just store that consumer into a static final and call it a day? I think then
we could avoid the `static` init, and leave most of the code unchanged, except
for the additional static field?
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/22610#discussion_r1886612756
