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