Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

8268231: Aarch64: Use ldp in intrinsics for String.compareTo #4722

Closed
wants to merge 8 commits into from

Conversation

Wanghuang-Huawei
Copy link

@Wanghuang-Huawei Wanghuang-Huawei commented Jul 8, 2021

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.


Progress

  • Change must not contain extraneous whitespace
  • Commit message must refer to an issue
  • Change must be properly reviewed

Issue

  • JDK-8268231: Aarch64: Use Ldp in intrinsics for String.compareTo

Reviewers

Contributors

  • Wang Huang <whuang@openjdk.org>
  • Sun Jianye <sunjianye@huawei.com>
  • Wu Yan <wuyan@openjdk.org>

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.java.net/jdk pull/4722/head:pull/4722
$ git checkout pull/4722

Update a local copy of the PR:
$ git checkout pull/4722
$ git pull https://git.openjdk.java.net/jdk pull/4722/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 4722

View PR using the GUI difftool:
$ git pr show -t 4722

Using diff file

Download this PR as a diff file:
https://git.openjdk.java.net/jdk/pull/4722.diff

@bridgekeeper
Copy link

@bridgekeeper bridgekeeper bot commented Jul 8, 2021

👋 Welcome back whuang! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

@openjdk
Copy link

@openjdk openjdk bot commented Jul 8, 2021

@Wanghuang-Huawei The following labels will be automatically applied to this pull request:

  • core-libs
  • hotspot-compiler

When this pull request is ready to be reviewed, an "RFR" email will be sent to the corresponding mailing lists. If you would like to change these labels, use the /label pull request command.

@openjdk openjdk bot added hotspot-compiler core-libs labels Jul 8, 2021
@Wanghuang-Huawei
Copy link
Author

@Wanghuang-Huawei Wanghuang-Huawei commented Jul 8, 2021

/contributor add Wang Huang whuang@openjdk.org
/contributor add Sun Jianye sunjianye@huawei.com

@openjdk
Copy link

@openjdk openjdk bot commented Jul 8, 2021

@Wanghuang-Huawei
Contributor Wang Huang <whuang@openjdk.org> successfully added.

@openjdk
Copy link

@openjdk openjdk bot commented Jul 8, 2021

@Wanghuang-Huawei
Contributor Sun Jianye <sunjianye@huawei.com> successfully added.

@Wanghuang-Huawei Wanghuang-Huawei changed the title Aarch64: Use ldp in intrinsics for String.compareTo 8268231: Aarch64: Use ldp in intrinsics for String.compareTo Jul 8, 2021
@openjdk openjdk bot added the rfr label Jul 8, 2021
@mlbridge
Copy link

@mlbridge mlbridge bot commented Jul 8, 2021

@theRealAph
Copy link
Contributor

@theRealAph theRealAph commented Jul 9, 2021

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 ?

@Wanghuang-Huawei
Copy link
Author

@Wanghuang-Huawei Wanghuang-Huawei commented Jul 12, 2021

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 ?

I refacted compare_string_16_bytes_same() as a draft, the performance comparision is listed here,

Benchmark (diff_pos) (size) Mode Cnt Score Error Units
StringCompare.compareLLDiffStrings 7 128 avgt 5 4.252 ± 0.001 us/op
StringCompare.compareLLDiffStrings 15 128 avgt 5 4.714 ± 0.001 us/op
StringCompare.compareLLDiffStrings 31 128 avgt 5 6.139 ± 0.445 us/op
StringCompare.compareLLDiffStrings 47 128 avgt 5 13.861 ± 0.001 us/op
StringCompare.compareLLDiffStrings 63 128 avgt 5 8.823 ± 0.007 us/op
StringCompare.compareLLDiffStringsWithLdp 7 128 avgt 5 3.867 ± 0.001 us/op
StringCompare.compareLLDiffStringsWithLdp 15 128 avgt 5 5.571 ± 0.756 us/op
StringCompare.compareLLDiffStringsWithLdp 31 128 avgt 5 5.408 ± 0.001 us/op
StringCompare.compareLLDiffStringsWithLdp 47 128 avgt 5 6.896 ± 0.825 us/op
StringCompare.compareLLDiffStringsWithLdp 63 128 avgt 5 6.787 ± 0.001 us/op
StringCompare.compareLLDiffStringsWithRefactor 7 128 avgt 5 3.481 ± 0.001 us/op
StringCompare.compareLLDiffStringsWithRefactor 15 128 avgt 5 10.023 ± 0.012 us/op
StringCompare.compareLLDiffStringsWithRefactor 31 128 avgt 5 5.627 ± 0.017 us/op
StringCompare.compareLLDiffStringsWithRefactor 47 128 avgt 5 13.369 ± 0.544 us/op
StringCompare.compareLLDiffStringsWithRefactor 63 128 avgt 5 8.382 ± 0.988 us/op

@theRealAph
Copy link
Contributor

@theRealAph theRealAph commented Jul 12, 2021

Two machines to compare here, Apple M1 and ThunderX2:

