RFR(XS): 8204135: jtreg: Fix failing RTM test TestUseRTMXendForLockBusy

[Gustavo Romero]

Hi Igor,

On 05/31/2018 12:53 AM, Igor Ignatyev wrote:
> Hi Gustavo,
> 
> Unfortunately, I don't recall any detail on this test. Filipp (added) might remember something. but I wouldn't expect him to answer though as he hasn't been actively contributing in open jdk lately.
> 
> I have checked the history, the test hasn't been changed since its integration -- http://hg.openjdk.java.net/jdk9/jdk9/hotspot/rev/3c9c3ba62dfd. although your explanation sounds reasonable to me, I don't understand how the test passed before.

OK. I'll wait Filipp's reply. He already helped me a while ago with some
other RTM  tests :-)

Thanks for reviewing it and for checking the history.

BTW, after change 8204134 [1], change 8204136 [2], and this change I
finally got all RTM tests passing again on Intel.

I'm fixing the RTM tests for Power and so I think it's good to agree on how
it has to be on Intel.


Regards,
Gustavo

[1] http://mail.openjdk.java.net/pipermail/hotspot-compiler-dev/2018-May/029144.html
[2] http://mail.openjdk.java.net/pipermail/hotspot-compiler-dev/2018-May/029146.html


> Thanks,
> -- Igor
> 
>> On May 30, 2018, at 7:04 PM, Gustavo Romero <gromero at linux.vnet.ibm.com <mailto:gromero at linux.vnet.ibm.com>> wrote:
>>
>> Hi,
>>
>> Could the following change be reviewed please?
>>
>> webrev: http://cr.openjdk.java.net/~gromero/8204135/v1
>> bug   : https://bugs.openjdk.java.net/browse/JDK-8204135
>>
>> It fixes currently failing RTM test "TestUseRTMXendForLockBusy" by changing
>> in which cases UseRTMForStackLocks flags must be enabled/disabled.
>>
>> I could not track down on the timeline when exactly that test stopped to
>> pass, however my understanding is that if inflateMonitor is 'false' that
>> should /not/ imply that UseRTMForStackLocks is also 'false'. Thus I
>> understand that for this particular test if inflated monitors are used,
>> then UseRTMForStackLocks has to be disabled, i.e. 'false'. On the other
>> hand, if inflated monitors are /not/ used, then UseRTMForStackLocks must be
>> enabled.
>>
>> Currently the test is failing for the following cases where inflateMonitor
>> is 'false', because it implies that UseRTMForStackLocks is disabled and so
>> no abort statistics is generated (which is correct from the JVM's
>> perspective):
>>
>>        // stack lock, xabort on lock busy
>>        verifyXendForLockBusy(false, false);
>>        // stack lock, xend on lock busy
>>        verifyXendForLockBusy(false, true);
>>
>> In other words, "stack lock" case should imply UseRTMForStackLocks = 'true',
>> not 'false'.
>>
>> Cases:
>>        // inflated lock, xabort on lock busy
>>        verifyXendForLockBusy(true, false);
>>        // inflated lock, xend on lock busy
>>        verifyXendForLockBusy(true, true);
>>
>> are not failing because UseRTMForStackLocks = 'true' and they will generate
>> proper abort statistics when  +UseRTMXendForLockBusy ('xend' is used to
>> finish the transaction) or when -UseRTMXendForLockBusy ('xabort' is used
>> to finish the transaction).
>>
>> So, in summing up: (a) inflated lock cases are not being tested correctly
>> (since UseRTMForStackLocks is always enabled) and (b) stack lock cases are
>> failing because UseRTMForStackLocks is always disabled.
>>
>> @Igor, it's a long time ago, but maybe you recall some details on that
>> test... :-)
>>
>>
>> Thank you.
>>
>> Best regards,
>> Gustavo
>>
>