RFR: 8357531: The `SegmentBulkOperations::fill` method can be improved using overlaps [v8]

[Maurizio Cimadamore]

On Thu, 5 Jun 2025 19:05:34 GMT, Per Minborg <pminborg at openjdk.org> wrote:

>> This PR builds on a concept John Rose told me about some time ago. Instead of combining memory operations of various sizes, a single large and skewed memory operation can be made to clean up the tail of remaining bytes.
>> 
>> This has the effect of simplifying and shortening the code. The number of branches to evaluate is reduced.
>> 
>> It should be noted that the performance of the fill operation affects the allocation of new segments (as they are zeroed out before being returned to the client code).
>> 
>> This PR passes tier1, tier2, and tier3 on multiple platforms.
>
> Per Minborg has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Use a fixed threashold for fill

src/java.base/share/classes/jdk/internal/foreign/SegmentBulkOperations.java line 55:

> 53: 
> 54:     // All the threshold values below MUST be a power of two and greater or equal to 2^3.
> 55:     private static final int NATIVE_THRESHOLD_FILL = 32;

Is there any reason you removed this property? While I understand that performance can regress for bigger sizes, taking away the property seems harsh/asymmetric?

test/micro/org/openjdk/bench/java/lang/foreign/SegmentBulkFill.java line 104:

> 102:     @Benchmark
> 103:     public void heapSegmentFillUnsafe() {
> 104:         SCOPED_MEMORY_ACCESS.setMemory(heapSegment.sessionImpl(), heapSegment.unsafeGetBase(), heapSegment.unsafeGetOffset(), heapSegment.byteSize(), (byte) 0);

I think I would prefer just using Unsafe here. You can do that by saving the segment address in a `long` non-final field, and then use that field as the unsafe offset for the unsafe access operation. No need to use internal routines and/or ScopedMemoryAccess. I understand that, perhaps, using ScopedMemoryAccess allows you to include the cost of the liveness check, but I think using plain Unsafe as a reference is what this (and other similar benchmarks) should be doing.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/25383#discussion_r2138344248
PR Review Comment: https://git.openjdk.org/jdk/pull/25383#discussion_r2138343109