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

[Per Liden]

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
>>>
>>
>