RFR(L): 8196989: Revamp G1 JMX MemoryPoolMXBean, GarbageCollectorMXBean, and jstat counter definitions

[Hohensee, Paul]

At JVMLS, so can't look in depth this instant, but I'm fine with your approach, except I'd get the new JMX and jstat structure in place before fixing the data that gets reported. Imo it'll be easier to fit correct data into the new JMX/jstat setup than into the old one, and doing it the new way will give us a good idea of exactly what we should do for the legacy ones.

Your archive region set webrev looks pretty much the same as what I wrote, but I got a trace trap when I tried to execute the resulting JVM. Not a clue why, so I abandoned it.

I'd not have thought of making a G1MonitoringScope, looks good.

Thanks,

Paul

On 7/30/18, 6:04 AM, "Thomas Schatzl" <thomas.schatzl at oracle.com> wrote:

    Hi Paul,
    
      did some prototyping and wanted to show you the results and get your
    input:
    
    On Thu, 2018-07-26 at 16:06 +0200, Thomas Schatzl wrote:
    > 
    [...]
    > Could we work together on first refactoring the code before adding
    > new
    > kinds of spaces to the MXBeans?
    > 
    > Looking at this change and mine roughly the following issues would
    > need to be resolved first:
    > - find a solution for archive regions as suggested above :) At the
    > moment, without doing the change, I would tend to make archive
    > regions separate from old regions.
    
    I went with that and I am currently testing https://bugs.openjdk.java.n
    et/browse/JDK-8208498 ; here's a webrev to look at: http://cr.openjdk.j
    ava.net/~tschatzl/8208498/webrev/
    
    > - move serviceability stuff as much as possible to
    > g1MonitoringSupport
    
    Preliminary webrev:
    http://cr.openjdk.java.net/~tschatzl/move-serviceability-stuff/webrev/
    
    I think this came out better than expected: while we maybe want to add
    a ServiceabilitySupport interface that collects the
    get_memory_manager/pools/* methods in the future, imho this is a lot
    better than current code as it tightens the G1MonitoringSupport
    interface quite a bit.
    
    Particularly of note should be the G1MonitoringScope class that
    collects both TraceCollectorStats and TraceMemoryManagerStats into a
    single class. (Instead of the two bools passed to it something
    indicating the GC directly would probably be better too).
    
    It would be nice if something similar could be made for the concurrent
    Trace*Stats.
    
    > - clean up MemoryPool, remove duplicate information
    > - provide and return sane memory pool used/committed values to the
    > MXBeans
    > - clean up G1MonitoringSupport, e.g. avoid "*used/*committed"
    > variables
    > for every single memory pool. Use MemoryUsage structs for them. Make
    > reading of memory pool information atomic wrt to its readers (note
    > that I think it is currently just impossible to get consistent output
    > for other statistics like jstat) - that's JDK-8207200.
    > - add whatever serviceability stuff for the new pools/jstat/* in
    > steps.
    
    
    Thanks,
      Thomas