RFR: 8321283: Reuse StringLatin1::equals in regionMatches

[Francesco Nigro]

On Mon, 4 Dec 2023 15:57:45 GMT, Claes Redestad <redestad at openjdk.org> wrote:

>> `StringUTF16::equals` was used before https://bugs.openjdk.org/browse/JDK-8215017 - I don't remember exactly what performance verification I did back then. x86 intrinsics seem to emit the exact same asm, aarch64 does a little bit of extra optimization for the UTF16 case (avoids code and a branch for checking tailing byte, but only if the UTF16 string is 1-3 elements long). Perhaps we should double-check that JDK-8215017 didn't significantly regress some UTF16 equality checks before we rip out code.
>
> Going out on this tangent... I've compared the mainline (Base) with a build where `String::equals` is restored to pre-JDK-8215017 using a modified version of the StringEquals microbenchmark that tests UTF16 Strings of size 3. One test (EQ) where the strings are equals, another (NE) where they are not. The EQ one is the main contender for a case that would benefit from avoiding the trailing byte check on aarch64:
> 
> On my M1 (aarch64):
> 
> 
> Name                          Cnt  Base   Error   Test   Error  Unit  Change
> StringEquals.equalsUTF16_3_EQ   5 1,750 ± 0,009  1,864 ± 0,401 ns/op   0,94x (p = 0,070 )
> StringEquals.equalsUTF16_3_NE   5 1,674 ± 0,026  1,839 ± 0,179 ns/op   0,91x (p = 0,001*)
>   * = significant
>  ```
> 
> Similarly restoring the pre-JDK-8215017 version seem to be a net loss on x86:
> 
> Name                          Cnt  Base   Error   Test   Error  Unit  Change
> StringEquals.equalsUTF16_3_EQ   5 2.885 ± 0.054  2.886 ± 0.036 ns/op   1.00x (p = 0.837 )
> StringEquals.equalsUTF16_3_NE   5 2.581 ± 0.002  2.756 ± 0.003 ns/op   0.94x (p = 0.000*)
>   * = significant
> 
> 
> So it seems JDK-8215017 was either neutral or a small performance win (phew!). The avoided branch and overall reduction in code complexity outweighs the win (if any) from not having the redundant `COMPARE_BYTE` chunk of code emitted in the StringUTF16 case. There are other platforms I can't currently check, but I think it's likely that we'd see similar numbers there. So it seems to be the case that `StringUTF16::equals` can be safely removed.

Nice one, I can happily send a PR for it, but I feel bad about it; you have performed the check yourself (and were aware of what dropping it would mean...)

separated question @cl4es, how do you obtained the 'Change` column with the `p` value? is in some jdk script I've missed?

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

PR Review Comment: https://git.openjdk.org/jdk/pull/16933#discussion_r1414125814
PR Review Comment: https://git.openjdk.org/jdk/pull/16933#discussion_r1414129103