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

[Mikael Gerdin]

Erik,

On 2013-06-11 17:27, 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').
> - 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/

This looks much better, ship it!

/Mikael

>
> 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
>>>