RFR(S): 8203669: PPC64: Fix jtreg RTM tests after "8203305: Improve TM detection for enabling RTM on Linux / POWER9"
Lindenmaier, Goetz
goetz.lindenmaier at sap.com
Wed May 30 13:05:26 UTC 2018
Hi Gustavo,
> OK. But are you saying that in addition to those RTM tests failing before
> 8203305 there are now others (even before my change there were test
> failing)?
No, I meant the whole testing failed. Some infrastructure problem.
Great you tracked down the below issue!
Best regards,
Goetz.
> If so, do you mind to point them out and on which CPU/OS that's happening?
>
> On Linux I've tested on various CPU/OS combinations and with 8203669 I see
> all RTM tests as they were before my change (8203305), i.e. no regressions.
>
> Regarding the test failing before 8203305, I noted yesterday that at least two
> of them on PPC (and several on x86) are failing because of addressSize() in
> Unsafe since change
> http://hg.openjdk.java.net/jdk9/jdk9/jdk/rev/05a1166a8201 made
> addressSize() not any more a native method. Native methods are used in
> RTM test
> suite as a RTM abort provoker, so the following change (I'll submit soon that)
> make them pass again (getSize() is still native):
>
>
> diff -r 65c0162dea88
> test/hotspot/jtreg/compiler/rtm/locking/TestRTMAbortRatio.java
> --- a/test/hotspot/jtreg/compiler/rtm/locking/TestRTMAbortRatio.java
> Mon May 21 13:23:55 2018 -0400
> +++ b/test/hotspot/jtreg/compiler/rtm/locking/TestRTMAbortRatio.java
> Wed May 30 08:51:57 2018 -0400
> @@ -136,7 +136,7 @@
> public void lock(boolean abort) {
> synchronized(monitor) {
> if (abort) {
> - Test.UNSAFE.addressSize();
> + Test.UNSAFE.pageSize();
> }
> }
> }
> diff -r 65c0162dea88
> test/hotspot/jtreg/compiler/rtm/locking/TestRTMDeoptOnLowAbortRatio.ja
> va
> ---
> a/test/hotspot/jtreg/compiler/rtm/locking/TestRTMDeoptOnLowAbortRatio
> .java Mon May 21 13:23:55 2018 -0400
> +++
> b/test/hotspot/jtreg/compiler/rtm/locking/TestRTMDeoptOnLowAbortRatio
> .java Wed May 30 08:51:57 2018 -0400
> @@ -140,7 +140,7 @@
> public void forceAbort(boolean abort) {
> synchronized(monitor) {
> if (abort) {
> - Test.UNSAFE.addressSize();
> + Test.UNSAFE.pageSize();
> }
> }
> }
> diff -r 65c0162dea88
> test/hotspot/jtreg/compiler/rtm/locking/TestRTMLockingThreshold.java
> ---
> a/test/hotspot/jtreg/compiler/rtm/locking/TestRTMLockingThreshold.java
> Mon May 21 13:23:55 2018 -0400
> +++
> b/test/hotspot/jtreg/compiler/rtm/locking/TestRTMLockingThreshold.java
> Wed May 30 08:51:57 2018 -0400
> @@ -151,7 +151,7 @@
> public void lock(boolean abort) {
> synchronized(monitor) {
> if (abort) {
> - Test.field += Test.UNSAFE.addressSize();
> + Test.field += Test.UNSAFE.pageSize();
> }
> }
> }
> diff -r 65c0162dea88
> test/hotspot/jtreg/compiler/testlibrary/rtm/XAbortProvoker.java
> --- a/test/hotspot/jtreg/compiler/testlibrary/rtm/XAbortProvoker.java
> Mon May 21 13:23:55 2018 -0400
> +++ b/test/hotspot/jtreg/compiler/testlibrary/rtm/XAbortProvoker.java
> Wed May 30 08:51:57 2018 -0400
> @@ -46,7 +46,7 @@
> @Override
> public void forceAbort() {
> synchronized(monitor) {
> - XAbortProvoker.field = UNSAFE.addressSize();
> + XAbortProvoker.field = UNSAFE.pageSize();
> }
> }
>
> @@ -54,7 +54,7 @@
> public String[] getMethodsToCompileNames() {
> return new String[] {
> getMethodWithLockName(),
> - Unsafe.class.getName() + "::addressSize"
> + Unsafe.class.getName() + "::pageSize"
> };
> }
> }
>
>
> Best regards,
> Gustavo
>
> > Best regards,
> > Goetz.
> >
> >> -----Original Message-----
> >> From: Gustavo Romero [mailto:gromero at linux.vnet.ibm.com]
> >> Sent: Montag, 28. Mai 2018 16:32
> >> To: Doerr, Martin <martin.doerr at sap.com>; hotspot-compiler-
> >> dev at openjdk.java.net; ppc-aix-port-dev at openjdk.java.net
> >> Cc: Lindenmaier, Goetz <goetz.lindenmaier at sap.com>
> >> Subject: Re: RFR(S): 8203669: PPC64: Fix jtreg RTM tests after "8203305:
> >> Improve TM detection for enabling RTM on Linux / POWER9"
> >>
> >> Hi Martin,
> >>
> >> On 05/28/2018 06:43 AM, Doerr, Martin wrote:
> >>> Hi Gustavo,
> >>>
> >>> thanks for fixing AIX. Looks correct.
> >>>
> >>> I think the tests don't fail because of the warning. I was just wondering
> why
> >> we're spending time for something we don't want to support. I think you
> can
> >> remove the warning with this change if you like.
> >>
> >> Got it. The warning message is removed and now the JVM behaves as
> >> on x86 regarding UseRTMLocking:
> >>
> >> http://cr.openjdk.java.net/~gromero/POWER9/8203669/v3/
> >>
> >>
> >> Best regards,
> >> Gustavo
> >>
> >>> Maybe the tests don't fit for PPC64. We have too many other things to
> do
> >> in jdk11 time frame. Maybe it'd be better to switch them off for PPC64.
> >>>
> >>> Best regards,
> >>> Martin
> >>>
> >>>
> >>> -----Original Message-----
> >>> From: Gustavo Romero [mailto:gromero at linux.vnet.ibm.com]
> >>> Sent: Freitag, 25. Mai 2018 22:44
> >>> To: Doerr, Martin <martin.doerr at sap.com>; hotspot-compiler-
> >> dev at openjdk.java.net; ppc-aix-port-dev at openjdk.java.net
> >>> Cc: Lindenmaier, Goetz <goetz.lindenmaier at sap.com>
> >>> Subject: Re: RFR(S): 8203669: PPC64: Fix jtreg RTM tests after "8203305:
> >> Improve TM detection for enabling RTM on Linux / POWER9"
> >>>
> >>> Hi Martin,
> >>>
> >>> On 05/25/2018 10:29 AM, Doerr, Martin wrote:
> >>>> Hi Gustavo,
> >>>>
> >>>> after talking with Götz, we found a couple of problems.
> >>>>
> >>>> vm_version_ppc.cpp:
> >>>> AIX 7.2 on Power7 would report "rtm" in the feature string which is not
> >> true.
> >>>> The check for Power8 is missing here. However,
> PowerArchitecturePPC64
> >> can't be used here because it's not initialized at this point. "has_lqarx()"
> could
> >> be used.
> >>>
> >>> I see. In the end "rtm" for AIX is being reported if AIX version matches
> and
> >> is
> >>> not taking the CPU version into account. On Linux that seems not an
> issue
> >>> because if we find any HTM support on AUXV it's pretty sure that the
> CPU
> >>> supports it so reporting "rtm" after only checking AUXV is correct. So I
> kept
> >>> code unchanged for Linux. As has_lqarx() checks '_features' and not
> >> 'features' I
> >>> decided for moving the code below '_features = features' attribution.
> >>>
> >>> Would that version work on AIX?
> >>>
> >>> http://cr.openjdk.java.net/~gromero/POWER9/8203669/v2/
> >>>
> >>>
> >>>> Some tests complain:
> >>>> OpenJDK 64-Bit Server VM warning: UseRTMLocking is only available as
> >> experimental option on this platform.
> >>>
> >>> hmm right. Do you mean on Linux as well? That change was supposed to
> >> not cause
> >>> that kind of test complain... Can you share some logs on that?
> >>>
> >>> On POWER8 LE (just like before 8203305 when in) I see only 10 test
> failing:
> >>>
> >>> FAILED: compiler/rtm/locking/TestRTMAbortThreshold.java
> >>> FAILED: compiler/rtm/locking/TestRTMAfterNonRTMDeopt.java
> >>> FAILED: compiler/rtm/locking/TestRTMDeoptOnHighAbortRatio.java
> >>> FAILED: compiler/rtm/locking/TestRTMDeoptOnLowAbortRatio.java
> >>> FAILED: compiler/rtm/locking/TestRTMLockingCalculationDelay.java
> >>> FAILED: compiler/rtm/locking/TestRTMLockingThreshold.java
> >>> FAILED: compiler/rtm/locking/TestRTMRetryCount.java
> >>> FAILED: compiler/rtm/locking/TestRTMSpinLoopCount.java
> >>> FAILED: compiler/rtm/locking/TestUseRTMXendForLockBusy.java
> >>> FAILED: compiler/rtm/print/TestPrintPreciseRTMLockingStatistics.java
> >>>
> >>> and none is because of RTM being an experimental feature. Here are
> the
> >> reasons
> >>> (not in the same order of the above list):
> >>>
> >>> gromero at gromero16:~/hg/jdk11/jdk$ egrep -E "TEST.*failed:" rtm.log
> >>> TEST RESULT: Failed. Execution failed: `main' threw exception:
> >> java.lang.RuntimeException: Expected that method with rtm lock elision
> was
> >> deoptimized after 1 lock attempts: expected 10000 to equal 1
> >>> TEST RESULT: Failed. Execution failed: `main' threw exception:
> >> java.lang.RuntimeException: Total aborts count (2001) should be less or
> equal
> >> to 1001: expected that 2001 <= 1001
> >>> TEST RESULT: Failed. Execution failed: `main' threw exception:
> >> java.lang.RuntimeException: VM output should contain two RTM locking
> >> statistics entries.: expected 1 to equal 2
> >>> TEST RESULT: Failed. Execution failed: `main' threw exception:
> >> java.lang.RuntimeException: Expected to get only one deoptimization due
> to
> >> rtm state change: expected 0 to equal 1
> >>> TEST RESULT: Failed. Execution failed: `main' threw exception:
> >> java.lang.RuntimeException: Expected to get only one deoptimization due
> to
> >> rtm state change: expected 0 to equal 1
> >>> TEST RESULT: Failed. Execution failed: `main' threw exception:
> >> java.lang.RuntimeException: At least one deoptimization due to
> >> rtm_state_chage is expected: expected 0 > 0
> >>> TEST RESULT: Failed. Execution failed: `main' threw exception:
> >> java.lang.RuntimeException: RTM locking statistics should contain non
> zero
> >> total aborts count: expected 0 > 0
> >>> TEST RESULT: Failed. Execution failed: `main' threw exception:
> >> java.lang.RuntimeException: Two uncommon traps with reason
> >> rtm_state_change should be fired.: expected 0 to equal 2
> >>> TEST RESULT: Failed. Execution failed: `main' threw exception:
> >> java.lang.RuntimeException: It is expected to get 2 aborts: expected 3 to
> >> equal 2
> >>> TEST RESULT: Failed. Execution failed: `main' threw exception:
> >> java.lang.RuntimeException: VM output should contain exactly one rtm
> >> locking statistics entry for method compiler.testlibrary.rtm.BusyLock::test:
> >> expected 0 to equal 1
> >>>
> >>>
> >>>> I think the warnings or vm_exit stuff could be removed if you want to
> >> support it officially. X86 only uses it for certain older processors.
> >>>
> >>> Got it. I see the same failing RTM tests on my x86 machine (Skylake) with
> >> RTM
> >>> support. I intended to set it as non-experimental once I was able to get
> all
> >>> tests passing on PPC64. But ok, as you noted maybe on x86 it's just there
> >> on
> >>> because of the initial broken support for RTM on old processors.
> >>>
> >>> Would you like to address it in that bug or should I open another bug?
> >>>
> >>>
> >>>> We'll have to check other tests, too. We'll do that next week.
> >>>
> >>> Thank you so much. I wished I had a change to read/reply that email
> earlier
> >> but
> >>> the whole email system was down until now.
> >>>
> >>>
> >>> Best regards,
> >>> Gustavo
> >>>
> >>>> Best regards,
> >>>> Martin
> >>>>
> >>>>
> >>>> -----Original Message-----
> >>>> From: Doerr, Martin
> >>>> Sent: Donnerstag, 24. Mai 2018 11:19
> >>>> To: 'Gustavo Romero' <gromero at linux.vnet.ibm.com>; hotspot-
> >> compiler-dev at openjdk.java.net; ppc-aix-port-dev at openjdk.java.net
> >>>> Cc: Lindenmaier, Goetz <goetz.lindenmaier at sap.com>
> >>>> Subject: RE: RFR(S): 8203669: PPC64: Fix jtreg RTM tests after "8203305:
> >> Improve TM detection for enabling RTM on Linux / POWER9"
> >>>>
> >>>> Hi Gustavo,
> >>>>
> >>>> the fix looks good. I especially like that you use "rtm" like x86, now.
> >>>> I can sponsor it after we got a 2nd review.
> >>>>
> >>>> Best regards,
> >>>> Martin
> >>>>
> >>>>
> >>>> -----Original Message-----
> >>>> From: ppc-aix-port-dev [mailto:ppc-aix-port-dev-
> >> bounces at openjdk.java.net] On Behalf Of Gustavo Romero
> >>>> Sent: Mittwoch, 23. Mai 2018 15:49
> >>>> To: hotspot-compiler-dev at openjdk.java.net; ppc-aix-port-
> >> dev at openjdk.java.net
> >>>> Subject: RFR(S): 8203669: PPC64: Fix jtreg RTM tests after "8203305:
> >> Improve TM detection for enabling RTM on Linux / POWER9"
> >>>>
> >>>> Hi,
> >>>>
> >>>> Could the following small fix be reviewed please?
> >>>>
> >>>> It touches shared code.
> >>>>
> >>>> bug: https://bugs.openjdk.java.net/browse/JDK-8203669
> >>>> webrev: http://cr.openjdk.java.net/~gromero/POWER9/8203669
> >>>>
> >>>> Since change "8203305: Improve TM detection for enabling RTM on
> Linux /
> >> POWER9"
> >>>> JVM on PPC64 is not advertising anymore the feature "tcheck" which
> >> indicated
> >>>> that RTM was supported by the JVM in case both CPU and OS
> supported
> >> hardware
> >>>> transactional memory.
> >>>>
> >>>> It happens that several tests under /test/hotspot/jtreg/compiler/rtm
> rely
> >> on the
> >>>> JVM to advertise if RTM is supported by the JVM on a given CPU/OS
> and
> >> so they
> >>>> were failing to be skipped (or not) on different PPC64/Linux
> >> combinations.
> >>>>
> >>>> That change fixes it by re-introducing a feature to inform that RTM is
> >> supported
> >>>> by the JVM when appropriated (given valid OS/CPU support) and now
> it
> >> does it by
> >>>> advertising that feature as on x86_64, reporting the feature as "rtm".
> >>>>
> >>>> It also incorporates a check for when OS does not support RTM but CPU
> >> does in
> >>>> test:
> >>>>
> >>>> TestUseRTMLockingOptionOnUnsupportedCPU.java.
> >>>>
> >>>> Just on PPC64 it might be the case that although CPU supports RTM
> (HTM)
> >>>> instructions the OS version might not support it, like on old kernels that
> do
> >>>> not abort transactions on syscalls. JVM correctly reports each case so
> the
> >> test
> >>>> was adapted to test them.
> >>>>
> >>>> That change also corrects a couple of nit typos elsewhere not related to
> >>>> "8203305".
> >>>>
> >>>>
> >>>> Thank you.
> >>>>
> >>>> Best regards,
> >>>> Gustavo
> >>>>
> >>>
> >
More information about the ppc-aix-port-dev
mailing list