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

Nick Gasson ngasson at openjdk.java.net
Wed Jul 28 08:28:34 UTC 2021


On Thu, 15 Jul 2021 03:30:46 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 style and add unalign test case

I don't think we want to keep two copies of the compareTo intrinsic. If there are no cases where the LDP version is worse than the original version then we should just delete the old one and replace it with this.

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

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


More information about the core-libs-dev mailing list