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

[Igor Ignatyev]

Hi Gustavo,

as your patch has been reviewed by two reviewers, it is fine to be pushed.

-- Igor

> On Jun 20, 2018, at 5:17 AM, Gustavo Romero <gromero at linux.vnet.ibm.com> wrote:
> 
> Hi Igor,
> 
> On 06/15/2018 10:12 AM, Lindenmaier, Goetz wrote:
>> I had a look at your fix. As I understand the test it's just
>> what was intended to do. Also, it fixed the test in our runs.
>> Reviewed.
>> I would volunteer to sponsor this.
> 
> Goetz reviewed the change. Is it OK to be pushed?
> 
> 
> Thanks.
> 
> Best regards,
> Gustavo
> 
>> Best regards,
>>   Goetz.
>>> -----Original Message-----
>>> From: Gustavo Romero [mailto:gromero at linux.vnet.ibm.com]
>>> Sent: Donnerstag, 31. Mai 2018 04:05
>>> To: hotspot-compiler-dev at openjdk.java.net; igor.ignatyev at oracle.com
>>> Cc: Lindenmaier, Goetz <goetz.lindenmaier at sap.com>; Doerr, Martin
>>> <martin.doerr at sap.com>
>>> Subject: RFR(XS): 8204135: jtreg: Fix failing RTM test
>>> TestUseRTMXendForLockBusy
>>> 
>>> 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
>