On Thu, 13 Jun 2024 02:06:42 GMT, Shaojin Wen <d...@openjdk.org> wrote:
>> After PR https://github.com/openjdk/jdk/pull/16245, C2 optimizes stores into >> primitive arrays by combining values into larger stores. >> >> This PR rewrites the code of appendNull and append(boolean) methods so that >> these two methods can be optimized by C2. > > 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 @ bci:2 (line 179) StringBuilders::appendWithNull8Latin1 @ bci:38 (line 267) StringBuilders_appendWithNull8Latin1_jmhTest::appendWithNull8Latin1_avgt_jmhStub @ bci:17 (line 190) 1388 StoreB === 1387 1962 1963 1341 [[ 833 ]] @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:77 (line 836) 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) 833 StoreB === 1387 1388 1389 1341 [[ 5311 425 1400 ]] @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:92 (line 837) 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) [TraceMergeStores]: with 6207 ConI === 0 [[ 6208 6210 ]] #int:1819047278 6210 StoreI === 1387 5370 2980 6207 [[ ]] @byte[int:>=0] (java/lang/Cloneable,java/io/Serializable):NotNull:exact[2] *, idx=13; mismatched Memory: @byte[int:>=0] (java/lang/Cloneable,java/io/Serializable):NotNull:exact[2] *, idx=13; ------------- PR Comment: https://git.openjdk.org/jdk/pull/19626#issuecomment-2165167049
