RFR 8251118: BiasedLocking::preserve_marks should not have a HandleMark
Harold Seigel
harold.seigel at oracle.com
Tue Aug 11 20:29:07 UTC 2020
Hi Patricio,
I think your modifications of TestFullGCALot.java are fine for testing
your fix. No need for a brand new test.
Thanks, Harold
On 8/11/2020 4:11 PM, Patricio Chilano wrote:
> 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