Benchmark                                       (diff_pos)  (size)  Mode  Cnt  Score   Error  Units
StringCompare.compareLLDiffStrings                       7     128  avgt    3  2.194 ± 0.001  us/op
StringCompare.compareLLDiffStrings                      15     128  avgt    3  2.195 ± 0.018  us/op
StringCompare.compareLLDiffStrings                      31     128  avgt    3  2.508 ± 0.003  us/op
StringCompare.compareLLDiffStrings                      47     128  avgt    3  2.821 ± 0.001  us/op
StringCompare.compareLLDiffStrings                      63     128  avgt    3  3.446 ± 0.003  us/op
StringCompare.compareLLDiffStringsWithLdp                7     128  avgt    3  2.194 ± 0.001  us/op
StringCompare.compareLLDiffStringsWithLdp               15     128  avgt    3  2.195 ± 0.001  us/op
StringCompare.compareLLDiffStringsWithLdp               31     128  avgt    3  2.508 ± 0.001  us/op
StringCompare.compareLLDiffStringsWithLdp               47     128  avgt    3  2.510 ± 0.006  us/op
StringCompare.compareLLDiffStringsWithLdp               63     128  avgt    3  2.824 ± 0.003  us/op
StringCompare.compareLLDiffStringsWithRefactor           7     128  avgt    3  1.882 ± 0.018  us/op
StringCompare.compareLLDiffStringsWithRefactor          15     128  avgt    3  2.019 ± 0.002  us/op
StringCompare.compareLLDiffStringsWithRefactor          31     128  avgt    3  2.355 ± 0.003  us/op
StringCompare.compareLLDiffStringsWithRefactor          47     128  avgt    3  2.821 ± 0.010  us/op
StringCompare.compareLLDiffStringsWithRefactor          63     128  avgt    3  3.135 ± 0.002  us/op
Benchmark                                       (diff_pos)  (size)  Mode  Cnt   Score   Error  Units
StringCompare.compareLLDiffStrings                       7     128  avgt    3   6.014 ± 0.016  us/op
StringCompare.compareLLDiffStrings                      15     128  avgt    3   6.431 ± 0.030  us/op
StringCompare.compareLLDiffStrings                      31     128  avgt    3   7.214 ± 0.012  us/op
StringCompare.compareLLDiffStrings                      47     128  avgt    3   8.816 ± 0.011  us/op
StringCompare.compareLLDiffStrings                      63     128  avgt    3  10.283 ± 0.033  us/op
StringCompare.compareLLDiffStringsWithLdp                7     128  avgt    3   5.214 ± 0.002  us/op
StringCompare.compareLLDiffStringsWithLdp               15     128  avgt    3   6.899 ± 0.359  us/op
StringCompare.compareLLDiffStringsWithLdp               31     128  avgt    3   7.309 ± 0.038  us/op
StringCompare.compareLLDiffStringsWithLdp               47     128  avgt    3   7.723 ± 0.080  us/op
StringCompare.compareLLDiffStringsWithLdp               63     128  avgt    3   9.822 ± 0.103  us/op
StringCompare.compareLLDiffStringsWithRefactor           7     128  avgt    3   4.814 ± 0.002  us/op
StringCompare.compareLLDiffStringsWithRefactor          15     128  avgt    3   8.395 ± 0.259  us/op
StringCompare.compareLLDiffStringsWithRefactor          31     128  avgt    3   7.615 ± 0.004  us/op
StringCompare.compareLLDiffStringsWithRefactor          47     128  avgt    3   8.817 ± 0.013  us/op
StringCompare.compareLLDiffStringsWithRefactor          63     128  avgt    3  10.413 ± 0.002  us/op

@theRealAph
Copy link
Contributor

@theRealAph theRealAph commented Jul 12, 2021

And with longer strings, M1 and ThunderX2:

Benchmark                                       (diff_pos)  (size)  Mode  Cnt   Score   Error  Units
StringCompare.compareLLDiffStrings                    1023    1024  avgt    3  50.849 ± 0.087  us/op
StringCompare.compareLLDiffStringsWithLdp             1023    1024  avgt    3  23.676 ± 0.015  us/op
StringCompare.compareLLDiffStringsWithRefactor        1023    1024  avgt    3  28.967 ± 0.168  us/op
StringCompare.compareLLDiffStrings                    1023    1024  avgt    3  98.681 ± 0.026  us/op
StringCompare.compareLLDiffStringsWithLdp             1023    1024  avgt    3  82.576 ± 0.656  us/op
StringCompare.compareLLDiffStringsWithRefactor        1023    1024  avgt    3  98.801 ± 0.321  us/op

LDP wins on M1 here, but on ThunderX2 it makes almost no difference at all. And how often are we comparing such long strings?
I don't know what to think, really. It seems that we're near to a place where we're optimizing for micro-architecture, and I don't want to see that here. On the other hand, using LDP is not worse anywhere, so we should allow it.

@Wanghuang-Huawei
Copy link
Author

@Wanghuang-Huawei Wanghuang-Huawei commented Jul 13, 2021

And with longer strings, M1 and ThunderX2:

Benchmark                                       (diff_pos)  (size)  Mode  Cnt   Score   Error  Units
StringCompare.compareLLDiffStrings                    1023    1024  avgt    3  50.849 ± 0.087  us/op
StringCompare.compareLLDiffStringsWithLdp             1023    1024  avgt    3  23.676 ± 0.015  us/op
StringCompare.compareLLDiffStringsWithRefactor        1023    1024  avgt    3  28.967 ± 0.168  us/op
StringCompare.compareLLDiffStrings                    1023    1024  avgt    3  98.681 ± 0.026  us/op
StringCompare.compareLLDiffStringsWithLdp             1023    1024  avgt    3  82.576 ± 0.656  us/op
StringCompare.compareLLDiffStringsWithRefactor        1023    1024  avgt    3  98.801 ± 0.321  us/op

LDP wins on M1 here, but on ThunderX2 it makes almost no difference at all. And how often are we comparing such long strings?
I don't know what to think, really. It seems that we're near to a place where we're optimizing for micro-architecture, and I don't want to see that here. On the other hand, using LDP is not worse anywhere, so we should allow it.

Thank you for your suggestion. I inspect the result and find that the result of my first commit (c5e29b9) is better. Because of that , I will choose the version without refacting compare_string_16_bytes_same as the final version.

Copy link
Member

@nick-arm nick-arm left a comment

Have you tested when the data in the byte[] array is not 16B aligned? With the default JVM options the array data naturally starts 16B into the object, but you can force a different alignment with e.g. -XX:-UseCompressedClassPointers. I tried that on N1 and it's very slightly slower than with the 16B alignment, but still faster than the non-LDP version for length 1024 strings. On A72 the difference is a bit bigger but again faster than non-LDP.

N1, -UseCompressedClassPointers

Benchmark                                  (diff_pos)  (size)  Mode  Cnt    Score   Error  Units
StringCompare.compareLLDiffStrings               1023    1024  avgt    5   67.789 ? 0.095  us/op
StringCompare.compareLLDiffStringsWithLdp        1023    1024  avgt    5   45.912 ? 0.059  us/op
StringCompare.compareUUDiffStrings               1023    1024  avgt    5  133.365 ? 0.086  us/op
StringCompare.compareUUDiffStringsWithLdp        1023    1024  avgt    5   89.009 ? 0.312  us/op

