RFR: 8013590: NPG: Add a memory pool MXBean for Metaspace

[Erik Helin]

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?

Thanks,
Erik

> >- 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:
> >http://cr.openjdk.java.net/~ehelin/8013590/webrev.01/
> 
> Rest looks fine.
> 
> Jon
> 
> >
> >On 2013-06-04, Jon Masamitsu wrote:
> >>http://cr.openjdk.java.net/~ehelin/8013590/webrev.00/test/gc/metaspace/TestMetaspaceMemoryPool.java.html
> >>
> >>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
> >-1").
> >
> >I would like to do these changes as a separate change. Jon, what do you
> >about that?
> >
> >Thanks,
> >Erik
> >
> >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.
> >>>
> >>>Webrev:
> >>>http://cr.openjdk.java.net/~ehelin/8013590/webrev.00/
> >>Overall I think your fix looks good but I disagree with your choice
> >>of exposing an EmptyCompressedKlassSpacePool for the case of
> >>-UseCompressedClassPointers.
> >>
> >>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
> >>MemoryPoolMXBeans
> >>available in a given VM instance.
> >>
> >>/Mikael
> >>
> >>>Testing:
> >>>- One new jtreg test
> >>>- JPRT
> >>>- All the tests that failed in nighly testing last time now pass
> >>>
> >>>Thanks,
> >>>Erik
> >>>
>