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

Stefan Johansson stefan.johansson at oracle.com
Thu Mar 27 14:40:29 UTC 2014

I still need a review from someone with reviewer status. Would be good 
if someone from the runtime team who feels comfortable reviewing the 
small CheckUnhandledOops changes could have a look.


On 2014-03-06 19:35, 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/
> 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