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

[Fairoz Matte]

Hi Thomas,

Thanks for looking into this.

> -----Original Message-----
> From: Thomas Schatzl
> Sent: Wednesday, September 05, 2018 8:40 PM
> To: Fairoz Matte <fairoz.matte at oracle.com>; hotspot-gc-
> dev at openjdk.java.net
> Subject: Re: RFR: 8175375: MemoryPoolMXBean.getCollectionUsage() does
> not works with -XX:+UseG1GC -XX:+ExplicitGCInvokesConcurrent
> 
> Hi Fairoz,
> 
> On Wed, 2018-09-05 at 06:55 -0700, Fairoz Matte wrote:
> > Hi,
> >
> > Please review this change.
> > MemoryPoolMXBean.getCollectionUsage() does not works in G1 with "-
> > XX:+ExplicitGCInvokesConcurrent", it always returns 0.
> > In case of "+ExplicitGCInvokesConcurrent" call to heap usage
> > collection doesn't honor full gc.
> > With the change of  8175375, it will check for full GC request and set
> > the heap usage collection accordingly.
> >
> > 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 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/ 

Thanks,
Fairoz
> 
> I am kind of surprised that gccause is GCCause::_java_lang_system_gc for
> this young collection, but that may be correct.
> 
> Thanks,
>   Thomas
>