+hotspot-gc-use.

I’ve filed a CSR for the current patch, see 
https://bugs.openjdk.java.net/browse/JDK-8196719. Here’s the argument in favor.


It’s possible that there are JDK8 users that rely on current G1 old gen 
CollectionUsage behavior, but imo it’s unlikely because it’s of little use. 
Perhaps Kirk and others with operational experience can weigh in.

Let’s think about use cases. G1 full GC’s happen rarely and only under severe 
pressure, so when they do external reaction is pretty much limited to reducing 
load so the JVM can get back to a usable steady state, or just restarting the 
JVM. Old gen CollectionUsage is zero until a full GC occurs, after which its 
value includes both long-lived objects and any transient data that was in eden 
and the survivor space. That value doesn’t tell you anything about long term 
old gen occupancy or survivor size because it lumps them all together. So, it 
isn’t a useful metric, nor will it be after any subsequent full GCs. The only 
information it provides is on the first zero to non-zero transition, which just 
tells you that the JVM is/was in trouble. Further, the effect of the runup to a 
full GC is SLA violations, which are noticed before the full GC happens, so 
detecting the first full GC is confirmation, not prediction.

Conclusion: G1 old gen CollectionUsage is unlikely to be in use in its current 
form, so changing its definition to something usable is low risk.


“G1 Old Space” is fine, as is “G1 Archive Space”. Are you ok with the G1 
archive space overlapping the G1 old space? Should we add an archive space to 
the other collectors? If so, how would it be defined and would having it 
overlap with the old generation as a live prefix be ok?

"G1 Young Generation" is the currently young+mixed collector.

You’re right, if one is iterating over all collectors, there will be redundancy 
if we keep the old ones. I’m usually leery of introducing a flag such as 
UseG1LegacyMXBeans (I changed the name, since all the interfaces are MXBeans, 
hope that’s ok) which must be either indefinitely maintained, or go through a 
deprecation cycle. I don’t see a way out of the ‘iterate over all collectors’ 
problem without it though.

Paul

On 1/31/18, 3:42 AM, "Erik Helin" <erik.he...@oracle.com> wrote:

    On 01/31/2018 02:30 AM, Hohensee, Paul wrote:
    > It’s true that my patch doesn’t completely solve the larger problem, but 
it fixes the most immediately important part of it, particularly for JDK8 where 
current expected behavior is entrenched.
    
    Yes, your patch fixes part of the problem, but as I said, can 
    potentially lead to more confusion. I'm not sure that doing this 
    behavioral change for a public API in an JDK 8 update release is the 
    right thing. There are likely users that rely on the memory pool "G1 Old 
    Gen" only being updated by a full collection (even though that behavior 
    is not correct), those uses will encounter a new behavior in an update 
    release with your patch.
    
    The good thing is that we have very experienced engineers participating 
    in the CSR process that have much more experience than I have in 
    evaluating the impact of behavioral changes such as this one. Would you 
    please file a CSR request for your patch to get their opinion?
    
    See https://wiki.openjdk.java.net/display/csr/Main for more details 
    about CSR.
    
    On 01/31/2018 02:30 AM, Hohensee, Paul wrote:
    If we’re going to fix the larger problem, imo we should file another 
    bug/rfe to do it. I’d be happy to fix that one too, once we have a spec.
    > 
    > What do you think of my suggestions? To summarize:
    > 
    > - Keep the existing memory pools and add humongous and archive pools.
    > - Make the archive pool part of the old gen, and generalize it to all 
collectors.
    > - Split humongous regions off from the old gen pool into their own pool. 
The old gen and humongous pools are disjoint region sets.
    > - Keep the existing “G1 Young Generation” and “G1 Old Generation” 
collectors and their associated memory pools (net of this patch). We add the 
humongous pool to them.
    > - Add “G1 Full” as an alias of the existing “G1 Old Generation” collector.
    > - Add the “G1 Young”, “G1 Mixed” and “G1 Concurrent Cycle” collectors.
    > - Set the G1 old gen memory pool max size to –Xmx, the archive space max 
size to whatever it is, and the rest of the G1 memory pool max sizes to -1 == 
undefined, as now.
    > 
    > The resulting memory pools:
    > 
    > “G1 Eden Space”
    > “G1 Survivor Space”
    > “G1 Old Gen”
    > “G1 Humongous Space”
    > “Archive Space”
    
    The "Space" suffix is unfortunate, but acceptable. I'm least happy about 
    the "Gen" suffix for the "G1 Old Gen", since G1's old regions differ 
    from a generation in the traditional sense as applied to e.g. Serial, 
    Parallel and CMS. I would be more happy to use a consistent naming 
    scheme in the form of "G1 Old Space" (having only one pool ending "Gen" 
    begs the question how it differs from the others ending in "Space"). 
    Again, we could introduce a flag such as -XX:+UseG1LegacyPoolsAndBeans 
    for those that really wants the old names.
    
    "Archive Space" should be named "G1 Archive Space" since it differs in 
    implementation from the other collectors. It would be unfortunate if 
    users thought they could change collector and the "Archive Space" memory 
    pool would keep the same behavior.
    
    > The resulting collectors and their memory pools:
    > 
    > “G1 Young Generation” (the existing young/mixed collector), “G1 Old 
