[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-serviceability-dev
Subject:    Re: RFR: 8013590: NPG: Add a memory pool MXBean for Metaspace
From:       Mikael Gerdin <mikael.gerdin () oracle ! com>
Date:       2013-05-31 12:09:41
Message-ID: 51A89305.3070005 () oracle ! com
[Download RAW message or body]

(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@openjdk.java.net, sending to
> serviceability-dev@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
>
[prev in list] [next in list] [prev in thread] [next in thread]