RFR: 8175375: MemoryPoolMXBean.getCollectionUsage() does not works with -XX:+UseG1GC -XX:+ExplicitGCInvokesConcurrent

Thomas Schatzl thomas.schatzl at oracle.com
Thu Sep 6 14:01:29 UTC 2018


On Thu, 2018-09-06 at 04:59 +0000, Fairoz Matte wrote:
> Hi Thomas,
> Thanks for looking into this.
> > > [...]
> > > JBS issue - https://bugs.openjdk.java.net/browse/JDK-8175375
> > > Webrev - http://cr.openjdk.java.net/~fmatte/8175375/webrev.00/
> > 
> >   I think this change breaks actual accounting: the
> > G1MonitoringScopes will
> > immediately be destructed at the end of the if/else blocks, so they
> > will not
> > be measuring values correctly.
> > 
> > I think you need to set the full_gc parameter of the scope
> > depending on the
> > gc cause instead.
> This call is to only initialize G1MonitoringScopes, so when we call
> G1MonitoringSupport::update_sizes() it still in same scoping level as
> an active TraceMemoryManagerStats object. 
> Just forgot to mention that, I have added new test case
> "TestTenuredGenPoolCollectionUsage.java", which gives expected
> results before and after the patch.

But the MonitoringScope also implicitly starts and ends the embedded
TraceCollectorStats/TraceMemoryManagerStats, i.e. calls their
destructors, which e.g. save some current time stamp. Which will be
completely off compared to before.

> But it is good idea to keep the scope depending on the gc cause. I
> have updated the patch accordingly. 
> Please find the updated webrev -
> http://cr.openjdk.java.net/~fmatte/8175375/webrev.01/ 

The fix is good, but looking at the CR I am not sure that this fix
addresses the entire problem, or I believe the problem has not been
understood rightly.

In this case the user wanted to have the old pool MemoryMXBean updated
with the concurrent start gc. I assume that person is moving from CMS
noting this inconsistency. CMS seems to update the old pool for any
concurrent start gc.

So this change only fixes the case when calling system.gc(), not any
other start of marking gc. (Probably the predicate collector_state()-
>in_initial_mark_gc() is correct here?)

I believe further investigation of what MXBeans/pool(s)/jstat counters
CMS updates in any initial mark GC would be warranted to get expected
behavior, and fix this appropriately.

I personally do not have an opinion, as an concurrent start could be
considered both part of a "young" and "old" gc. Actually I would be
really happy if somebody wrote down the expectations of what
MemoryMXBean, pools and the CollectionCounters (jstat?) should be
affected when (in form of a unit test preferably).

Because changing this "full_gc" parameter depending on type of young gc
or gc cause of the G1MonitoringScope also has the subtle effect that
instead of the "young" collection counters
(G1MonitoringSupport::_incremental_collection_counters) instead of the
"old" collection counters
(G1MonitoringSupport::_full_collection_counters) are updated. Instead
of always the former.

Cc'ed phh and ysuenaga because they seem to be users of these :)


More information about the hotspot-gc-dev mailing list