Generation”/”G1 Full”, “G1 Mixed”
    > - “G1 Eden Space”
    > - “G1 Survivor Space”
    > - “G1 Old Gen”
    > - “G1 Humongous Space”
    > “G1 Young”
    > - “G1 Eden Space”
    > - “G1 Survivor Space”
    > - “G1 Humongous Space”
    > “G1 Concurrent Cycle”
    > - “G1 Old Gen”
    > - “G1 Humongous Space”
    > 
    > I’m not religious about the names, but I like my suggestions. :)
    I think it will be confusing for users to have both "G1 Old Generation" 
    and "G1 Full", particularly for tools iterating over all 
    GarbageCollectorMXBeans. There is no way to indicate that a 
    GarbageCollectorMXBeans is an alias of another GarbageCollectorMXBean (I 
    thought about such a solution as well).
    
    I guess I don't follow what the GarbageCollectorMXBean "G1 Young 
    Generation" is meant to represent?
    
    > The significant addition to my previous email, and an incompatible 
change, is splitting humongous regions off from the old gen pool. This means 
that apps that specifically monitor old gen occupancy will no longer see 
humongous regions. Monitoring apps that just add up info about all a 
collector’s pools won’t see a difference. I may be corrected (by Kirk, 
perhaps), but imo it’s not as bad a compatibility issue as one might think, 
because the type of app that uses a lot of humongous regions isn’t all that 
common. E.g., apps that cache data in the heap in the form of large compressed 
arrays, and apps with large hashmap bucket list arrays. The heaps such apps use 
are very often large enough to use 32mb regions, hence need really big objects 
to actually allocate humongous regions.
    
    So why not enable backwards compatibility by allowing a user to set the 
    flag -XX:+UseG1LegacyPoolsAndBeans? It is not that cumbersome for us to 
    maintain the current definition of memory pools and collectors. Such a 
    flag allows us to start over and do this right and a user who relies on 
    the current behavior can get that by just setting a flag. Doing such a 
    change in a major release also allows us to clearly highlight the change 
    in the release notes (users are more prepared for larger changes in a 
    major release and that they might have to add flags to keep old behavior).
    
    It is not uncommon for memory pools to change in major releases. The 
    perm gen pool was removed in JDK 8, the default pools changed when 
    Parallel Old became default old collector way back in JDK 7 and changed 
    again when G1 became the default collector in JDK 9.
    
    Thanks,
    Erik
    
    > Thanks,
    > 
    > Paul
    > 
    > On 1/30/18, 5:51 AM, "Erik Helin" <erik.he...@oracle.com> wrote:
    > 
    >      On 01/30/2018 03:07 AM, Hohensee, Paul wrote:
    >      > That’s one reviewer who’s ok with a short term patch. Anyone else? 
And,
    >      > any reviewers for said short term patch? :)
    >      
    >      Well, the patch is not really complete as it is. The problem is the
    >      definitions of the MemoryPoolMXBeans and GarbageCollectorMXBeans, 
which,
    >      as I tried to hint at in my first email, is a mess for G1. The names 
and
    >      implementations of these MemoryPoolMXBeans and 
GarbageCollectionMXBeans
    >      for G1 are very old, G1 has changed a lot since those were 
implemented
    >      (hence my suggestion for finally fixing this).
    >      
    >      The issue with your patch is that the MemoryPoolMXBean named "G1 Old
    >      Gen" consists of both old and humongous regions (it will also include
    >      archive regions). Old regions can be collected by mixed, concurrent 
and
    >      full collections. Humongous regions can be collected by young, mixed 
or
    >      full collections and the concurrent cycle. Archive regions will 
never be
    >      collected. Your patch will update the pool in the case of a mixed
    >      collection collecting old regions or humongous regions, but misses 
the
    >      following cases:
    >      - a young collection collecting humongous regions
    >      - a concurrent cycle collecting humongous regions
    >      - a concurrent cycle collecting old regions
    >      
    >      Unfortunately I could not come up with a good way to solve the above
    >      without re-designing the pools. I'm not sure about accepting your 
patch
    >      as is, since it might cause even more confusion for users compared to
    >      the current (already confusing) situation.
    >      
    >      One idea we have discussed is to implement the re-design but also 
add a
    >      flag, -XX:+UseG1LegacyPoolsAndBeans (false by default), to allow for 
