JDK-8173585: Intrinsify StringLatin1.indexOf(char)

[Roger Riggs]

Hi Jason,

Thanks for checking, the difference of the utf16 numbers seemed to be 
just outside of the error range.

Regards, Roger


On 9/9/20 3:17 PM, Tatton, Jason wrote:
> Hi Roger,
>
> Thanks for your question. The code path for UTF16 has hasn't been interacted with in a meaningful way in this patch so I think this is just noise. To validate this hypothesis I re-ran the benchmark with 10 forks (default is 5), the results indicate that the performance of the UTF16 implementation of indexOf(char) has not been materially changed:
>
> Without the new StringLatin1 indexOf(char) intrinsic:
> Benchmark                           Mode  Cnt      Score     Error  Units
> IndexOfBenchmark.latin1_mixed_char  avgt   25  27550.832 ± 347.570  ns/op
> IndexOfBenchmark.utf16_mixed_char   avgt   25  18472.190 ± 219.185  ns/op
>
>
> With the new StringLatin1 indexOf(char) intrinsic:
> Benchmark                           Mode  Cnt      Score     Error  Units
> IndexOfBenchmark.latin1_mixed_char  avgt    5  17806.338 ± 217.399  ns/op
> IndexOfBenchmark.utf16_mixed_char   avgt    5  18276.366 ± 470.528  ns/op
>
> In fact, on this run the performance of utf16 was better than without this patch, this is however a 1% improvement which fits within the error range seen on these tests of between 1-2%. So I think it's fair to say that this patch has no effect on the performance of the existing StringUTF16 indexOf(char) method.
>
> -----Original Message-----
> From: core-libs-dev <core-libs-dev-retn at openjdk.java.net> On Behalf Of Roger Riggs
> Sent: 08 September 2020 15:54
> To: core-libs-dev at openjdk.java.net
> Subject: RE: [EXTERNAL] JDK-8173585: Intrinsify StringLatin1.indexOf(char)
>
> CAUTION: This email originated from outside of the organization. Do not click links or open attachments unless you can confirm the sender and know the content is safe.
>
>
>
> Hi Jason,
>
> With respect to the increased ns/op in the utf16_mixed_char benchmark, how should we understand the lower performance?
>
> Thanks, Roger
>
>
> On 9/8/20 8:02 AM, Tatton, Jason wrote:
>> Hi Andrew, thank you for taking the time to review this.
>>
>> Since we have now moved to git, I have raised a new PR for this RFR:
>>
>> https://urldefense.com/v3/__https://github.com/openjdk/jdk/pull/71__;!!GqivPVa7Brio!NewvwHYXhjOSSwNJArOOHrkBu818bCSzfDOe8hgvEM5idglmYuTub0vn0Ydzi02o$
>> https://bugs.openjdk.java.net/browse/JDK-8173585
>>
>> I have improved the micro benchmark in the ways which you and others have requested, namely:
>> + The benchmark is now included in test/micro/org/openjdk/bench/java/lang as StringIndexOfChar (as advised by my colleagues here at AWS; Xin Liu and Volker Simonis).
>> + Times are now in nanoseconds.
>> + Terminating characters ('a') are in 66.666% of tested strings.
>> + I have added four new benchmarks which operate on a random length strings (32 characters being the average) of type either StringLatin1 of StringUTF16 and call indexOf(char) or indexOf(String).
>>
>> I have included below the output of these four tests below:
>>
>> Without the new StringLatin1 indexOf(char) intrinsic:
>>
>> Benchmark                           Mode  Cnt      Score     Error  Units
>> IndexOfBenchmark.latin1_mixed_char  avgt    5  26389.129 ± 182.581  ns/op
>> IndexOfBenchmark.utf16_mixed_char   avgt    5  17885.383 ± 435.933  ns/op
>>
>>
>> With the new StringLatin1 indexOf(char) intrinsic:
>>
>> Benchmark                           Mode  Cnt      Score     Error  Units
>> IndexOfBenchmark.latin1_mixed_char  avgt    5  17875.185 ± 407.716  ns/op
>> IndexOfBenchmark.utf16_mixed_char   avgt    5  18292.802 ± 167.306  ns/op
>>
>> The objective of the patch is to bring the performance of StringLatin1 indexOf(char) in line with StringUTF16 indexOf(char) for x86 and ARM64. We can see above that this has been achieved. Similar results were obtained when running on ARM.
>>
>> Regards,
>> Jason