RFR: 8028498: runtime/SharedArchiveFile/CdsDifferentObjectAlignment.java asserts in RT_Baseline

Stefan Johansson stefan.johansson at oracle.com
Thu Mar 20 12:47:30 UTC 2014

Thanks Per for reviewing.

I still need a review from someone with reviewer status, and preferably 
someone who feels comfortable reviewing the CheckUnhandledOops changes.


On 2014-03-17 12:32, Per Liden wrote:
> Hi Stefan,
> On 03/06/2014 07:35 PM, Stefan Johansson wrote:
>> As Per noticed the _gc_locked_out_count isn't updated anymore when
>> removing the GC locker lock/unlock methods. I've looked through the code
>> to see what would be the proper action to take in
>> clear_unhandled_oops(). The first observation was that the check for
>> is_gc_locked_out() only had any effect during a short period of time
>> during startup (the only place we used GC locker lock/unlock). This
>> leads to the question, do we need to avoid clearing the unhandled oops
>> when the GC lockers other lock mechanism is used, and I've look this
>> through together with Per and it seems like it's not needed.
>> One can argue that if we've locked out the GC using the GC locker, it
>> should be fine to have unhandled oops, but since we don't make use of
>> that today I see no reason for enabling it. If I haven't missed anything
>> this should allow us to remove the check from clear_unhandled_oops() and
>> the _gc_locked_out_count with accessors all together.
>> Please let me know if you have any fears or thoughts around this. I've
>> run both JPRT tests and some bigapps testing with +CheckUnhandledOops
>> without any problem as sanity testing.
>> Regarding the message I updated it to use the methods suggested by Per.
>> Here's the updated webrev:
>> http://cr.openjdk.java.net/~sjohanss/8028498/webrev.03/
> Looks good to me.
> /Per
>> Thanks,
>> Stefan
>> On 2014-02-17 10:58, Per Liden wrote:
>>> Stefan,
>>> I think this is a nice fix for the problem and it also cleans up the
>>> gclocker a bit. But I have a few minor comments:
>>> vmGCOperations.cpp:
>>>  110       err_msg("GC triggered before VM initialization completed.
>>> Try increasing "
>>>  111               "NewSize, current value " UINTX_FORMAT "K.",
>>> NewSize / K));
>>> I'd suggest you use
>>> byte_size_in_proper_unit()/proper_unit_for_byte_size() instead of a
>>> "K", or maybe just not print the size at all.
>>> With the removal of GC_locker::lock()/unlock() there's no longer
>>> anyone how modifies Thread::current()->_gc_locked_out_count. So we
>>> should be able to remove it. However,
>>> UnhandledOops::clear_unhandled_oops() still uses is_gc_locked_out() so
>>> that needs to be changed to check for something else,
>>> is_init_completed maybe?
>>> cheers,
>>> /Per
>>> On 02/12/2014 06:12 PM, Stefan Johansson wrote:
>>>> Hi,
>>>> Stefan K pointed out to me that in the Metaspace code we use
>>>> is_init_completed() to verify that the VM is fully initialized. I
>>>> started looking at where we do set_init_completed() and realized 
>>>> that it
>>>> is right where we are ready to handle a GC. It makes sense to try to
>>>> minimize the different ways we check if the VM is fully initialized.
>>>> Using is_init_completed() instead of the GC locker to prevent GCs 
>>>> during
>>>> startup has another nice effect too; we can remove one of the two
>>>> different lock mechanisms in the GC locker.
>>>> The new proposal removes the use of GC_locker::lock()/unlock() 
>>>> which is
>>>> only used to prevent GCs during startup. Instead there is a check 
>>>> in the
>>>> GC operations prologue, to ensure that the VM is initialized when 
>>>> the GC
>>>> starts.
>>>> Please review the new the change:
>>>> http://cr.openjdk.java.net/~sjohanss/8028498/webrev.02/
>>>> Testing:
>>>> * JPRT
>>>> * GC locker test through UTE
>>>> Thanks,
>>>> Stefan
>>>> On 2014-02-11 16:12, Stefan Johansson wrote:
>>>>> Hi again =)
>>>>> Don't spend time on the current webrev (02). I've gotten some more
>>>>> comments on the proposed fix and will look at a solution that 
>>>>> allow us
>>>>> to remove the GC locker usage during initialization.
>>>>> Cheers,
>>>>> Stefan
>>>>> On 2014-02-10 18:51, Stefan Johansson wrote:
>>>>>> Hi again,
>>>>>> After a couple of offline discussions with different people I've 
>>>>>> come
>>>>>> to the conclusion to split out the sizing of the young generation
>>>>>> from the fix for this issue. I've created a new bug (JDK-8033426) 
>>>>>> for
>>>>>> that, which is currently out for review.
>>>>>> The discussions also lead to a different approach for fixing this
>>>>>> issue. The new proposal is to use the GC locker to prevent GCs until
>>>>>> we are ready to handle them and otherwise fail the VM 
>>>>>> initialization.
>>>>>> The gc locker has two different mechanisms for locking out the GC,
>>>>>> one which is used during initialization and one used to stall GCs
>>>>>> when doing critical JNI work. The part of the initialization that
>>>>>> currently is locked by the GC locker is to narrow and there is no
>>>>>> guarantee that a GC can be handled after the GC locker is unlocked.
>>>>>> This fix will extend the part that is locked and also add a check
>>>>>> that if a GC is initiated while locked during startup the VM exits.
>>>>>> New webrev:
>>>>>> http://cr.openjdk.java.net/~sjohanss/8028498/webrev.01/
>>>>>> Thanks,
>>>>>> Stefan
>>>>>> On 2014-01-29 14:54, Stefan Johansson wrote:
>>>>>>> Hi,
>>>>>>> Please review this fix for:
>>>>>>> https://bugs.openjdk.java.net/browse/JDK-8028498
>>>>>>> Webrev:
>>>>>>> http://cr.openjdk.java.net/~sjohanss/8028498/webrev.00/
>>>>>>> Summary:
>>>>>>> The initial young generation size has been fairly small by default
>>>>>>> (1.5M) and if setting ObjectAlignmentInBytes to something larger
>>>>>>> than the default it can causes CDS to trigger a GC when dumping the
>>>>>>> archive. At this point the VM is not ready to handle a GC and we
>>>>>>> will hit an assertion. Making sure we can handle a GC at this point
>>>>>>> is not trivial and the proposed solution is to alter the default
>>>>>>> sizing of the young generation as well as adding a safety check 
>>>>>>> when
>>>>>>> dumping the archive to exit the VM if the young generation is too
>>>>>>> small.
>>>>>>> Testing:
>>>>>>> * JPRT build and test passes
>>>>>>> * Failing test now passes on all platforms (tested throught JPRT)
>>>>>>> * Verified that the GC and Runtime jtreg tests still passes
>>>>>>> Cheers,
>>>>>>> Stefan

More information about the hotspot-dev mailing list