RFR 8085965: VM hangs in C2Compiler

[Poonam Bajaj Parhar]

Hello Kim,

Thanks for the review!

On 6/13/2015 11:53 PM, Kim Barrett wrote:
> On Jun 11, 2015, at 3:34 PM, Jon Masamitsu <jon.masamitsu at oracle.com> wrote:
>> Poonam,
>>
>> Thanks for the changes.
>>
>> What do you think about deleting this part of the
>> change (line 2759) from the flag processing?
>> It feels like we're fixing something in two places where
>> as we only really need the change in
>> Arguments::set_cms_and_parnew_gc_flags().
>>
>> http://cr.openjdk.java.net/~poonam/8085965/webrev.01/src/share/vm/runtime/arguments.cpp.frames.html
>>
>>
>> 2759       FLAG_SET_CMDLINE(bool, CMSClassUnloadingEnabled, false);
> That seems wrong to me.
>
> It seems to me that the problem here is inconsistencies in the (final) CLA values.
> I think no change would be needed in concurrentMarkSweepGeneration.cpp if the CLA
> values were consistent.  By consistent here I mean:
Yes, the problem here is the inconsistent values of ClassUnloading and 
the concurrent-class-unloading options.

> If ClassUnloading is false, then all CLA variations on class unloading must also be false.
> There are at least:
>
> ClassUnloadingWithConcurrentMark
> CMSClassUnloadingEnabled
> ExplicitGCInvokesConcurrentAndUnloadsClasses
>
> I don’t know if there are more.
>
> At least some of those might be false when ClassUnloading is true.  And of course, all three of
> those should be treated as effectively false (and perhaps set false) when a non-concurrent collector
> is invoked.

I agree that that all the concurrent class unloading options should be 
set false when ClassUnloading is switched off. I will set 
ExplicitGCInvokesConcurrentAndUnloadsClasses too to false as part of 
this fix but would avoid changing the G1 specific option as I won't be 
able to verify the effects of that change for G1.

During the testing another issue was found where class unloading was 
still occurring during Full GC events when running with CMS and 
-XX:-ClassUnloading. And the reason for that was that currently we are 
not marking the classes during the roots processing(in 
mark_sweep_phase1()) even when ClassUnloading is off. The following 
change will fix that problem. The changes are currently under testing 
and I will resubmit the webrev after verification of this change.

   gch->gen_process_roots(level,
                          false, // Younger gens are not roots.
                          true,  // activate StrongRootsScope
                          SharedHeap::SO_None,
                          GenCollectedHeap::StrongRootsOnly,  <----
                          &follow_root_closure,
                          &follow_root_closure,
                          &follow_cld_closure);

to
   gch->gen_process_roots(level,
                          false, // Younger gens are not roots.
                          true,  // activate StrongRootsScope
                          SharedHeap::SO_None,
                          ClassUnloading,  <-----
                          &follow_root_closure,
                          &follow_root_closure,
                          &follow_cld_closure);

Thanks,
Poonam

>
> I think Gerard’s argument checking work will have some impact here.  I’d suggest that there should
> be constraint functions for these arguments that verify consistency at the end of argument processing.
>
>