RFR: 8013590: NPG: Add a memory pool MXBean for Metaspace
erik.helin at oracle.com
Thu Jun 13 08:07:08 UTC 2013
On 2013-06-12, Jon Masamitsu wrote:
> On 6/11/2013 8:27 AM, Erik Helin wrote:
> >Hi Mikael and Jon,
> >thanks for reviewing!
> >I have updated the webrev. The changes from webrev.00 are:
> >- Only exposing the compressed class space memory pool if compressed
> > classes are used.
> >- Renamed the name of the pool to "Compressed Class Space" (was
> > "Compressed Klass Space", note the 'K').
> There was a discussion about the way the flag
> UseCompressedKlassPointers was spelled.
> I had advocated for keeping the "K" in Klass but after the other
> responses I thought
> I had lost. In particular I had thought the flag had existed before
> the NPG changes and
> had been exposed to users and was wrong.
> Why did you decide to keep the flag (and internal variable names)
> with the "K" but
> change the name of the space to "Compressed Class Space"?
I'm sorry, I should be more explicit when I write my emails :(
Yes, we've decided to use 'C' instead of 'K' for all external names
regarding compressed klass space (and compressed klass pointers). I'm
going to send out a separate patch with these changes, but I should have
mentioned that in this email to avoid any confusion.
If you want, I can continue to keep 'K' for this change and then change
all of the naming in a separate patch (that would be more consistent).
Would you prefer to do it this way?
> >- Renamed the variable _cks_pool to _compressed_class_pool.
> >- Using max_uintx instead of _undefined_size.
> >- Updated the test and the rest of the code to take these new change
> > into account.
> >Please see new webrev at:
> Rest looks fine.
> >On 2013-06-04, Jon Masamitsu wrote:
> >>The functions assertEquals(), assertTrue(), and assertDefined() seem
> >>useful. Is there
> >>a more global place where they can be put so other can use them?
> >I agree that assertTrue and assertEquals should be move to a separate
> >file, but I think that assertDefined is mostly usable for tests dealing
> >with MXMemoryBeans (since defined is not always the same as "differ from
> >I would like to do these changes as a separate change. Jon, what do you
> >about that?
> >On 2013-05-31, Mikael Gerdin wrote:
> >>(merging the gc-dev and svc-dev threads)
> >>Hi Erik,
> >>On 2013-05-29 10:44, Erik Helin wrote:
> >>>Hi all,
> >>>this want sent to hotspot-gc-dev at openjdk.java.net, sending to
> >>>serviceability-dev at openjdk.java.net as well since the change is about
> >>>memory pools.
> >>>This change adds two memory pools for metaspace, one for Metaspace and
> >>>one for compressed klass space. The memory pool for compressed klass
> >>>space will only have valus that differ from 0 or -1 (undefined) if
> >>>compressed klass pointers are used.
> >>>Question: Should I use an empty pool when compressed klass pointers are
> >>>*not* used or should I just not expose the pool?
> >>>This change also adds a manager for the pools: Metaspace Manager.
> >>>I have also added a test that checks that the metaspace manager is
> >>>present and that the two pools are present. The test also verifies that
> >>>the compressed klass space pool act correct according to the
> >>>UseCompressedKlass flag.
> >>>The last time I added metaspace memory pools, it triggered some
> >>>unforeseen bugs:
> >>>- Two asserts in jmm_GetMemoryUsage that asserted that a memory pool was
> >>> either of heap type or had an undefined init/max size.
> >>>- The jdk/test/java/lang/management/MemoryMXBean/MemoryTest.java failed
> >>>- The service lock was taken out of order with the metaspace locks
> >>>These bugs have all been fixed:
> >>>- The asserts have been removed since they are no longer true
> >>>- The test has been updated but is not part of this change since it is a
> >>> JDK change
> >>>- This change does not make use of functions requiring the metaspace
> >>> lock. I had to remove some verification code in free_chunks_total to
> >>> ensure this.
> >>Overall I think your fix looks good but I disagree with your choice
> >>of exposing an EmptyCompressedKlassSpacePool for the case of
> >>Given the dynamic nature of the MemoryManagerMXBeans and
> >>MemoryPoolMXBeans it seems more true to the intentions of the API to
> >>ony expose a MemoryPool if it contains interesting information.
> >>Generally I don't think users of the management APIs can (or should)
> >>depend on the exact set of MemoryManagerMXBeans and
> >>available in a given VM instance.
> >>>- One new jtreg test
> >>>- JPRT
> >>>- All the tests that failed in nighly testing last time now pass
More information about the hotspot-gc-dev