a
    >      smoother transition. Would that solution work for you?
    >      
    >      Thanks,
    >      Erik
    >      
    >      > Thanks,
    >      >
    >      > Paul
    >      >
    >      > *From: *mandy chung <mandy.ch...@oracle.com>
    >      > *Organization: *Oracle Corporation
    >      > *Date: *Monday, January 29, 2018 at 1:41 PM
    >      > *To: *"Hohensee, Paul" <hohen...@amazon.com>
    >      > *Cc: *"serviceability-dev@openjdk.java.net"
    >      > <serviceability-dev@openjdk.java.net>, 
"hotspot-gc-...@openjdk.java.net"
    >      > <hotspot-gc-...@openjdk.java.net>
    >      > *Subject: *Re: RFR (S): 8195115: G1 Old Gen MemoryPool
    >      > CollectionUsage.used values don't reflect mixed GC results
    >      >
    >      > I created  JDK-8196362 to look into whether it makes sense to 
provide
    >      > some categorization to differentiate eden space vs the heap space 
for
    >      > long-lived objects.
    >      >
    >      > W.r.t. to JDK-8195115, I have to defer to GC team to comment on the
    >      > mixed collection update.  If they are okay, I have no objection to
    >      > implement a short-term fix and do the proper G1 memory pools as a
    >      > separate patch.
    >      >
    >      > Mandy
    >      >
    >      > On 1/29/18 1:02 PM, Hohensee, Paul wrote:
    >      >
    >      >     We don’t use getType, and you guessed correctly: we use the 
memory
    >      >     pool name as an indicator of the specific characteristics of a
    >      >     memory pool, in particular eden.
    >      >
    >      >     What we want is an indication of long term heap occupancy. We
    >      >     calculate it using CollectionUsage for non-eden heap memory 
pools,
    >      >     regardless of collector. We don’t use JMX notification, rather 
we
    >      >     periodically poll CollectionUsage for memory pools with names 
that
    >      >     contain “Old”, “Tenured”, or “Survivor”. We get the memory 
pools
    >      >     from the GarbageCollectorMXBeans (we don’t care what the 
collector
    >      >     names are). For the named memory pools, we sum 
CollectionUsage.used
    >      >     and divide by the sum of CollectionUsage.max to get a long 
term heap
    >      >     occupancy percentage. We don’t want to include eden because 
it’s
    >      >     really just an allocation buffer and not part of the storage 
for
    >      >     long-lived objects. I suppose we could use a negative test 
instead
    >      >     by using all memory pools with names that don’t include “Eden”.
    >      >
    >      >     The bug is that the “G1 Old Gen” memory pool isn’t being 
updated
    >      >     when the “G1 Young Generation” collector runs a mixed 
collection. As
    >      >     far as JMX is concerned, that collector only knows about eden 
and
    >      >     the survivor space. The patch adds the old gen to the memory 
pools
    >      >     it knows about and has mixed collections update the old gen’s
    >      >     CollectionUsage.
    >      >
    >      >     I managed to get a submit repo run to succeed last week and it 
found
    >      >     a problem. I’ve uploaded a new webrev that fixes the failure 
of the
    >      >     jtreg test TestMemoryMXBeansAndPoolsPresence.java due to the 
young
    >      >     gen collector being expected to know only about eden and the
    >      >     survivor space.
    >      >
    >      >     http://cr.openjdk.java.net/~phh/8195115/webrev.hs.01/
    >      >     <http://cr.openjdk.java.net/%7Ephh/8195115/webrev.hs.01/>
    >      >
    >      >     Waiting on the submit repo to come back with a result on it.
    >      >
    >      >     Thanks,
    >      >
    >      >     Paul
    >      >
    >      >     *From: *mandy chung <mandy.ch...@oracle.com>
    >      >     <mailto:mandy.ch...@oracle.com>
    >      >     *Organization: *Oracle Corporation
    >      >     *Date: *Monday, January 29, 2018 at 10:52 AM
    >      >     *To: *"Hohensee, Paul" <hohen...@amazon.com>
    >      >     <mailto:hohen...@amazon.com>, Erik Helin 
<erik.he...@oracle.com>
    >      >     <mailto:erik.he...@oracle.com>, David Holmes
    >      >     <david.hol...@oracle.com> <mailto:david.hol...@oracle.com>
    >      >     *Cc: *"serviceability-dev@openjdk.java.net"
    >      >     <mailto:serviceability-dev@openjdk.java.net>
    >      >     <serviceability-dev@openjdk.java.net>
    >      >     <mailto:serviceability-dev@openjdk.java.net>,
    >      >     "hotspot-gc-...@openjdk.java.net"
    >      >     <mailto:hotspot-gc-...@openjdk.java.net>
    >      >     <hotspot-gc-...@openjdk.java.net>
    >      >     <mailto:hotspot-gc-...@openjdk.java.net>
    >      >     *Subject: *Re: RFR (S): 8195115: G1 Old Gen MemoryPool
    >      >     CollectionUsage.used values don't reflect mixed GC results
    >      >
    >      >     On 1/29/18 10:35 AM, mandy chung wrote:
    >      >
    >      >         Thanks for the reply Paul.   Try to understand a little 
