8175375: MemoryPoolMXBean.getCollectionUsage() does not works with -XX:+UseG1GC -XX:+ExplicitGCInvokesConcurrent
hohensee at amazon.com
Thu Sep 6 18:05:12 UTC 2018
Also, forgot to note that there's a jstat counter for concurrent collection cycles, see in g1MonitoringSupport.hpp "CollectorCounters* _conc_collection_counters;" and associated code.
On 9/6/18, 10:59 AM, "hotspot-gc-dev on behalf of Hohensee, Paul" <hotspot-gc-dev-bounces at openjdk.java.net on behalf of hohensee at amazon.com> wrote:
I don't think this is a bug. It makes perfect sense to me that the G1 Old Gen pool may not be updated by a concurrent cycle. If the heap is big enough (and your test doesn't set -Xmx, so it is), there will be no mixed collection, so nothing will be promoted to the old gen, so the G1 Old Gen pool will correctly read as empty. Run with GC logging enabled to see if that's what's happening.
I suggest writing a test that uses the white box API and a small -Xmx to force a mixed collection (see test/hotspot/jtreg/gc/g1/mixedgc/TestOldGenCollectionUsage.java for an example). You should see that the G1 Old Gen pool is updated after a concurrent cycle if a mixed collection occurs. The fix for https://bugs.openjdk.java.net/browse/JDK-8195115 made the G1 Old Gen pool correctly reflect the result of a mixed collection.
I'm in the process of modifying the patch for https://bugs.openjdk.java.net/browse/JDK-8196989 to fit with Thomas' recent G1 serviceability refactoring. It details the way things will work once the patch is done.
On 9/6/18, 7:02 AM, "Thomas Schatzl" <thomas.schatzl at oracle.com> wrote:
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 -
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
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