RFR 8251118: BiasedLocking::preserve_marks should not have a HandleMark
Patricio Chilano
patricio.chilano.mateo at oracle.com
Tue Aug 11 20:11:21 UTC 2020
Hi Harold,
Thanks for looking at this.
On 8/11/20 4:11 PM, Harold Seigel wrote:
> Hi Patricio,
>
> The fix looks good. Could you also add a test based on the failure
> discussed in the bug description?
Sure. From the mach5 job I run without the patch I see that
test/hotspot/jtreg/gc/TestFullGCALot.java fails pretty much consistently
on all platforms and tiers 1-3. What do you think about adding a new run
on that test with -XX:+UseBiasedLocking? Change would be:
diff --git a/test/hotspot/jtreg/gc/TestFullGCALot.java
b/test/hotspot/jtreg/gc/TestFullGCALot.java
--- a/test/hotspot/jtreg/gc/TestFullGCALot.java
+++ b/test/hotspot/jtreg/gc/TestFullGCALot.java
@@ -26,8 +26,9 @@
/*
* @test TestFullGCALot
- * @bug 4187687 8187819
+ * @bug 4187687 8187819 8251118
* @summary Ensure no access violation when using FullGCALot
* @requires vm.debug
* @run main/othervm -XX:NewSize=10m -XX:+FullGCALot
-XX:FullGCALotInterval=120 gc.TestFullGCALot
+ * @run main/othervm -XX:NewSize=10m -XX:+FullGCALot
-XX:FullGCALotInterval=120 -XX:+UseBiasedLocking gc.TestFullGCALot
*/
I would suggest adding a new run to FieldMonitor.java with
-XX:+UseBiasedLocking instead, but it doesn't fail as consistently.
Alternatively a new test would be pretty much like TestFullGCALot.java.
The only change I would make is to explicitly add a synchronized statement:
/*
* @test TestFullGCOnSync
* @bug 8251118
* @run main/othervm -XX:NewSize=10m -XX:+FullGCALot
-XX:FullGCALotInterval=120 -XX:+UseBiasedLocking TestFullGCOnSync
*/
public class TestFullGCOnSync {
public static void main(String args[]) throws Exception {
Integer x = new Integer(10);
synchronized (x) {
System.gc();
System.out.println("Hello world!");
}
}
}
Thanks,
Patricio
> Thanks, Harold
>
> On 8/11/2020 2:11 PM, Patricio Chilano wrote:
>> Hi all,
>>
>> Please review the following small fix which involves removing the
>> added HandleMark in BiasedLocking::preserve_marks():
>>
>> Bug: https://bugs.openjdk.java.net/browse/JDK-8251118
>> Webrev: http://cr.openjdk.java.net/~pchilanomate/8251118/v1/webrev/
>>
>> I've inspected the callers of BiasedLocking::preserve_marks() and
>> they all have an assert that the current thread is the VMThread.
>> Since the VMThread creates a HandleMark object before executing a VM
>> operation the extra HandleMark added in 8249192 is not needed.
>> I've run tiers1-3 in mach5 with -XX:+UseBiasedLocking and without the
>> patch I get several crashes in BiasedLocking::restore_marks(). With
>> the patch tests completed successfully.
>>
>> Thanks,
>> Patricio
>>
More information about the hotspot-runtime-dev
mailing list