RFR(S): 8203669: PPC64: Fix jtreg RTM tests after "8203305: Improve TM detection for enabling RTM on Linux / POWER9"
Gustavo Romero
gromero at linux.vnet.ibm.com
Wed May 30 13:13:27 UTC 2018
On 05/30/2018 10:05 AM, Lindenmaier, Goetz wrote:
>> 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.
ah got it. Thanks for reviewing!
> Great you tracked down the below issue!
Yup :-) I'm trying to understand what's going on with the remaining ones.
Best regards,
Gustavo
> 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