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

[Thomas Stüfe]

Hi Erik,


On Wed, May 27, 2020 at 5:20 PM Erik Gahlin <erik.gahlin at oracle.com> wrote:

> Hi Thomas.
>
>
>
> On Wed, May 27, 2020 at 10:06 AM Erik Gahlin <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.
>
>
No problem. You thinking got me thinking :) So far I have almost
exclusively worked with jcmd and the like since I prefer command line toos;
but if JMC is used by hotspot devs too it makes sense to expose more
information.


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

Yes. I know the metaspace implementation quite well but I find what
JMC tells me not that useful. Part is different terminology (e.g. "Data
Space"), part is just the selection of information. I think this can be
improved. But since I would like to push this beyond Elastic Metaspace
(JDK-8221173) this will not happen before JDk16.


> 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.
>
>
Okay, thanks.

>
> 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
>
>
Cheers, Thomas

>
> Thanks, Thomas
>
>
>
>> > On 27 May 2020, at 08:57, Thomas Stüfe <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
>> > 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
>>
>>
>