RFR(S): 8227225: ParallelGC: add subspace transitions for young gen for gc+heap=info log lines

[Tony Printezis]

Hey Thomas,

Thanks for taking a look and again apologies for the late reply, I was on
vacation until earlier this week. Please see inline.


—————
Tony Printezis | @TonyPrintezis | tprintezis at twitter.com


On July 10, 2019 at 11:19:37 AM, Thomas Schatzl (thomas.schatzl at oracle.com)
wrote:

On Fri, 2019-07-05 at 11:06 -0700, Tony Printezis wrote:
> Hi all,
>
> This is a follow-up to 8223575 and adds subspace info for eden and
> from space to the gc+heap=info output for ParallelGC. I also added
> the before capacity, similar to 8223575, as it can change during the
> GC (see, in fact, example below).
>
> Before:
>
> [11.123s][info][gc,heap ] GC(17) PSYoungGen: 4759616K-
> >1664K(4545536K)
> [11.123s][info][gc,heap ] GC(17) ParOldGen: 1280K-
> >1280K(1376256K)
>
> After:
>
> [12.133s][info][gc,heap ] GC(25) PSYoungGen:
> 2065952K(2066432K)->1536K(1983488K) Eden: 2064384K(2064384K)-
> >0K(1981952K)
> From: 1568K(2048K)->1536K(1536K)

Note that I think this is confusing, or the "From" label should be
renamed. The second part of this heap transition actually prints the
"To" space from what I understand.


That’s not entirely true, though. The spaces do switch roles at the end of
the GC, so the To space does become the From space of the following GC. So
using From here is not entirely incorrect (but I do get your point).



What does it print on evacuation failure?


[55.239s][info][gc,start     ] GC(18) Pause Young (Allocation Failure)

[57.547s][info][gc,promotion ] GC(18) Promotion failed

[57.548s][info][gc,heap      ] GC(18) PSYoungGen:
3522720K(4032000K)->3522720K(4032000K) Eden:
3456000K(3456000K)->3456000K(3456000K) From:
66720K(576000K)->66720K(576000K)

[57.548s][info][gc,heap      ] GC(18) ParOldGen:
502048K(634880K)->634872K(634880K)

[57.548s][info][gc,metaspace ] GC(18) Metaspace: 2681K(5504K)->2681K(5504K)
NonClass: 2376K(4864K)->2376K(4864K) Class: 305K(640K)->305K(640K)

[57.548s][info][gc           ] GC(18) Pause Young (Allocation Failure)
3930M->4060M(4557M) 2309.101ms

[57.548s][info][gc,cpu       ] GC(18) User=7.43s Sys=5.66s Real=2.31s



It would be cleaner imho to print From and To space separately.


I can definitely add To space. It will be empty most of the time, minus
some edge cases (IIRC).



>
> [12.133s][info][gc,heap ] GC(25) ParOldGen:
> 1272K(1376256K)->1272K(1376256K)
>
> http://cr.openjdk.java.net/~tonyp/8227225/webrev.0/
>
> I have a similar change ready to go for GenCollectedHeap (Serial /
> CMS).
> I’ll see what the feedback is on this before I open that for code
> review.
>
> A couple of notes:
>
> - I moved the PreGCValues class to gc/shared as I’d like to re-use
> it for the GenCollectedHeap changes too.

It's a bit awkwards as there are non-generational GCs now, but I do not
have a better idea. Maybe somebody else has.


I could just rename this to GenPreGCValues so it’s clear it’s only for the
generational GCs? Or I could come up with a convoluted class hierarchy that
can handle both generational and non-generational GCs. But I’d rather not,
given we don’t have any use cases for this for the latter. So, maybe,
rename PreGCValues to GenPreGCValues and, if we need to in the future, we
can just split the class as needed?



> - Is it worth also adding info about To space to the output?

I think so.

(Note that I did not really review this change yet, just answering
questions)


#understood (and thanks for the explanation re: the gc-pending-review label)


Tony




Thanks,
Thomas

>
> Tony
>
> PS Should I add the gc-pending-review label to any CR I open for code
> review?

As discussed, feel free to do so, but it may or may not help. Note that
there is no need to remove that label manually: if you set your JIRA
filter to ignore already fixed/resolved issues they will not be shown
automatically (if you filter).

Once or twice a year we typically bulk-remove these labels from already
resolved/closed issues.