RFR: 8048269: Add flag to turn off class unloading after G1 concurrent mark

[Bengt Rutisson]

On 2014-07-15 20:49, Jon Masamitsu wrote:
>
> On 07/14/2014 12:18 AM, Bengt Rutisson wrote:
>>
>> Jon,
>>
>> On 2014-07-11 19:17, Jon Masamitsu wrote:
>>> I think the reason for this flags is that it increases the pause times
>>> and some applications care more about pause times than about
>>> class bloat.
>>
>> In most of the benchmarks that we have run pause times decrease with 
>> class unloading enabled. Young GC times go down as they have to scan 
>> much fewer classes. So, I'm not sure it is as simple as choosing 
>> between class bloat (memory footprint) and pause times.
>
> OK.
>
>>
>> As far as I understand the main concern is that remark pauses can 
>> increase. For those applications that experience that as a problem we 
>> need the flag to turn class unloading off after concurrent cycles.
>
> Is there really a connection to a concurrent cycle?  I see that it is
> class unloading after a concurrent cycle but if that phase were
> done STW, does that change the concept of this flag?

I thought we wanted this flag because we were worried about the extra 
overhead that the new feature introduced. If we add a STW class 
unloading feature at some point we probably want to control that 
separately too.

>>
>>> How about a name like
>>>
>>> ReduceClassUnloading
>>
>> I think this is a too generic name. What if we want to do class 
>> unloading at some other point? What if we would for example try to 
>> unload classes after a normal young GC? Would this flag then turn off 
>> both that and the unloading after a concurrent cycle?
>
> If that situation arises, I would probably consider a new flag but I 
> would
> think about reusing UseReducedClassUnloading just to keep the number of
> flags down.   The description I would give for UseReducedClassUnloading
>
> UseReducedClassUnloading - For use with applications that do most
> class loading at startup, reduces the overhead of class unloading.

I'm not convinced that we should document a flag by saying in which 
situations it should be used. I prefer documenting what the flag 
actually does.

>
>>
>> My expectation is that the new flag to turn off class unloading after 
>> a concurrent cycle will be very rarely used. It is mostly something 
>> we can pull out of our sleeve if there are regressions reported due 
>> to class unloading. As such I prefer the flag to be explicit about 
>> what it is doing.
>
> I don't really know what to expect in terms of how often it is used.
> An application that does all its class loading at startup might
> consider anything afterwards with regard to class loading to be
> a waste of time.

My expectation is that normally everyone wants class unloading enabled 
for concurrent cycles and that the overhead for it is small enough for 
most applications. I'm guessing that we will only need the flag to work 
around specifically reported regressions in some odd benchmarks.

Bengt


>
> Jon
>
>>
>>
>>>
>>> or
>>>
>>> UseReducedClassUnloading
>>>
>>> and add
>>>
>>> ReducedClassUnloadingInterval
>>>
>>> where the interval is the number of GC's
>>> between class unloading.  Or don't add
>>> an interval.  We could add it later or
>>> something equivalent if some customer
>>> asks for it.
>>>
>>> The same flag would make sense for STW collectors
>>> that might want the same type of option.
>>
>> An interval may be a good idea, but is kind of a separate issue in my 
>> opinion. I prefer to do what you suggested - add it when it turns out 
>> that it is actually needed.
>>
>> Thanks,
>> Bengt
>>
>>>
>>> Jon
>>>
>>>
>>> On 7/11/2014 12:52 AM, Bengt Rutisson wrote:
>>>>
>>>> On 2014-07-11 07:44, Stefan Karlsson wrote:
>>>>> On 2014-07-11 01:11, Jon Masamitsu wrote:
>>>>>> Could we use a more GC generic name for the flag such as
>>>>>> GCClassUnloadingEnabled instead of a G1 specific name and
>>>>>> at some point move CMS to use the GC generic name?
>>>>>
>>>>> Yes, me and Bengt was talking about that, but IMHO we couldn't 
>>>>> come up with a flag with a reasonably short and to-the-point flag 
>>>>> name. I don't like the GCClassUnloadingEnabled since it could be 
>>>>> mistaken to mean turn of class unloading entirely, which isn't 
>>>>> what the flag is supposed to do.
>>>>>
>>>>> Some of the names we thought about:
>>>>>   -XX:+UseConcurrentClassUnloading
>>>>>     This sounds we're doing the class unloading concurrently, 
>>>>> which isn't the case.
>>>>>
>>>>>   -XX:+UseConcMarkClassUnloading
>>>>>
>>>>>   -XX:+UseClassUnloadingAfterConcMark
>>>>>
>>>>>   -XX:+ClassUnloadingAfterConcMark
>>>>>
>>>>> Any more suggestions?
>>>>
>>>> Talked some more to Stefan about this.
>>>>
>>>> The suggestion he made to call it -XX:+ClassUnloadingAfterConcMark 
>>>> is based on that the high level flag to control all class unloading 
>>>> is called -XX:+ClassUnloading. So, this name is kind of a 
>>>> specialization of the overall flag. I kind of like that (even if I 
>>>> think the overall flag should have been called 
>>>> .XX:+UseClassUnloading). However, ConcMark sounds very CMS-like for 
>>>> me. I think my preferred flag name for the moment is:
>>>>
>>>> -XX:+ClassUnloadingAfterConcurrentMark
>>>>
>>>> If we introduce a common flag for CMS and G1 I would suggest to 
>>>> deprecate CMSClassUnloadingEnable in JDK9 and have it only be an 
>>>> alias for the new flag. And then remove CMSClassUnloadingEnable in 
>>>> JDK10.
>>>>
>>>> Thanks,
>>>> Bengt
>>>>
>>>>>
>>>>> thanks,
>>>>> StefanK
>>>>>
>>>>>>
>>>>>> Jon
>>>>>>
>>>>>> On 07/10/2014 08:19 AM, Stefan Karlsson wrote:
>>>>>>> Hi all,
>>>>>>>
>>>>>>> Please, review this patch to add code and a flag to turn off 
>>>>>>> class unloading after G1 concurrent mark.
>>>>>>>
>>>>>>> http://cr.openjdk.java.net/~stefank/8048269/webrev.00/
>>>>>>> https://bugs.openjdk.java.net/browse/JDK-8048269
>>>>>>>
>>>>>>> thanks,
>>>>>>> StefanK
>>>>>>
>>>>>
>>>>
>>>
>>
>