RFR(S): 8223575: add subspace transitions to gc+metaspace=info log lines

[Thomas Stüfe]

Hi Tony,

I think this makes sense. I agree, the output as it is now can be
confusing. Note that users can still be confused since users may simply not
be aware that the default ClassSpace reserved size is 1G.

Can your change break scripts for folks parsing GC logs? I see that you
only appended content and did not change the existing text, so I guess this
is okay.

---

Format and naming: Different forms are used in different places. In hs-err
files, we print "Metaspace" and "Class Space", with the former being the
total sum and the latter the class space part.

But in other corners, eg. in the jcmd VM.metaspace command, we print
non-class space and class space and maybe the sum in addition. Usually
non-class space is called "Non-Class".

I personally would prefer, in this case, the latter names: "Class Space"
and "Non-Class Space"

---

Note that for non-class space, reserved size can actually change when
allocating and releasing metaspace, if VirtualSpaceNodes get purged or
newly allocated. Hence total reserved size can change too. So we may want
to print out reserved before and reserved after sizes. I guess that was
also missing from the output before.

----

Since used = class used + nonclass used, I would simplify
PreMetaspaceValues to _non_class_used and _class_used and calculate the sum
on the fly.

---

You added a new dependency to heap.hpp into metaspace.cpp for the format
string. Is this really necessary? Can you move this define to metaspace.cpp?

If not, you need at least add include heap.hpp (did it build this way
without precompiled headers?)

----

The following issues are matters of taste, and I leave it up to you whether
you want to change something:

-> I wince a bit at yet another metaspace-sizes-structure - we have already
ClassLoaderMetaspaceStatistics and MetaspaceCounters, and maybe others. JFR
may also do something similar. Would it be possible to reuse one of those?
(I guess ClassLoaderMetaspaceStatistics is too fine granular and too
expensive to collect.)

-> I would prefer the class PreMetaspaceValues to live inside an own header
in metaspace folder, and in the metaspace namespace like we did for similar
helpers.

-> I would take the "Pre" out of the name and the member names and just
name it "Snapshot" and the members "used" and "class used".

-> I would probably have made PreMetaspaceValues a dumb structure and added
a function "MetaspaceAux::get_dimensions()" instead of adding the
implementation to the constructor and to the header file. At least I would
prefer the ctor implementation to the cpp file.

Cheers, Thomas

On Wed, May 8, 2019 at 4:20 PM Tony Printezis <tprintezis at twitter.com>
wrote:

> Any chance of someone taking a look at this?
>
> http://cr.openjdk.java.net/~tonyp/8223575/webrev.0/
>
> As I said on the JIRA, users can be confused when they metaspace capacity
> shows as over 1G even though they asked for 128M. Breaking down the spaces,
> when coops are enabled, can address this confusion (and having the actual
> occupancy of each space separately is also a lot more informative and
> avoids guesswork). The output looks like this when coops are enabled:
>
> [10.090s][info][gc,metaspace     ] GC(8) Metaspace: 2130K->2130K(1056768K)
> ClassMetadata: 1906K->1906K(8192K) ClassSpace: 223K->223K(1048576K)
>
> and like this (i.e., what it was before the change) when coops are not
> enabled:
>
> [25.767s][info][gc,metaspace     ] GC(28) Metaspace: 8107K->8107K(10240K)
>
> A couple of notes on the changes:
>
> - I’m open to different suggestions for what to call ClassMetadata and
> ClassSpace. :-)
> - I didn’t know where to put the HEAP_CHANGE_FORMAT and
> HEAP_CHANGE_FORMAT_ARGS macros, so that they can be easily shared, and I
> ended up putting them in heap.hpp. Let me know if there’s a better place
> for them.
>
> Tony
>
>
> —————
> Tony Printezis | @TonyPrintezis | tprintezis at twitter.com
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.org/pipermail/hotspot-gc-dev/attachments/20190508/14db073f/attachment.htm>