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
