RFR 8085965: VM hangs in C2Compiler

[Poonam Bajaj Parhar]

Hi Jon,

On 6/16/2015 11:47 AM, Poonam Bajaj Parhar wrote:
> 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.
>
Here's the result:

java -XX:+ExplicitGCInvokesConcurrentAndUnloadsClasses -XX:-ClassUnloading -XX:+UseConcMarkSweepGC  -XX:+PrintFlagsFinal -version
[Global flags]
     uintx AdaptiveSizeDecrementScaleFactor          = 4                                   {product}
     ...
     bool CMSClassUnloadingEnabled                 := false                               {product}
     ...
     bool ClassUnloading                           := false                               {product}
     ...
     bool ExplicitGCInvokesConcurrent               = false                               {product}
     bool ExplicitGCInvokesConcurrentAndUnloadsClasses := false                               {product}



Thanks,
Poonam


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