more on
    >      >         the specific from gc-specific memory pool you depend on.
    >      >
    >      >         On 1/29/18 8:27 AM, Hohensee, Paul wrote:
    >      >
    >      >             A name change would affect Amazon’s heap monitoring, 
and
    >      >             thus I expect it would affect other users as well.
    >      >
    >      >             As long as there are gc-specific memory pools, we’re 
going
    >      >             to need to be able to identify them, and right now 
that’s
    >      >             done via name.
    >      >
    >      >
    >      >         MemoryPoolMXBean::getType returns "heap" memory type for
    >      >         GC-specific memory pools.  Are you using this method?  Do 
you
    >      >         use the name to build in specific characteristic of a 
memory
    >      >         pool (e.g. eden vs old gen)?
    >      >
    >      >
    >      >
    >      >
    >      >             All the mxbeans are identified by name, so that’s a 
general
    >      >             design principle. The only way I can think of to get 
rid of
    >      >             name dependency would be to figure out what abstract 
metrics
    >      >             users want to monitor and implement them for all 
collectors.
    >      >             HeapUsage (instantaneous occupancy) is one, 
CollectionUsage
    >      >             (long-lived occupancy) is another, both of these for 
the
    >      >             entire heap, not just particular memory pools.
    >      >
    >      >
    >      >         The sum of HeapUsage and CollectionUsage of all heap memory
    >      >         pools was expected to give an incorrect approximation for 
the
    >      >         entire heap usage.  Are you seeing issue/bug with the sum 
result?
    >      >
    >      >
    >      >     typo: s/an incorrect approximation/an approximation.
    >      >
    >      >     Mandy
    >      >
    >      >
    >      >
    >      >         Mandy
    >      >
    >      >
    >      >
    >      >             That said, imo there will always be a demand for the 
ability
    >      >             to get collector and memory pool specific details, so I
    >      >             don’t see a way to get around providing named entities.
    >      >
    >      >             Paul
    >      >
    >      >             *From: *mandy chung <mandy.ch...@oracle.com>
    >      >             <mailto:mandy.ch...@oracle.com>
    >      >             *Organization: *Oracle Corporation
    >      >             *Date: *Friday, January 26, 2018 at 2:38 PM
    >      >             *To: *"Hohensee, Paul" <hohen...@amazon.com>
    >      >             <mailto:hohen...@amazon.com>, Erik Helin
    >      >             <erik.he...@oracle.com> <mailto:erik.he...@oracle.com>,
    >      >             David Holmes <david.hol...@oracle.com>
    >      >             <mailto:david.hol...@oracle.com>
    >      >             *Cc: *"serviceability-dev@openjdk.java.net"
    >      >             <mailto:serviceability-dev@openjdk.java.net>
    >      >             <serviceability-dev@openjdk.java.net>
    >      >             <mailto:serviceability-dev@openjdk.java.net>,
    >      >             "hotspot-gc-...@openjdk.java.net"
    >      >             <mailto:hotspot-gc-...@openjdk.java.net>
    >      >             <hotspot-gc-...@openjdk.java.net>
    >      >             <mailto:hotspot-gc-...@openjdk.java.net>
    >      >             *Subject: *Re: RFR (S): 8195115: G1 Old Gen MemoryPool
    >      >             CollectionUsage.used values don't reflect mixed GC 
results
    >      >
    >      >             On 1/25/18 1:04 PM, Hohensee, Paul wrote:
    >      >
    >      >
    >      >              > The JMX API spec doesn’t specify what the memory 
pool or
    >      >             garbage > collector names are, but the current names 
are
    >      >             de-facto part of the > API, so if we change the 
existing
    >      >             ones, imo a CSR should be filed.
    >      >
    >      >             The names are implementation details but I can see how 
an application
    >      >
    >      >             might be impacted if they depend on it.  CSR approval 
is not strictly
    >      >
    >      >             necessary while I think filing one to document the 
change would be
    >      >
    >      >             good.
    >      >
    >      >             Does the name change impact any application you know 
of?  I'm trying to
    >      >
    >      >             understand if any improvement to API is needed so that 
applications
    >      >
    >      >             don't need to depend on the names.
    >      >
    >      >
    >      >             Mandy
    >      >
    >      >
    >      >
    >      >
    >      >
    >      >
    >      >
    >      >
    >      
    > 
    

Reply via email to