8214155: GCHeapLog inconsistents with CollectorCounters

[Thomas Schatzl]

Hi Yasumasa,

On Thu, 2018-11-22 at 23:02 +0900, Yasumasa Suenaga wrote:
> Hi Thomas,
> 
> I erroneously assumed that `full` value in hs_err GC events means #
> of STWs. I will withdraw this CR.

Maybe it is better to show this - that was mostly the question I had.
:)

> > - the sun.gc.collector.2.name label "G1 stop-the-world phases"
> > seems to be just wrong. Looking at
> > src/jdk.jcmd/share/classes/sun/tools/jstat/resources/jstat_options,
> > the comment for sun.gc.collector.2.* and the actual code indicates
> > that this should actually be called something like "G1 concurrent
> > (stop-the-world) phases".
> 
> I agree with you.
> Should I file this to JBS?

Please.

> > - not sure it is useful to notify about remark and cleanup pause
> > separately in the sun.gc.collector.2 statistics - isn't it
> > sufficient to have a concurrent cycles count there? (idk about any
> > dependencies).
> 
> If sun.gc.collector.2 is not available, first STW phase will not be
> counted when concurrent GC cycle is aborted: concurrent mode failure.

Currently the count will be updated in the Remark pause - wouldn't it
be better to do that later then when the concurrent cycle really
completed?

> > - some recent change might have introduced an issue: looking at
> > g1ConcurrentMark.cpp:1359 where the "total_collection" count is
> > increased there is some comment about races which I do not
> > understand.
> > However that count might be more appropriately increased in the
> > Cleanup pause. Or not at all there, but rather at the end of the
> > concurrent cycle (and isn't this a kind of "full" collection?).
> > 
> > What would be the compatibility implications of all that?
> 
> G1ConcurrentMark::cleanup() seems to be called at safepoint. It has
> assertion for safepoint. So I guess it will not race.
> 

You are right, it's in the Cleanup pause so it should be fairly okay.
Although there still could be an abort later. So it might be useful to
move the increment of total_full_collections really into the
(concurrent) end-of-concurrent cycle code?

Thanks,
  Thomas