RFR 8085965: VM hangs in C2Compiler

[Poonam Bajaj Parhar]

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