RFR: 8338967: Improve performance for MemorySegment::fill [v4]

Per Minborg pminborg at openjdk.org
Mon Aug 26 13:34:06 UTC 2024


On Mon, 26 Aug 2024 12:50:53 GMT, Francesco Nigro <duke at openjdk.org> wrote:

>> Per Minborg has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Fix typo
>
> src/java.base/share/classes/jdk/internal/foreign/AbstractMemorySegmentImpl.java line 200:
> 
>> 198:             switch ((int) length) {
>> 199:                 case 0 : checkReadOnly(false); checkValidState(); break; // Explicit tests
>> 200:                 case 1 : set(JAVA_BYTE, 0, value); break;
> 
> beware using a switch, because if this code if is too big to be inlined (or we're unlucky) will die due to branch-mispredict in case the different "small fills" are unstable/unpredictable.
> Having a test which feed different fill sizes per each iteration + counting branch misses, will reveal if the improvement is worthy even with such cases

It is true, that this is a compromise where we give up inline space, code-cache space, and added complexity against the prospect of better small-size performance. Depending on the workload, this may or may not pay off. In the (presumably common) case where we allocate/fill small segments of constant sizes, this is likely a win. Writing a dynamic performance test sounds like a good idea.

> test/micro/org/openjdk/bench/java/lang/foreign/TestFill.java line 86:
> 
>> 84:     @Benchmark
>> 85:     public void buffer_fill() {
>> 86:         // Hopefully, the creation of the intermediate array will be optimized away.
> 
> This maybe won't....why not making the byte array a static final?

The size of the array varies with the length of the array. It seams that escape analysis works here though.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/20712#discussion_r1731249923
PR Review Comment: https://git.openjdk.org/jdk/pull/20712#discussion_r1731252199


More information about the core-libs-dev mailing list