RFR: 8268231: Aarch64: Use ldp in intrinsics for String.compareTo [v6]

[Nick Gasson]

On Fri, 6 Aug 2021 09:50:54 GMT, Wang Huang <whuang at openjdk.org> wrote:

>> Dear all, 
>>     Can you do me a favor to review this patch. This patch use `ldp` to implement String.compareTo.
>>    
>> * We add a JMH test case 
>> * Here is the result of this test case
>>  
>> Benchmark	                       |(size)| Mode| Cnt|Score | Error	 |Units	
>> ---------------------------------|------|-----|----|------|--------|-----
>> StringCompare.compareLL       |  64  | avgt| 5  |7.992 | ±	0.005|us/op
>> StringCompare.compareLL       |  72  | avgt| 5  |15.029| ±	0.006|us/op
>> StringCompare.compareLL       |  80  | avgt| 5  |14.655| ±	0.011|us/op
>> StringCompare.compareLL       |  91  | avgt| 5  |16.363| ±	0.12 |us/op
>> StringCompare.compareLL       |  101 | avgt| 5  |16.966| ±	0.007|us/op
>> StringCompare.compareLL       |  121 | avgt| 5  |19.276| ±	0.006|us/op
>> StringCompare.compareLL       |  181 | avgt| 5  |19.002| ±	0.417|us/op
>> StringCompare.compareLL       |  256 | avgt| 5  |24.707| ±	0.041|us/op
>> StringCompare.compareLLWithLdp|  64  | avgt| 5  |8.001	| ±	0.121|us/op
>> StringCompare.compareLLWithLdp|  72  | avgt| 5  |11.573| ±	0.003|us/op
>> StringCompare.compareLLWithLdp|  80  | avgt| 5  |6.861 | ±	0.004|us/op
>> StringCompare.compareLLWithLdp|  91  | avgt| 5  |12.774| ±	0.201|us/op
>> StringCompare.compareLLWithLdp|  101 | avgt| 5  |8.691 | ±	0.004|us/op
>> StringCompare.compareLLWithLdp|  121 | avgt| 5  |11.091| ±	1.342|us/op
>> StringCompare.compareLLWithLdp|  181 | avgt| 5  |14.64 | ±	0.581|us/op
>> StringCompare.compareLLWithLdp|  256 | avgt| 5  |25.879| ±	1.775|us/op
>> StringCompare.compareUU       |  64  | avgt| 5  |13.476| ±	0.01 |us/op
>> StringCompare.compareUU       |  72  | avgt| 5  |15.078| ±	0.006|us/op
>> StringCompare.compareUU       |  80  | avgt| 5  |23.512| ±	0.011|us/op
>> StringCompare.compareUU       |  91  | avgt| 5  |24.284| ±	0.008|us/op
>> StringCompare.compareUU       |  101 | avgt| 5  |20.707| ±	0.017|us/op
>> StringCompare.compareUU       |  121 | avgt| 5  |29.302| ±	0.011|us/op
>> StringCompare.compareUU       |  181 | avgt| 5  |39.31	| ±	0.016|us/op
>> StringCompare.compareUU       |  256 | avgt| 5  |54.592| ±	0.392|us/op
>> StringCompare.compareUUWithLdp|  64  | avgt| 5  |16.389| ±	0.008|us/op
>> StringCompare.compareUUWithLdp|  72  | avgt| 5  |10.71 | ±	0.158|us/op
>> StringCompare.compareUUWithLdp|  80  | avgt| 5  |11.488| ±	0.024|us/op
>> StringCompare.compareUUWithLdp|  91  | avgt| 5  |13.412| ±	0.006|us/op
>> StringCompare.compareUUWithLdp|  101 | avgt| 5  |16.245| ±	0.434|us/op
>> StringCompare.compareUUWithLdp|  121 | avgt| 5  |16.597| ±	0.016|us/op
>> StringCompare.compareUUWithLdp|  181 | avgt| 5  |27.373| ±	0.017|us/op
>> StringCompare.compareUUWithLdp|  256 | avgt| 5  |41.74 | ±	3.5	 |us/op
>> 
>> From this table, we can see that in most cases, our patch is better than old one.
>> 
>> Thank you for your review. Any suggestions are welcome.
>
> Wang Huang has updated the pull request incrementally with one additional commit since the last revision:
> 
>   fix codestyle

I've run the benchmark on several different machines and didn't see any performance regressions, and the speed-up for longer strings looks quite good. I also ran jtreg tier1-3 with no new failures so I think this is ok.

If you fix the Windows build I'll approve it. But please wait for another review, preferably from @theRealAph.

Note that JDK-8269559 (#5129) is also adding a JMH benchmark for this intrinsic: it would be good if we could merge them, either now or later.

src/hotspot/cpu/aarch64/stubGenerator_aarch64.cpp line 4871:

> 4869:     // exit from large loop when less than 64 bytes left to read or we're about
> 4870:     // to prefetch memory behind array border
> 4871:     int largeLoopExitCondition = MAX(64, SoftwarePrefetchHintDistance)/(isLL ? 1 : 2);

This breaks the Windows AArch64 build:


Creating support/modules_libs/java.base/server/jvm.dll from 1051 file(s)
d:\a\jdk\jdk\jdk\src\hotspot\cpu\aarch64\stubGenerator_aarch64.cpp(4871): error C3861: 'MAX': identifier not found
make[3]: *** [lib/CompileJvm.gmk:143: /cygdrive/d/a/jdk/jdk/jdk/build/windows-aarch64/hotspot/variant-server/libjvm


https://github.com/Wanghuang-Huawei/jdk/runs/3260986937

Should probably be left as `MAX2`.

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

Changes requested by ngasson (Reviewer).

PR: https://git.openjdk.java.net/jdk/pull/4722