N1, +UseCompressedClassPointers

Benchmark                                  (diff_pos)  (size)  Mode  Cnt    Score   Error  Units
StringCompare.compareLLDiffStrings               1023    1024  avgt    5   67.878 ? 0.156  us/op
StringCompare.compareLLDiffStringsWithLdp        1023    1024  avgt    5   46.487 ? 0.115  us/op
StringCompare.compareUUDiffStrings               1023    1024  avgt    5  133.576 ? 0.111  us/op
StringCompare.compareUUDiffStringsWithLdp        1023    1024  avgt    5   90.462 ? 0.176  us/op

A72, -UseCompressedClassPointers

Benchmark                                  (diff_pos)  (size)  Mode  Cnt    Score   Error  Units
StringCompare.compareLLDiffStrings               1023    1024  avgt    5  122.697 ? 0.235  us/op
StringCompare.compareLLDiffStringsWithLdp        1023    1024  avgt    5   73.883 ? 0.136  us/op
StringCompare.compareUUDiffStrings               1023    1024  avgt    5  243.022 ? 0.457  us/op
StringCompare.compareUUDiffStringsWithLdp        1023    1024  avgt    5  150.659 ? 0.313  us/op

A72, +UseCompressedClassPointers

Benchmark                                  (diff_pos)  (size)  Mode  Cnt    Score   Error  Units
StringCompare.compareLLDiffStrings               1023    1024  avgt    5  122.754 ? 0.563  us/op
StringCompare.compareLLDiffStringsWithLdp        1023    1024  avgt    5   68.513 ? 0.282  us/op
StringCompare.compareUUDiffStrings               1023    1024  avgt    5  243.043 ? 0.882  us/op
StringCompare.compareUUDiffStringsWithLdp        1023    1024  avgt    5  139.152 ? 0.333  us/op

__ sub(cnt2, cnt2, isLL ? 8 : 4);

