RFR(xs): 8245926: JFR: Remove chunkSz and rename blockSz from classloader statistics

[Erik Gahlin]

Hi Thomas.

> 
> 
> On Wed, May 27, 2020 at 10:06 AM Erik Gahlin <erik.gahlin at oracle.com <mailto:erik.gahlin at oracle.com>> wrote:
> Hi Thomas, 
> 
> An important use case for JFR is to have all relevant information available for support since they can’t typically run jcmd. If jcmd has better information, I think we should strive to make that information available for JFR as well.
> 
> An alternative would be to create a new event for metaspace targeted for JVM engineers/support, with similar information as jcmd(?), and only keep fields relevant to a Java developer in ClassLoaderStatistics.
> 
> Thanks
> Erik 
> 
> 
> Okay, you have a point. I may have misunderstood you before.

Sorry for the mixed message, but when you said jcmd had better information, it got me thinking.

Maybe we should create something more useful than just modify what we have today?

> I'll withdraw the RFE for now. Since a lot of implementation details will change, if this kind of detail is to be exposed via JFR, this will need brushing after JDK-8221173 hits mainline anyway so it makes no sense to put much work into it now.

Sounds good.

> 
> My question about backward compatibility remains though: can one just change/remove event fields? I assume JMC uses those fields for things beyond displaying events in the generic event browser, yes? How does JMC handle backward compatibility - is it only supposed to work with one hotspot or with a range of them?

Users of the API should check that a field exists. It is stated in the Javadoc.

JMC has its own parser and it supports JVMs back to JDK 7 so there is a mechanism to handle missing information. 

That said, we should not remove/change fields unless there is a good reason. 

> 
> For instance, the "Class Loader Statistics" Tab in the "Class Loading" section of the VM still talks about "Anonymous Classes" and shows no content there; I assume this was broken with the Hidden Classes JEP which renamed the fields in ClassLoaderStatistics from "Anonymousxxx" to "Hiddenxxx". So is it bound to one specific hotspot version? Just a guess, I did not look at the JMC sources.

I’m looping in JMC, so they can file a bug on it. 

Thanks
Erik

> 
> Thanks, Thomas
> 
> 
> 
> > On 27 May 2020, at 08:57, Thomas Stüfe <thomas.stuefe at gmail.com <mailto:thomas.stuefe at gmail.com>> wrote:
> > 
> > Hi,
> > 
> > may I have opinions/reviews for this change please:
> > 
> > JBS: https://bugs.openjdk.java.net/browse/JDK-8245926 <https://bugs.openjdk.java.net/browse/JDK-8245926>
> > Webrev:
> > http://cr.openjdk.java.net/~stuefe/webrevs/8245926-jfr-remove-chunksz-rename-blocksz/webrev.00/webrev/ <http://cr.openjdk.java.net/~stuefe/webrevs/8245926-jfr-remove-chunksz-rename-blocksz/webrev.00/webrev/>
> > 
> > This is part of an effort to reduce exposure of Metaspace implementation
> > details somewhat in preparation for JDK-8221173.
> > 
> > In JFR, the classloader statistics event contains the following fields:
> > 
> > chunkSize
> > blockSize
> > hiddenChunkSize
> > hiddenBlockSize
> > 
> > blockSize is the interesting one, the number of bytes of Metaspace used by
> > this class loader for storing metadata. I propose to rename this to
> > "totalMetaspaceUsed" resp. "hiddenClassesMetaspaceUsed".
> > 
> > chunkSize is the total size of chunks given to a class loader. Delta
> > between blockSize and chunkSize constitutes waste if the loader stops
> > loading; however, as far as waste goes this is not very informative since
> > it only is a usually small part of waste in the current metaspace
> > allocator.
> > 
> > Moreover, with JDK-8221173 chunks may be partially uncommitted, so that the
> > delta between chunkSize and blockSize may or may not contribute to RSS. So
> > the chunkSize stat becomes more meaningless.
> > 
> > I'd argue that this stat is quite useless to the average JFR user; and to
> > the VM developer interested in fixing up Metaspace there are better tools
> > (e.g. jcmd VM.metaspace). Therefore I would like to simply remove this stat.
> > 
> > --
> > 
> > Please note that this is my first change to JFR events. Some things are
> > unclear to me, e.g. how backward compatibility in JMC is handled. So if
> > something is missing from the patch, please advice.
> > 
> > 
> > Thanks!
> > 
> > .. Thomas
>