On Thu, 8 Jul 2021 11:50:36 GMT, Wang Huang <whu...@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. I'm quite tempted to approve this. It looks generally better, simpler, and easier to understand than what we have today. However, the improvement isn't great, and I suspect is mostly because of the reduction in traffic between Base and Vector registers. What happens if you rewrite `compare_string_16_bytes_same()` to use `ldp` ? ------------- PR: https://git.openjdk.java.net/jdk/pull/4722
