A couple nits on
http://cr.openjdk.java.net/~tschatzl/move-serviceability-stuff/webrev/.
g1CollectedHeap.cpp: in initialize_serviceability(), memory_managers(), and
memory_pools(), use g1mm() instead of _g1mm.
g1MonitoringSupport.cpp: there's an extra newline after ~G1MonitoringSupport().
Otherwise looks good.
Paul
On 7/30/18, 12:18 PM, "Hohensee, Paul" <hohen...@amazon.com> wrote:
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.scha...@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
