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/ 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-...@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 > >