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

[Gustavo Romero]

Hi Igor,

On 06/20/2018 03:23 PM, Igor Ignatyev wrote:
> as your patch has been reviewed by two reviewers, it is fine to be pushed.

Thanks for reviewing it.

Goetz, could you please sponsor the change?


Regards,
Gustavo
  
> -- 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
>>
>