On Thu, 13 Jun 2024 09:51:21 GMT, Claes Redestad <redes...@openjdk.org> wrote:
>> Shaojin Wen has updated the pull request incrementally with two additional >> commits since the last revision: >> >> - rename benchmark >> - code format & use long address > > Replacing `StringUTF16.putChar` with an `Unsafe.putByte` construct seems safe > since the former already depends on bounds checks having been done elsewhere. > Replacing `byte[]` stores with `Unsafe.putByte` requires a thorough > examination as that would drop implicit bounds checks. > > On my M1 laptop I get similar numbers: > > Name Cnt Base Error Test Error Unit > Change > StringBuilders.appendWithBool8Latin1 15 7,500 ± 0,072 6,112 ± 0,049 ns/op > 1,23x (p = 0,000*) > StringBuilders.appendWithBool8Utf16 15 9,650 ± 0,021 7,304 ± 0,047 ns/op > 1,32x (p = 0,000*) > StringBuilders.appendWithNull8Latin1 15 7,118 ± 0,033 5,158 ± 0,062 ns/op > 1,38x (p = 0,000*) > StringBuilders.appendWithNull8Utf16 15 9,336 ± 0,083 6,870 ± 0,069 ns/op > 1,36x (p = 0,000*) > * = significant > > > On a linux-x64 workstation some of the micros regress, though: > > Name Cnt Base Error Test Error > Unit Change > StringBuilders.appendWithBool8Latin1 15 10.781 ± 0.236 16.279 ± 1.087 > ns/op 0.66x (p = 0.000*) > StringBuilders.appendWithBool8Utf16 15 22.942 ± 0.405 23.733 ± 0.792 > ns/op 0.97x (p = 0.001*) > StringBuilders.appendWithNull8Latin1 15 24.313 ± 10.479 24.397 ± 7.483 > ns/op 1.00x (p = 0.979 ) > StringBuilders.appendWithNull8Utf16 15 38.704 ± 5.972 27.542 ± 5.620 > ns/op 1.41x (p = 0.000*) > * = significant > > > `-XX:+TraceMergeStores` seem to indicate the merging happens as it should in > each case: > > > [TraceMergeStores]: Replace > 2493 StoreB === 1387 5370 2980 2939 [[ 1962 ]] @byte[int:>=0] > (java/lang/Cloneable,java/io/Serializable):exact+any *, idx=13; unsafe > Memory: @byte[int:>=0] > (java/lang/Cloneable,java/io/Serializable):NotNull:exact[2] *, idx=13; !jvms: > StringLatin1::putCharsAt @ bci:50 (line 834) > AbstractStringBuilder::appendNull @ bci:34 (line 642) > AbstractStringBuilder::append @ bci:5 (line 587) StringBuilder::append @ > bci:2 (line 179) StringBuilders::appendWithNull8Latin1 @ bci:38 (line 267) > StringBuilders_appendWithNull8Latin1_jmhTest::appendWithNull8Latin1_avgt_jmhStub > @ bci:17 (line 190) > 1962 StoreB === 1387 2493 2494 2442 [[ 1388 ]] @byte[int:>=0] > (java/lang/Cloneable,java/io/Serializable):exact+any *, idx=13; unsafe > Memory: @byte[int:>=0] > (java/lang/Cloneable,java/io/Serializable):NotNull:exact[2] *, idx=13; !jvms: > StringLatin1::putCharsAt @ bci:62 (line 835) > AbstractStringBuilder::appendNull @ bci:34 (line 642) > AbstractStringBuilder::append @ bci:5 (line 587) StringBuilder::append... The performance regression issue on x64 platform has been fixed. Please continue to review @cl4es @liach ------------- PR Comment: https://git.openjdk.org/jdk/pull/19626#issuecomment-2179620100
