RFR: 8333893: Optimization for StringBuilder append boolean & null [v15]

[Chen Liang]

On Tue, 23 Jul 2024 13:42:20 GMT, Shaojin Wen <duke at openjdk.org> wrote:

>> src/java.base/share/classes/java/lang/StringLatin1.java line 832:
>> 
>>> 830:     static void putCharsAt(byte[] val, int index, int c1, int c2, int c3, int c4) {
>>> 831:         assert index >= 0 && index + 3 < length(val) : "Trusted caller missed bounds check";
>>> 832:         // Don't use the putChar method, Its instrinsic will cause C2 unable to combining values into larger stores.
>> 
>> `StringLatin1` has no `putChar` method. I think you can just convert this to explicit array stores for the merge store optimization.
>
> explicit array stores  has the overhead of boundary checking. If putCharsAt of StringLatin1 is not implemented based on Unsafe, the performance will be worse than StringUTF16.
> 
> Of course, this is a common problem. StringUTF16.putChar is equivalent to Unsafe.putChar, without boundary checking. I found in many test scenarios that the UTF16 version performs better than the StringLatin1 version.
> 
> We may need to change some StringLatin1 related implementations to use Unsafe, otherwise users will turn off COMPACT_STRINGS to improve performance.

Thank you for telling what the blocking issue is here. Does C2 not merge the bound checks when it does the merge stores? Interesting, and I think a fix from their side should be the way to go.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/19626#discussion_r1688102742