RFR: 8036703: Add trace event with statistics for the metaspace chunk free lists

[Mikael Gerdin]

Erik,

On Thursday 13 March 2014 14.00.33 Erik Helin wrote:
> Hi Mikael and Stefan,
> 
> thanks for having a look and special thanks to Mikael for finding the bug!
> 
> Please see new webrev at:
> http://cr.openjdk.java.net/~ehelin/8036703/webrev.04/
> 
> Please see the incremental webrev at:
> http://cr.openjdk.java.net/~ehelin/8036703/webrev.03-04/

Looks good.

/Mikael

> 
> Based on some internal feedback, I also corrected a few nits (see the
> incremental webrev for details).
> 
> Thanks,
> Erik
> 
> On 2014-03-13 10:40, Stefan Karlsson wrote:
> > On 2014-03-13 09:57, Mikael Gerdin wrote:
> >> Hi Erik,
> >> 
> >> On Wednesday 12 March 2014 17.04.26 Erik Helin wrote:
> >>> Based on feedback from Mikael and StefanK, I've reworked the patch to
> >>> put the ChunkFreeListSummaries in the class MetaspaceSummary.
> >>> 
> >>> Please see the new version at:
> >>> http://cr.openjdk.java.net/~ehelin/8036703/webrev.03/
> >> 
> >> I think this version is much better!
> >> Just a small issue:
> >> 
> >> collectedHeap.cpp
> >> 
> >>    99
> >>   
> >>   100   const MetaspaceChunkFreeListSummary& ms_chunk_free_list_summary =
> >>   101     MetaspaceAux::chunk_free_list_summary(Metaspace::ClassType);
> >>   102   const MetaspaceChunkFreeListSummary&
> >> 
> >> class_chunk_free_list_summary =
> >> 
> >>   103     MetaspaceAux::chunk_free_list_summary(Metaspace::NonClassType);
> >>   104
> >> 
> >> It looks like you pass the wrong MetadataType to
> >> chunk_free_list_summary here,
> >> ClassType and NonClassType should be swapped.
> > 
> > Except for this bug, this looks good.
> > 
> > thanks,
> > StefanK
> > 
> >> /Mikael
> >> 
> >>> Thanks,
> >>> Erik
> >>> 
> >>> On 2014-03-06 11:56, Erik Helin wrote:
> >>>> A small bug had found its way into this patch:
> >>>> 
> >>>> metaspace.cpp:
> >>>> +bool MetaspaceAux::has_chunk_free_list(Metaspace::MetadataType
> >>>> mdtype) {
> >>>> +  return Metaspace::get_chunk_manager(mdtype) == NULL;
> >>>> +}
> >>>> 
> >>>> should of course be:
> >>>> 
> >>>> +bool MetaspaceAux::has_chunk_free_list(Metaspace::MetadataType
> >>>> mdtype) {
> >>>> +  return Metaspace::get_chunk_manager(mdtype) != NULL;
> >>>> +}
> >>>> 
> >>>> Please see new webrev at:
> >>>> http://cr.openjdk.java.net/~ehelin/8036703/webrev.01/
> >>>> 
> >>>> Thanks,
> >>>> Erik
> >>>> 
> >>>> On 2014-03-06 10:35, Erik Helin wrote:
> >>>>> Hi all,
> >>>>> 
> >>>>> this patch adds a new trace event -
> >>>>> vm/gc/metaspace/chunk_free_list_summary for getting statistics for
> >>>>> metaspace's chunk free lists. The event contains the numbers of chunks
> >>>>> in the free list and the size of the chunks in the free lists. This
> >>>>> information is provided for all different kinds of chunks - special,
> >>>>> small, medium and humongous.
> >>>>> 
> >>>>> This information will be useful when diagnosing fragmentation
> >>>>> issues in
> >>>>> metaspace.
> >>>>> 
> >>>>> Webrev:
> >>>>> http://cr.openjdk.java.net/~ehelin/8036703/webrev.00/
> >>>>> 
> >>>>> Enhancement:
> >>>>> https://bugs.openjdk.java.net/browse/JDK-8036703
> >>>>> 
> >>>>> Testing:
> >>>>> - JFR JTREG tests
> >>>>> - JPRT
> >>>>> - Manually verifying that the numbers look correct by looking at
> >>>>> 
> >>>>>     recordings from small test cases.
> >>>>> 
> >>>>> Thanks,
> >>>>> Erik