RFR(XS) 8213250 CDS archive creation aborts due to metaspace object allocation failure

[Ioi Lam]

Hi Thomas,

Yes, it was an oversight. I've added the following line:

ClassLoaderMetaspace::~ClassLoaderMetaspace() {
+ Metaspace::assert_not_frozen();
DEBUG_ONLY(Atomic::inc(&g_internal_statistics.num_metaspace_deaths));
   ...

Thanks

- Ioi



On 11/2/18 12:36 PM, Thomas Stüfe wrote:
> Hi Ioi,
>
> this not a Review, since I do not know the code well enough.
>
> But while looking at your change, I came across Metaspace::freeze()
> and saw that we do "assert_not_frozen()" on allocation paths:
> ClassMetaSpace::allocate and ::expand_and_allocate (redundant?) and
> the atypical deallocation path (::deallocate()). But not on the normal
> deallocation path when the CMS just dies because its class is unloaded
> (~ClassLoaderMetaspace()). Is this by design or an oversight?
>
> Best Regards, Thomas
>
>
> On Fri, Nov 2, 2018 at 5:47 AM Ioi Lam <ioi.lam at oracle.com> wrote:
>> Hi,
>>
>> I've revised the webrev to get to the the root of the problem -- we
>> shouldn't allocate metaspace objects from within the VM Thread!
>>
>> http://cr.openjdk.java.net/~iklam/jdk12/8213250-cds-dump-run-out-of-metaspace.v02/
>>
>> The original bug that caused the "!(DumpSharedSpaces &&
>> THREAD->is_VM_thread())" check to be added in Metaspace::allocate() was
>>
>>       https://bugs.openjdk.java.net/browse/JDK-8196626
>>       UseAppCDS.java crashes with "VM thread using lock Heap_lock (not
>> allowed to block on)"
>>
>> The fix in JDK-8196626 is just a band-aid solution -- avoiding the GC.
>> The proper fix is not to do any allocation inside the VM thread. So
>> instead of calling finalize_verification_constraints, which allocates
>> MetaspaceObjs, very late in the CDS dumping process, now we call be as
>> soon as a class has been verified.
>>
>> Thanks
>> - Ioi
>>
>>
>>
>>
>>
>> On 11/1/18 11:07 AM, Ioi Lam wrote:
>>> https://bugs.openjdk.java.net/browse/JDK-8213250
>>> http://cr.openjdk.java.net/~iklam/jdk12/8213250-cds-dump-run-out-of-metaspace.v01/
>>>
>>>
>>> When Metaspace::allocate() fails, usually we first try to GC to see if
>>> some MetaspaceObjs can be freed. This is an optimization to avoid
>>> prematurely
>>> expanding the metaspace.
>>>
>>> However, GC cannot run in the last phase of CDS which runs inside the
>>> VM thread.
>>> The old code just aborts the CDS dump. The fix is to unconditionally
>>> expand the
>>> metaspace without doing a GC.
>>>
>>> (During GC dump time, no classes are unloaded, so even if we force a
>>> GC, it would
>>> not free up any metaspace anyway).
>>>
>>> Testing:
>>>
>>> Before the fix, the test case (jruby) would fail to dump once a couple
>>> of times. Now
>>> it has dumped hundreds of times without failure.
>>>
>>> I am also running hs-tier 1/2
>>>
>>> Thanks
>>> - Ioi