RFR (XS) 8215221: Serial GC misreports young GC time

[Kim Barrett]

> On Jan 24, 2019, at 10:37 AM, Aleksey Shipilev <shade at redhat.com> wrote:
> 
> On 12/18/18 4:52 PM, Kim Barrett wrote:
>>> On Dec 11, 2018, at 3:23 PM, Aleksey Shipilev <shade at redhat.com> wrote:
>>> Bug:
>>> https://bugs.openjdk.java.net/browse/JDK-8215221
>>> 
>>> One of our guys pointed out the oddity in Serial GC logs: "Pause Young" includes the timing for Full
>>> collection, and wrecks up GCId order as well. This is because GCTraceTime-s are nested on those
>>> paths, and putting them in proper order, closer to collect_generation, fixes it.
>>> 
>>> The downside is that some of the cleanup actions are now outside the GCTraceTime. Those look
>>> innocuous to omit to me, though.
>> 
>> ClassLoaderDataGraph::purge() doesn’t look so innocuous.  I haven’t
>> looked hard at other stuff being omitted from accounting by this change.
>> 
>> It seems to me that fixing this reporting problem might require some
>> restructuring of this rather complicated and flag-filled function.
> 
> Okay, another attempt. This restructures the code a little bit, which IMO makes the flow more
> understandable. It also eliminates a few flags. Then, the placement for GCTraceTime becomes natural.
> Well, almost, because it requires some dance around should_do_full_collection, if we want to include
> young-only epilog into timing.
> 
> See:
>  http://cr.openjdk.java.net/~shade/8215221/webrev.02/
> 
> This passes hotspot_tier1 and my ad-hoc tests. If this is the way to go, I would do jdk-submit, etc.
> then.
> 
> -Aleksey

Thanks; this looks somewhat better to me.  It seems like there should
be more opportunities for cleanup here, but doing much more might
require a complete top to bottom rewrite of this function.  Since I
don't have any concrete suggestions for further changes right now,
looks good.