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-r...@openjdk.java.net> On Behalf Of Roger Riggs Sent: 08 September 2020 15:54 To: core-libs-dev@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://github.com/openjdk/jdk/pull/71 > 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