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

Tony Printezis tprintezis at twitter.com
Thu Aug 8 13:50:32 UTC 2019


Hi Kim,

Inline.


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


On August 7, 2019 at 6:42:56 PM, Kim Barrett (kim.barrett at oracle.com) wrote:

> On Aug 7, 2019, at 10:05 AM, Tony Printezis <tprintezis at twitter.com>
wrote:
>
> Hi all,
>
> Similar to 8227225 but for the GenCollectedHeap GCs. Webrev is here:
>
> http://cr.openjdk.java.net/~tonyp/8227224/webrev.0/
>
> Tony
>
>
> —————
> Tony Printezis | @TonyPrintezis | tprintezis at twitter.com

Looks good.

I had a somewhat longer comment about the similarities between this new
code for GenCollectedHeap and the recently added code for
ParallelScavengeHeap, but re-reading the previous thread I agree with
the plan to keep things relatively isolated for now and perhaps look for
possible refactorings once we have more complete coverage, particularly
including non-generational collectors.


Yeah, we can definitely increase the amount of code sharing here. And IMHO
the main benefit is not to decrease the amount of code, but to ensure that
the output is consistent across all GCs. But can I also point out that,
before, there was NO code sharing whatsoever (all this code was replicated
multiple times). Now at least there’s some common code and common macros.
And we can improve on that further.

While we’re at it: I’m happy to work on follow-ups. What’s a good next
step? As Thomas had suggested, I can change the formatting code to use more
appropriate units instead of always K. Another possibility is to update the
‘gc' log lines to the same format? E.g.,

[29.884s][info][gc           ] GC(24) Pause Young (Allocation Failure)
6147M->3M(9216M) 2.705ms



One pre-existing nit I noticed while looking at the two side by side.
In ParallelScavengeHeap::get_pre_gc_values() the young->to_space()
value is fetched but not used. I don't need a new webrev for removal
of that line.


Thanks for pointing this out. I had added the to-space transition in the
output but, after discussing it with Thomas, we decided that it’s probably
not necessary. So I removed it from the code. But I clearly forgot that
assignment. I’ll add the removal of that line to this change.


Tony



More information about the hotspot-gc-dev mailing list