RFR 8085965: VM hangs in C2Compiler

[Poonam Bajaj Parhar]

Hello Jon,

On 6/16/2015 10:59 AM, Jon Masamitsu wrote:
> Poonam,
>
> With your changes try
>
> java -XX:+ExplicitGCInvokesConcurrentAndUnloadsClasses 
> -XX:-ClassUnloading -XX:+UseConcMarkSweepGC  -XX:+PrintFlagsFinal 
> -version
>
> and check if ExplicitGCInvokesConcurrent is true or false.
>
> If it is true, I don't know if that is the right thing to do.  In this 
> case on the
> command line ExplicitGCInvokesConcurrentAndUnloadsClasses has been
> set to true and ClassUnloading has been set to false.  As a side effect,
> ExplicitGCInvokesConcurrent is set to true.  Whereas 
> ExplicitGCInvokesConcurrentAndUnloadsClasses
> has been set back to false (probably) That may be a surprise to the
> users.
>
For CMS, ExplicitGCInvokesConcurrent won't get set to true if 
ClassUnloading is false. In the argument parsing we would set 
ExplicitGCInvokesConcurrentAndUnloadsClasses to false and while 
constructing CMSCollector, we set ExplicitGCInvokesConcurrent.

   if (ExplicitGCInvokesConcurrentAndUnloadsClasses) {
     ExplicitGCInvokesConcurrent = true;
   }

I am testing it to confirm.

Thanks,
Poonam

> Jon
>
> On 06/15/2015 11:36 AM, Poonam Bajaj Parhar wrote:
>> Hello,
>>
>> Here's the updated webrev:
>> http://cr.openjdk.java.net/~poonam/8085965/webrev.03/
>> - Set ExplicitGCInvokesConcurrentAndUnloadsClasses too to false when 
>> ClassUnloading is turned off.
>> - removed the changes from unpdate_should_unload_classes() in 
>> concurrentMarkSweepGeneration.cpp
>> - Added the change to mark the classes in genMarkSweep.cpp when 
>> ClassUnloading is off to make sure that classes don't get unloaded 
>> with -XX:-ClassUnloading.
>>
>> Thanks,
>> Poonam
>>
>> On 6/15/2015 7:04 AM, Poonam Bajaj Parhar wrote:
>>> 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.
>>>>
>>>>
>>>
>>
>