__ bind(LESS8); // directly load last 8 bytes
if(!isLL) {
Copy link
Member

@nick-arm nick-arm Jul 14, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The indentation of the if seems wrong here: shouldn't it line up with the assembly block like the if on line 4989? Also needs a space before the parenthesis.

__ lsrv(tmp1, tmp1, rscratch2);
__ lsrv(tmp2, tmp2, rscratch2);
if (isLL) {
__ uxtbw(tmp1, tmp1);
Copy link
Member

@nick-arm nick-arm Jul 14, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Convention is to indent with two spaces but have four here.

Copy link
Author

@Wanghuang-Huawei Wanghuang-Huawei Jul 15, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for your suggestion. I have fixed the style and add unalign test case in my new commit.

@nick-arm
Copy link
Member

@nick-arm nick-arm commented Jul 14, 2021

I tried that on N1 and it's very slightly slower than with the 16B alignment

Sorry, ignore that, the result is actually the other way round. Not sure what's going on there, but there's no significant difference.

@Wanghuang-Huawei
Copy link
Author

@Wanghuang-Huawei Wanghuang-Huawei commented Jul 20, 2021

/contributor add Wu Yan wuyan@openjdk.org

@openjdk
Copy link

@openjdk openjdk bot commented Jul 20, 2021

@Wanghuang-Huawei
Contributor Wu Yan <wuyan@openjdk.org> successfully added.

@wuyan0
Copy link

@wuyan0 wuyan0 commented Jul 28, 2021

Could you do me a favor to review the patch? @theRealAph @nick-arm Thanks.

@nick-arm
Copy link
Member

@nick-arm nick-arm commented Jul 28, 2021

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.

@theRealAph
Copy link
Contributor

@theRealAph theRealAph commented Jul 28, 2021

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.

I agree. The trouble is, what does "worse" mean? I'm looking at SDEN-1982442, Neoverse N2 errata, 2001293, and I see that LDP has to be slowed down on streaming workloads, which will affect this. (That's just an example: I'm making the point that implementations differ.)

The trouble with this patch is that it (probably) makes things better for long strings, which are very rare. What we actually need to care about is performance for a large number of typical-sized strings, which are names, identifiers, passwords, and so on: about 10-30 characters.

@wuyan0
Copy link

@wuyan0 wuyan0 commented Jul 28, 2021

The trouble is, what does "worse" mean? I'm looking at SDEN-1982442, Neoverse N2 errata, 2001293, and I see that LDP has to be slowed down on streaming workloads, which will affect this. (That's just an example: I'm making the point that implementations differ.)

We are testing on HiSilicon TSV110, maybe we can enable this optimization by default on the verified platforms.

@nick-arm
Copy link
Member

@nick-arm nick-arm commented Jul 28, 2021

We are testing on HiSilicon TSV110, maybe we can enable this optimization by default on the verified platforms.

We don't really want to have different implementations for each microarchitecture, that would be a testing nightmare.

The existing stub uses prefetch instructions if SoftwarePrefetchHintDistance >= 0 but the new LDP version doesn't. Did you find there's no benefit to that? Adding prefetches was one of the reasons to introduce the separate stub for long strings, see the mail below:

https://mail.openjdk.java.net/pipermail/hotspot-compiler-dev/2018-April/028888.html

It seems the existing code was tuned for Thunder X/X2 so perhaps that's why Andrew sees little improvement there with the new version.

What testing have you done besides benchmarking? The patch linked above had at least two subtle bugs in corner cases.

@nick-arm
Copy link
Member

@nick-arm nick-arm commented Jul 30, 2021

I was (still am) tempted to approve it, but Nick says there are still bugs in corner cases.

I meant the earlier String.compareTo that this is partially replacing. This one might be fine but I just wanted to check it had be thoroughly tested. For reference they were:

https://bugs.openjdk.java.net/browse/JDK-8215100
https://bugs.openjdk.java.net/browse/JDK-8237524
https://bugs.openjdk.java.net/browse/JDK-8218966

@mlbridge
Copy link

@mlbridge mlbridge bot commented Jul 30, 2021

Mailing list message from Andrew Haley on hotspot-compiler-dev:

On 7/30/21 12:34 PM, Nick Gasson wrote:

On Fri, 30 Jul 2021 10:36:01 GMT, Andrew Haley <aph at redhat.com> wrote:

I was (still am) tempted to approve it, but Nick says there are still bugs in corner cases.

I meant the earlier String.compareTo that this is partially replacing. This one might be fine but I just wanted to check it had be thoroughly tested.

My apologies.

Ah yes, that was horrendous. Let's not go there again.

--
Andrew Haley (he/him)
Java Platform Lead Engineer
Red Hat UK Ltd. <https://www.redhat.com>
https://keybase.io/andrewhaley
EAC8 43EB D3EF DB98 CC77 2FAD A5CD 6035 332F A671

@Wanghuang-Huawei
Copy link
Author

@Wanghuang-Huawei Wanghuang-Huawei commented Aug 3, 2021

Thank you for your suggestion. I have pushed new commit.

Copy link
Member

@nick-arm nick-arm left a comment

Please provide the updated benchmark results for this version. Are you able to test it on several different machines?

__ ldp(tmp2, tmp2h, Address(str2, i * 16));
__ cmp(tmp1, tmp2);
__ ccmp(tmp1h, tmp2h, 0, Assembler::EQ);
__ br(__ NE, DIFF);
Copy link
Member

@nick-arm nick-arm Aug 4, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The line above uses Assembler::EQ for the condition code but this line uses __ NE. Better to be consistent and use Assembler:: everywhere.

Copy link

@wuyan0 wuyan0 Aug 5, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, I'll fix it.

@wuyan0
Copy link

@wuyan0 wuyan0 commented Aug 4, 2021

Please provide the updated benchmark results for this version. Are you able to test it on several different machines?

We tested this version on Raspberry Pi 4B.

base:
Benchmark                                     (diff_pos)  (size)  Mode  Cnt    Score    Error  Units
StringCompare.compareLLDiffStrings                     7     256  avgt    5   14.882 ?  0.157  us/op
StringCompare.compareLLDiffStrings                    15     256  avgt    5   15.514 ?  0.094  us/op
StringCompare.compareLLDiffStrings                    31     256  avgt    5   16.756 ?  0.050  us/op
StringCompare.compareLLDiffStrings                    47     256  avgt    5   18.196 ?  0.727  us/op
StringCompare.compareLLDiffStrings                    63     256  avgt    5   20.110 ?  0.075  us/op
StringCompare.compareLLDiffStrings                   127     256  avgt    5   31.458 ?  0.032  us/op
StringCompare.compareLLDiffStrings                   255     256  avgt    5   53.099 ?  1.212  us/op
StringCompare.compareUUDiffStrings                     7     256  avgt    5   15.419 ?  0.012  us/op
StringCompare.compareUUDiffStrings                    15     256  avgt    5   16.761 ?  0.078  us/op
StringCompare.compareUUDiffStrings                    31     256  avgt    5   20.132 ?  0.112  us/op
StringCompare.compareUUDiffStrings                    47     256  avgt    5   27.492 ?  0.104  us/op
StringCompare.compareUUDiffStrings                    63     256  avgt    5   32.147 ?  0.028  us/op
StringCompare.compareUUDiffStrings                   127     256  avgt    5   56.208 ?  0.016  us/op
StringCompare.compareUUDiffStrings                   255     256  avgt    5  100.439 ?  0.782  us/op
StringCompare.compareUUDiffStringsTurnOffCCP           7     256  avgt    5   15.441 ?  0.071  us/op
StringCompare.compareUUDiffStringsTurnOffCCP          15     256  avgt    5   16.781 ?  0.192  us/op
StringCompare.compareUUDiffStringsTurnOffCCP          31     256  avgt    5   20.109 ?  0.010  us/op
StringCompare.compareUUDiffStringsTurnOffCCP          47     256  avgt    5   27.463 ?  0.068  us/op
StringCompare.compareUUDiffStringsTurnOffCCP          63     256  avgt    5   32.168 ?  0.064  us/op
StringCompare.compareUUDiffStringsTurnOffCCP         127     256  avgt    5   56.283 ?  0.551  us/op
StringCompare.compareUUDiffStringsTurnOffCCP         255     256  avgt    5  100.419 ?  0.914  us/op

opt:
Benchmark                                     (diff_pos)  (size)  Mode  Cnt    Score   Error  Units
StringCompare.compareLLDiffStrings                     7     256  avgt    5   14.064 ? 0.048  us/op
StringCompare.compareLLDiffStrings                    15     256  avgt    5   16.079 ? 0.041  us/op
StringCompare.compareLLDiffStrings                    31     256  avgt    5   17.413 ? 0.033  us/op
StringCompare.compareLLDiffStrings                    47     256  avgt    5   18.750 ? 0.012  us/op
StringCompare.compareLLDiffStrings                    63     256  avgt    5   20.093 ? 0.052  us/op
StringCompare.compareLLDiffStrings                   127     256  avgt    5   27.432 ? 0.009  us/op
StringCompare.compareLLDiffStrings                   255     256  avgt    5   44.832 ? 0.173  us/op
StringCompare.compareUUDiffStrings                     7     256  avgt    5   16.071 ? 0.028  us/op
StringCompare.compareUUDiffStrings                    15     256  avgt    5   18.082 ? 0.015  us/op
StringCompare.compareUUDiffStrings                    31     256  avgt    5   20.753 ? 0.006  us/op
StringCompare.compareUUDiffStrings                    47     256  avgt    5   25.427 ? 0.051  us/op
StringCompare.compareUUDiffStrings                    63     256  avgt    5   28.170 ? 0.091  us/op
StringCompare.compareUUDiffStrings                   127     256  avgt    5   42.809 ? 0.143  us/op
StringCompare.compareUUDiffStrings                   255     256  avgt    5   75.056 ? 0.741  us/op
StringCompare.compareUUDiffStringsTurnOffCCP           7     256  avgt    5   16.132 ? 0.195  us/op
StringCompare.compareUUDiffStringsTurnOffCCP          15     256  avgt    5   17.423 ? 0.023  us/op
StringCompare.compareUUDiffStringsTurnOffCCP          31     256  avgt    5   20.102 ? 0.112  us/op
StringCompare.compareUUDiffStringsTurnOffCCP          47     256  avgt    5   25.529 ? 0.367  us/op
StringCompare.compareUUDiffStringsTurnOffCCP          63     256  avgt    5   26.804 ? 0.051  us/op
StringCompare.compareUUDiffStringsTurnOffCCP         127     256  avgt    5   40.988 ? 0.425  us/op
StringCompare.compareUUDiffStringsTurnOffCCP         255     256  avgt    5   77.157 ? 0.187  us/op

On the Raspberry Pi, the improvement is more obvious when the diff_pos is above 127.

I meant the earlier String.compareTo that this is partially replacing. This one might be fine but I just wanted to check it had be thoroughly tested. For reference they were:

https://bugs.openjdk.java.net/browse/JDK-8215100
https://bugs.openjdk.java.net/browse/JDK-8237524
https://bugs.openjdk.java.net/browse/JDK-8218966

Thank you for your reminder. This version can pass these three test cases. In addition, we tested tier1-tier3 and no new test cases fail.

@wuyan0
Copy link

@wuyan0 wuyan0 commented Aug 5, 2021

We also tested this version on Hisilicon, increasing the count of each test and the length of the string because the test data fluctuates more. When diff_pos is greater than 255, the improvement will be more obvious. And in all cases there was no significant decline.

base:
Benchmark                                     (diff_pos)  (size)  Mode  Cnt   Score   Error  Units
StringCompare.compareLLDiffStrings                     7     512  avgt   50   5.481 ? 1.230  us/op
StringCompare.compareLLDiffStrings                    31     512  avgt   50   6.944 ? 0.962  us/op
StringCompare.compareLLDiffStrings                    63     512  avgt   50  10.129 ? 0.973  us/op
StringCompare.compareLLDiffStrings                   127     512  avgt   50  15.944 ? 0.786  us/op
StringCompare.compareLLDiffStrings                   255     512  avgt   50  28.233 ? 0.737  us/op
StringCompare.compareLLDiffStrings                   511     512  avgt   50  51.612 ? 1.357  us/op
StringCompare.compareUUDiffStrings                     7     512  avgt   50   5.552 ? 0.809  us/op
StringCompare.compareUUDiffStrings                    31     512  avgt   50  12.024 ? 1.499  us/op
StringCompare.compareUUDiffStrings                    63     512  avgt   50  15.368 ? 0.009  us/op
StringCompare.compareUUDiffStrings                   127     512  avgt   50  28.354 ? 0.655  us/op
StringCompare.compareUUDiffStrings                   255     512  avgt   50  52.932 ? 0.598  us/op
StringCompare.compareUUDiffStrings                   511     512  avgt   50  99.377 ? 1.194  us/op
StringCompare.compareUUDiffStringsTurnOffCCP           7     512  avgt   50   5.599 ? 0.801  us/op
StringCompare.compareUUDiffStringsTurnOffCCP          31     512  avgt   50  10.200 ? 1.206  us/op
StringCompare.compareUUDiffStringsTurnOffCCP          63     512  avgt   50  15.897 ? 0.783  us/op
StringCompare.compareUUDiffStringsTurnOffCCP         127     512  avgt   50  28.420 ? 0.818  us/op
StringCompare.compareUUDiffStringsTurnOffCCP         255     512  avgt   50  53.377 ? 1.049  us/op
StringCompare.compareUUDiffStringsTurnOffCCP         511     512  avgt   50  98.362 ? 0.889  us/op

opt:
Benchmark                                     (diff_pos)  (size)  Mode  Cnt   Score   Error  Units
StringCompare.compareLLDiffStrings                     7     512  avgt   50   6.338 ? 1.513  us/op
StringCompare.compareLLDiffStrings                    31     512  avgt   50   6.734 ? 0.789  us/op
StringCompare.compareLLDiffStrings                    63     512  avgt   50   9.799 ? 1.757  us/op
StringCompare.compareLLDiffStrings                   127     512  avgt   50  13.954 ? 1.688  us/op
StringCompare.compareLLDiffStrings                   255     512  avgt   50  19.128 ? 1.024  us/op
StringCompare.compareLLDiffStrings                   511     512  avgt   50  38.108 ? 1.595  us/op
StringCompare.compareUUDiffStrings                     7     512  avgt   50   7.327 ? 1.376  us/op
StringCompare.compareUUDiffStrings                    31     512  avgt   50   8.969 ? 1.311  us/op
StringCompare.compareUUDiffStrings                    63     512  avgt   50  13.482 ? 1.623  us/op
StringCompare.compareUUDiffStrings                   127     512  avgt   50  20.602 ? 1.495  us/op
StringCompare.compareUUDiffStrings                   255     512  avgt   50  37.906 ? 3.763  us/op
StringCompare.compareUUDiffStrings                   511     512  avgt   50  72.682 ? 6.374  us/op
StringCompare.compareUUDiffStringsTurnOffCCP           7     512  avgt   50   6.638 ? 1.279  us/op
StringCompare.compareUUDiffStringsTurnOffCCP          31     512  avgt   50   8.044 ? 0.924  us/op
StringCompare.compareUUDiffStringsTurnOffCCP          63     512  avgt   50  12.921 ? 1.540  us/op
StringCompare.compareUUDiffStringsTurnOffCCP         127     512  avgt   50  19.010 ? 1.333  us/op
StringCompare.compareUUDiffStringsTurnOffCCP         255     512  avgt   50  36.326 ? 1.302  us/op
StringCompare.compareUUDiffStringsTurnOffCCP         511     512  avgt   50  75.746 ? 3.786  us/op

@wuyan0
Copy link

@wuyan0 wuyan0 commented Aug 24, 2021

Hi, @theRealAph @nick-arm, The test data looks OK on Raspberry Pi 4B and Hisilicon, do you have any other questions about this patch?

Copy link
Member

@nick-arm nick-arm left a comment

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.

int largeLoopExitCondition = MAX2(64, SoftwarePrefetchHintDistance)/(isLL ? 1 : 2);
// cnt1/cnt2 contains amount of characters to compare. cnt1 can be re-used
// update cnt2 counter with already loaded 8 bytes
int largeLoopExitCondition = MAX(64, SoftwarePrefetchHintDistance)/(isLL ? 1 : 2);
Copy link
Member

@nick-arm nick-arm Aug 25, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link

@wuyan0 wuyan0 Aug 26, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, I'll fix it.

Copy link
Contributor

@theRealAph theRealAph Sep 5, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's fine. I don't think it'll affect any real programs, so it's rather pointless. I don't know if that's any reason not to approve it.

Copy link

@wuyan0 wuyan0 Sep 17, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Andrew, can you help us to approve this?

Copy link
Contributor

@adinn adinn Sep 20, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree with Andrew Haley that this patch is not going to make an improvement for anything but a very small number of applications. Processing of strings over a few 10s of bytes is rare. On the other hand the doesn't seem to cause any performance drop for the much more common case of processing short strings. so it does no harm. Also, the new and old code are much the same in terms of complexity so that is no reason to prefer one over the other. The only real concern I have is that any change involves the risk of error and the ratio of cases that might benefit to cases that might suffer from an error is very low. I don't think that's a reason to avoid pushing this patch upstream but it does suggest that we should not backport it.

Copy link
Contributor

@theRealAph theRealAph Sep 20, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, thanks. That seems like a sensible compromise.

@wuyan0
Copy link

@wuyan0 wuyan0 commented Aug 26, 2021

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.

OK, Thank you very much!

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.

The JMH benchmark added by JDK-8269559 (#5129) can cover our test items (compareToLL and compareToUU), and can show the improvement of our patch, so we decided to delete our JMH benchmark in the next commit.
The test results using that JMH benchmark in JDK-8269559 are as follows:

Raspberry Pi 4B
base:
Benchmark                                   (delta)  (size)  Mode  Cnt   Score   Error  Units
StringCompareToDifferentLength.compareToLL        2      24  avgt    3   2.310 ? 0.050  ms/op
StringCompareToDifferentLength.compareToLL        2      36  avgt    3   2.818 ? 0.185  ms/op
StringCompareToDifferentLength.compareToLL        2      72  avgt    3   3.151 ? 0.215  ms/op
StringCompareToDifferentLength.compareToLL        2     128  avgt    3   4.171 ? 1.320  ms/op
StringCompareToDifferentLength.compareToLL        2     256  avgt    3   6.169 ? 0.653  ms/op
StringCompareToDifferentLength.compareToLL        2     512  avgt    3  10.911 ? 0.175  ms/op
StringCompareToDifferentLength.compareToLU        2      24  avgt    3   3.312 ? 0.102  ms/op
StringCompareToDifferentLength.compareToLU        2      36  avgt    3   4.162 ? 0.032  ms/op
StringCompareToDifferentLength.compareToLU        2      72  avgt    3   5.705 ? 0.152  ms/op
StringCompareToDifferentLength.compareToLU        2     128  avgt    3   9.301 ? 0.749  ms/op
StringCompareToDifferentLength.compareToLU        2     256  avgt    3  16.507 ? 1.353  ms/op
StringCompareToDifferentLength.compareToLU        2     512  avgt    3  30.160 ? 0.377  ms/op
StringCompareToDifferentLength.compareToUL        2      24  avgt    3   3.366 ? 0.280  ms/op
StringCompareToDifferentLength.compareToUL        2      36  avgt    3   4.308 ? 0.037  ms/op
StringCompareToDifferentLength.compareToUL        2      72  avgt    3   5.674 ? 0.210  ms/op
StringCompareToDifferentLength.compareToUL        2     128  avgt    3   9.358 ? 0.158  ms/op
StringCompareToDifferentLength.compareToUL        2     256  avgt    3  16.165 ? 0.158  ms/op
StringCompareToDifferentLength.compareToUL        2     512  avgt    3  29.857 ? 0.277  ms/op
StringCompareToDifferentLength.compareToUU        2      24  avgt    3   3.149 ? 0.209  ms/op
StringCompareToDifferentLength.compareToUU        2      36  avgt    3   3.157 ? 0.102  ms/op
StringCompareToDifferentLength.compareToUU        2      72  avgt    3   4.415 ? 0.073  ms/op
StringCompareToDifferentLength.compareToUU        2     128  avgt    3   6.244 ? 0.224  ms/op
StringCompareToDifferentLength.compareToUU        2     256  avgt    3  11.032 ? 0.080  ms/op
StringCompareToDifferentLength.compareToUU        2     512  avgt    3  20.942 ? 3.973  ms/op

opt:
Benchmark                                   (delta)  (size)  Mode  Cnt   Score   Error  Units
StringCompareToDifferentLength.compareToLL        2      24  avgt    3   2.319 ? 0.121  ms/op
StringCompareToDifferentLength.compareToLL        2      36  avgt    3   2.820 ? 0.096  ms/op
StringCompareToDifferentLength.compareToLL        2      72  avgt    3   2.511 ? 0.024  ms/op
StringCompareToDifferentLength.compareToLL        2     128  avgt    3   3.496 ? 0.382  ms/op
StringCompareToDifferentLength.compareToLL        2     256  avgt    3   5.215 ? 0.210  ms/op
StringCompareToDifferentLength.compareToLL        2     512  avgt    3   7.772 ? 0.448  ms/op
StringCompareToDifferentLength.compareToLU        2      24  avgt    3   3.432 ? 0.249  ms/op
StringCompareToDifferentLength.compareToLU        2      36  avgt    3   4.156 ? 0.052  ms/op
StringCompareToDifferentLength.compareToLU        2      72  avgt    3   5.735 ? 0.043  ms/op
StringCompareToDifferentLength.compareToLU        2     128  avgt    3   9.215 ? 0.394  ms/op
StringCompareToDifferentLength.compareToLU        2     256  avgt    3  16.373 ? 0.515  ms/op
StringCompareToDifferentLength.compareToLU        2     512  avgt    3  29.906 ? 0.245  ms/op
StringCompareToDifferentLength.compareToUL        2      24  avgt    3   3.361 ? 0.116  ms/op
StringCompareToDifferentLength.compareToUL        2      36  avgt    3   4.253 ? 0.061  ms/op
StringCompareToDifferentLength.compareToUL        2      72  avgt    3   5.744 ? 0.082  ms/op
StringCompareToDifferentLength.compareToUL        2     128  avgt    3   9.167 ? 0.343  ms/op
StringCompareToDifferentLength.compareToUL        2     256  avgt    3  16.591 ? 0.999  ms/op
StringCompareToDifferentLength.compareToUL        2     512  avgt    3  30.232 ? 2.057  ms/op
StringCompareToDifferentLength.compareToUU        2      24  avgt    3   3.147 ? 0.057  ms/op
StringCompareToDifferentLength.compareToUU        2      36  avgt    3   2.526 ? 0.027  ms/op
StringCompareToDifferentLength.compareToUU        2      72  avgt    3   3.832 ? 0.228  ms/op
StringCompareToDifferentLength.compareToUU        2     128  avgt    3   5.332 ? 0.173  ms/op
StringCompareToDifferentLength.compareToUU        2     256  avgt    3   8.417 ? 0.551  ms/op
StringCompareToDifferentLength.compareToUU        2     512  avgt    3  14.903 ? 0.782  ms/op

Hisilicon
base:
Benchmark                                   (delta)  (size)  Mode  Cnt   Score   Error  Units
StringCompareToDifferentLength.compareToLL        2      24  avgt   30   0.824 ? 0.003  ms/op
StringCompareToDifferentLength.compareToLL        2      36  avgt   30   1.123 ? 0.050  ms/op
StringCompareToDifferentLength.compareToLL        2      72  avgt   30   1.550 ? 0.052  ms/op
StringCompareToDifferentLength.compareToLL        2     128  avgt   30   2.015 ? 0.040  ms/op
StringCompareToDifferentLength.compareToLL        2     256  avgt   30   3.154 ? 0.032  ms/op
StringCompareToDifferentLength.compareToLL        2     512  avgt   30   5.519 ? 0.044  ms/op
StringCompareToDifferentLength.compareToLU        2      24  avgt   30   1.469 ? 0.196  ms/op
StringCompareToDifferentLength.compareToLU        2      36  avgt   30   1.777 ? 0.097  ms/op
StringCompareToDifferentLength.compareToLU        2      72  avgt   30   2.509 ? 0.073  ms/op
StringCompareToDifferentLength.compareToLU        2     128  avgt   30   3.914 ? 0.044  ms/op
StringCompareToDifferentLength.compareToLU        2     256  avgt   30   6.773 ? 0.049  ms/op
StringCompareToDifferentLength.compareToLU        2     512  avgt   30  12.504 ? 0.081  ms/op
StringCompareToDifferentLength.compareToUL        2      24  avgt   30   1.505 ? 0.107  ms/op
StringCompareToDifferentLength.compareToUL        2      36  avgt   30   1.976 ? 0.145  ms/op
StringCompareToDifferentLength.compareToUL        2      72  avgt   30   2.593 ? 0.082  ms/op
StringCompareToDifferentLength.compareToUL        2     128  avgt   30   3.998 ? 0.062  ms/op
StringCompareToDifferentLength.compareToUL        2     256  avgt   30   6.949 ? 0.110  ms/op
StringCompareToDifferentLength.compareToUL        2     512  avgt   30  12.617 ? 0.068  ms/op
StringCompareToDifferentLength.compareToUU        2      24  avgt   30   1.232 ? 0.038  ms/op
StringCompareToDifferentLength.compareToUU        2      36  avgt   30   1.505 ? 0.008  ms/op
StringCompareToDifferentLength.compareToUU        2      72  avgt   30   2.218 ? 0.066  ms/op
StringCompareToDifferentLength.compareToUU        2     128  avgt   30   3.329 ? 0.119  ms/op
StringCompareToDifferentLength.compareToUU        2     256  avgt   30   5.684 ? 0.030  ms/op
StringCompareToDifferentLength.compareToUU        2     512  avgt   30  10.520 ? 0.031  ms/op

opt:
Benchmark                                   (delta)  (size)  Mode  Cnt   Score   Error  Units
StringCompareToDifferentLength.compareToLL        2      24  avgt   30   0.824 ? 0.003  ms/op
StringCompareToDifferentLength.compareToLL        2      36  avgt   30   1.124 ? 0.032  ms/op
StringCompareToDifferentLength.compareToLL        2      72  avgt   30   1.376 ? 0.123  ms/op
StringCompareToDifferentLength.compareToLL        2     128  avgt   30   1.921 ? 0.040  ms/op
StringCompareToDifferentLength.compareToLL        2     256  avgt   30   2.656 ? 0.156  ms/op
StringCompareToDifferentLength.compareToLL        2     512  avgt   30   4.311 ? 0.267  ms/op
StringCompareToDifferentLength.compareToLU        2      24  avgt   30   1.391 ? 0.154  ms/op
StringCompareToDifferentLength.compareToLU        2      36  avgt   30   1.891 ? 0.170  ms/op
StringCompareToDifferentLength.compareToLU        2      72  avgt   30   2.496 ? 0.082  ms/op
StringCompareToDifferentLength.compareToLU        2     128  avgt   30   3.978 ? 0.046  ms/op
StringCompareToDifferentLength.compareToLU        2     256  avgt   30   6.811 ? 0.057  ms/op
StringCompareToDifferentLength.compareToLU        2     512  avgt   30  12.586 ? 0.054  ms/op
StringCompareToDifferentLength.compareToUL        2      24  avgt   30   1.462 ? 0.085  ms/op
StringCompareToDifferentLength.compareToUL        2      36  avgt   30   1.864 ? 0.070  ms/op
StringCompareToDifferentLength.compareToUL        2      72  avgt   30   2.651 ? 0.090  ms/op
StringCompareToDifferentLength.compareToUL        2     128  avgt   30   4.223 ? 0.383  ms/op
StringCompareToDifferentLength.compareToUL        2     256  avgt   30   6.858 ? 0.085  ms/op
StringCompareToDifferentLength.compareToUL        2     512  avgt   30  12.675 ? 0.099  ms/op
StringCompareToDifferentLength.compareToUU        2      24  avgt   30   1.200 ? 0.013  ms/op
StringCompareToDifferentLength.compareToUU        2      36  avgt   30   1.336 ? 0.156  ms/op
StringCompareToDifferentLength.compareToUU        2      72  avgt   30   2.364 ? 0.545  ms/op
StringCompareToDifferentLength.compareToUU        2     128  avgt   30   2.753 ? 0.154  ms/op
StringCompareToDifferentLength.compareToUU        2     256  avgt   30   5.179 ? 0.834  ms/op
StringCompareToDifferentLength.compareToUU        2     512  avgt   30   7.090 ? 0.423  ms/op

@wuyan0
Copy link

@wuyan0 wuyan0 commented Sep 3, 2021

@theRealAph do you have any other questions about this patch?

@nick-arm
Copy link
Member

@nick-arm nick-arm commented Sep 7, 2021

Please check the Windows tier1 failure: https://github.com/Wanghuang-Huawei/jdk/runs/3459332995

Seems unlikely that it's anything to do with this patch so you may just want to re-run it or merge from master.

@wuyan0
Copy link

@wuyan0 wuyan0 commented Sep 9, 2021

Please check the Windows tier1 failure: https://github.com/Wanghuang-Huawei/jdk/runs/3459332995

Seems unlikely that it's anything to do with this patch so you may just want to re-run it or merge from master.

OK, The rerun of presubmit test show that it passed all tests. The result is here: https://github.com/Wanghuang-Huawei/jdk/actions/runs/1181122290

@openjdk
Copy link

@openjdk openjdk bot commented Sep 9, 2021

@Wanghuang-Huawei This change now passes all automated pre-integration checks.

ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details.

After integration, the commit message for the final commit will be:

8268231: Aarch64: Use ldp in intrinsics for String.compareTo

Co-authored-by: Wang Huang <whuang@openjdk.org>
Co-authored-by: Sun Jianye <sunjianye@huawei.com>
Co-authored-by: Wu Yan <wuyan@openjdk.org>
Reviewed-by: ngasson, aph

You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed.

At the time when this comment was updated there had been 761 new commits pushed to the master branch:

  • 2166ed1: 8273894: ConcurrentModificationException raised every time ReferralsCache drops referral
  • 1c6fa11: 8273979: move some os time related functions to os_posix for POSIX platforms
  • 45adc92: 8273578: javax/swing/JMenu/4515762/bug4515762.java fails on macOS 12
  • 0fbbe4c: 8274033: Some tier-4 CDS EpsilonGC tests throw OOM
  • 9d3379b: 8267356: AArch64: Vector API SVE codegen support
  • 6031388: 8273714: jdk/jfr/api/consumer/TestRecordedFrame.java still times out after JDK-8273047
  • 8821b00: 8205137: Remove Applet support from SwingSet2
  • 57fe11c: 8274120: [JVMCI] CompileBroker should resolve parameter types for JVMCI compiles
  • 81d4164: 8272759: (fc) java/nio/channels/FileChannel/Transfer2GPlus.java failed in timeout
  • da38ced: 8271602: Add Math.ceilDiv() family parallel to Math.floorDiv() family
  • ... and 751 more: https://git.openjdk.java.net/jdk/compare/90c219f37bc7da2a556d1733a148a7d445e900e3...master

As there are no conflicts, your changes will automatically be rebased on top of these commits when integrating. If you prefer to avoid this automatic rebasing, please check the documentation for the /integrate command for further details.

As you do not have Committer status in this project an existing Committer must agree to sponsor your change. Possible candidates are the reviewers of this PR (@nick-arm, @theRealAph) but any other Committer may sponsor as well.

➡️ To flag this PR as ready for integration with the above commit message, type /integrate in a new comment. (Afterwards, your sponsor types /sponsor in a new comment to perform the integration).

@openjdk openjdk bot added the ready label Sep 9, 2021
@Wanghuang-Huawei
Copy link
Author

@Wanghuang-Huawei Wanghuang-Huawei commented Sep 23, 2021

/integrate

@openjdk openjdk bot added the sponsor label Sep 23, 2021
@openjdk
Copy link

@openjdk openjdk bot commented Sep 23, 2021

@Wanghuang-Huawei
Your change (at version 8cf3b2c) is now ready to be sponsored by a Committer.

@Hamlin-Li
Copy link

@Hamlin-Li Hamlin-Li commented Oct 9, 2021

/sponsor

@openjdk
Copy link

@openjdk openjdk bot commented Oct 9, 2021

Going to push as commit 6d1d4d5.
Since your change was applied there have been 957 commits pushed to the master branch:

  • aac6c4e: 8272229: BasicSplitPaneDivider:oneTouchExpandableChanged() returns leftButton and rightButton as null with GTKLookAndFeel
  • f640c7a: 8274806: Simplify equals() call on nullable variable and a constant in java.desktop
  • 9c431dd: 8274900: Too weak variable type leads to unnecessary cast in jdk.javadoc
  • 3cb9724: 8274934: Attempting to acquire lock JNICritical_lock/41 out of order with lock MultiArray_lock/41
  • 239a35a: 8233749: Files.exists javadoc doesn't mention eating IOException
  • ec19907: 8274864: Remove Amman/Cairo hacks in ZoneInfoFile
  • ccbce10: 8272756: Remove unnecessary explicit initialization of volatile variables in java.desktop
  • 36b89a1: 8274785: ciReplay: Potential crash due to uninitialized Compile::_ilt variable
  • 2aacd42: 8274145: C2: condition incorrectly made redundant with dominating main loop exit condition
  • 6364719: 8274004: Change 'nonleaf' rank name
  • ... and 947 more: https://git.openjdk.java.net/jdk/compare/90c219f37bc7da2a556d1733a148a7d445e900e3...master

Your commit was automatically rebased without conflicts.

@openjdk openjdk bot closed this Oct 9, 2021
@openjdk openjdk bot added integrated and removed ready rfr sponsor labels Oct 9, 2021
@openjdk
Copy link

@openjdk openjdk bot commented Oct 9, 2021

@Hamlin-Li @Wanghuang-Huawei Pushed as commit 6d1d4d5.

💡 You may see a message that your pull request was closed with unmerged commits. This can be safely ignored.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core-libs hotspot-compiler integrated
6 participants