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

[Mikael Gerdin]

On Wednesday 16 July 2014 12.50.21 Bengt Rutisson wrote:
> 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.

There is already a flag which controls the existing STW class unloading (using 
the G1MarkSweep code) -XX:-ClassUnloading or -Xnoclassgc

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

I think sustaining wants it to help narrow down potential bugs as well.

/Mikael

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