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 12:59:13 UTC 2018
Hi Goetz,
On 05/30/2018 06:34 AM, Lindenmaier, Goetz wrote:
> looks good now.
>
> Unfortunately some of our test runs failed altogether again,
> but let's advance with this for now.
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)?
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.java
--- 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