RFR 8227528: TestAbortVMOnSafepointTimeout.java failed due to "RuntimeException: 'Safepoint sync time longer than' missing from stdout/stderr"
patricio.chilano.mateo at oracle.com
Fri Jul 26 19:46:37 UTC 2019
On 7/26/19 3:19 PM, Daniel D. Daugherty wrote:
> On 7/26/19 2:46 PM, Patricio Chilano wrote:
>> Hi all,
>> Could you review this small fix for test
>> The test has been failing intermittently since 8191890. As explained
>> in the bug comments, it turns out that a bias revocation handshake
>> could happen in between the start of the "for" loop without safepoint
>> polls and the safepoint where we want to timeout. That allows for the
>> long loop to actually finish and prevents the desired timeout in the
>> later safepoint. The simple solution is to just avoid using biased
>> locking in this test (and therefore prevent the revocation
>> handshake), since we just want to test the correct behavior of flag
>> Webrev: http://cr.openjdk.java.net/~pchilanomate/8227528/v01/webrev
> The change itself is trivial. However, the reasons behind the change
> This part of the description caught my eye:
> the start of the "for" loop without safepoint polls
> and my brain did a "Say what?!?!" Of course, that was without looking at
> the test which has a huge number of options, including these:
> L70: "-XX:-UseCountedLoopSafepoints",
> L71: "-XX:LoopStripMiningIter=0",
> L72: "-XX:LoopUnrollLimit=0",
> Okay, now the world makes much more sense. We are intentionally telling
> the compiler to not emit safepoint polls in the counted loop and we're
> turning off other loop optimizations. Basically, we're telling the
> compiler we want to stall in that loop until we exceed the safepoint
> timeout limit. Got it...
> So the new biased locking handshake messes with the timeout that this
> test is trying to achieve. Disabling biased locking makes the test more
> robust by allowing the safepoint sync timeout to happen.
> A couple of minor suggestions:
> L30: * @bug 8219584
> You should add an @bug for this bug (8227528). I don't know if
> you can put more than one bug ID on an @bug line or if you need
> a separate @bug line.
Done! I added the bug number in the same line since that's the style I
see in other tests.
> L61: ProcessBuilder pb = ProcessTools.createJavaProcessBuilder(
> Please add a comment above this line:
> // -XX:-UseBiasedLocking - is used to prevent biased locking
> // handshakes from changing the timing of this test.
> Thumbs up. I don't need to see another webrev if you choose to make
> the above changes.
Great! Thanks for reviewing this Dan! : )
>> Bugid: https://bugs.openjdk.java.net/browse/JDK-8227528
More information about the hotspot-runtime-dev