RFR(S) 8207765: HeapMonitorIntervalRateTest fails with ZGC

Per Liden per.liden at oracle.com
Wed Aug 8 21:16:08 UTC 2018


Hi JC,

On 2018-08-08 20:07, JC Beyler wrote:
> Hi Per,
> 
> You are correct, my test shows that an int[1] will use 32 bytes 
> with -XX:-UseCompressedClassPointers under G1. My apologies for not 
> digging more into why with ZGC :-)
> 
> In any case, my test was fragile since using that flag would break it, 
> the fix was not a ZGC only fix, I basically made it first calculate the 
> size and then use that for the test. So, it is at least a better test 
> and should be stable now (famous last words).

Sounds good.

> 
> Thanks for pointing out the actual reason why there were 8 extra bytes!

No problem.

cheers,
Per

> Jc
> 
> On Wed, Aug 8, 2018 at 6:19 AM Per Liden <per.liden at oracle.com 
> <mailto:per.liden at oracle.com>> wrote:
> 
>     Hi JC,
> 
>     (Sorry, I didn't see this thread until now... been on vacation,
>     jvmls, etc)
> 
>     On 07/19/2018 11:52 PM, JC Beyler wrote:
>      > Hi Serguei,
>      >
>      > Done here:
>      > http://cr.openjdk.java.net/~jcbeyler/8207765/webrev.01/
>     <http://cr.openjdk.java.net/%7Ejcbeyler/8207765/webrev.01/>
>      >
>      > I added:
>      >
>      > + // Calculate the size of a 1-element array in order to assess
>     average
>      > sampling interval
>      > + // via the HeapMonitorStatIntervalTest. This is needed because
>     various
>      > GCs could add
>      > + // extra memory to arrays.
>      > + // This is done by allocating a 1-element array and then
>     looking in
>      > the heap monitoring
>      > + // samples for the average size of objects collected.
>      >
>      >
> 
>     I was just a bit curious as to what the actual problem was here. While
>     it's true that a GC might use extra memory for an object, ZGC
>     doesn't do
>     that. An int[1] in ZGC has the same size as in G1/Parallel/Serial/CMS.
>     The only exception I can think of is that you where using compressed
>     class pointers, which is generally enabled by default, but
>     disabled/unsupported in ZGC. Given this, I'm thinking you should have
>     seen the same issue with say G1 if you were using
>     -XX:-UseCompressedClassPointers, no?
> 
>     cheers,
>     Per
> 
>      > Let me know what you think and then I need one more review to
>     prepare
>      > the patch :-)
>      >
>      > Thanks all!
>      > Jc
>      >
>      > On Thu, Jul 19, 2018 at 2:33 PM serguei.spitsyn at oracle.com
>     <mailto:serguei.spitsyn at oracle.com>
>      > <mailto:serguei.spitsyn at oracle.com
>     <mailto:serguei.spitsyn at oracle.com>> <serguei.spitsyn at oracle.com
>     <mailto:serguei.spitsyn at oracle.com>
>      > <mailto:serguei.spitsyn at oracle.com
>     <mailto:serguei.spitsyn at oracle.com>>> wrote:
>      >
>      >     Hi Jc,
>      >
>      >     The fix looks good to me.
>      >     Just minor comments.
>      >
>      >
>     http://cr.openjdk.java.net/%7Ejcbeyler/8207765/webrev.00/test/hotspot/jtreg/serviceability/jvmti/HeapMonitor/MyPackage/HeapMonitor.java.frames.html
>      >
>      >     108 public static void calculateAverageOneElementSize() {
>      >
>      >        Could you, please, add a comment before
>      >     calculateAverageOneElementSize method
>      >        explaining shortly why it is needed and what it is doing?
>      >        Otherwise, it is not easy to understand this code from
>     scratch.
>      >
>      >     Thanks,
>      >     Serguei
>      >
>      >
>      >     On 7/19/18 10:08, JC Beyler wrote:
>      >>     I forgot to put the link:
>      >> https://bugs.openjdk.java.net/browse/JDK-8207763
>      >>
>      >>     It got renamed in jdk11 via:
>      >> http://hg.openjdk.java.net/jdk/jdk11/rev/1edcf36fe15f
>      >>
>      >>     Thanks!
>      >>     Jc
>      >>
>      >>     On Thu, Jul 19, 2018 at 10:07 AM JC Beyler
>     <jcbeyler at google.com <mailto:jcbeyler at google.com>
>      >>     <mailto:jcbeyler at google.com <mailto:jcbeyler at google.com>>>
>     wrote:
>      >>
>      >>         Hi Dan,
>      >>
>      >>
>      >>       
>       serviceability/jvmti/HeapMonitor/MyPackage/HeapMonitorStatRateTest.java
>      >>         became
>      >>       
>       serviceability/jvmti/HeapMonitor/MyPackage/HeapMonitorStatIntervalTest.java,
>      >>         when we updated the spec and said "rate" was the wrong word.
>      >>
>      >>         So yes, it fixes both since at some point all branches
>     should
>      >>         see that the StatRate test becomes renamed into the
>      >>         StatInterval test. Does that make sense?
>      >>
>      >>         Thanks!
>      >>         Jc
>      >>
>      >>
>      >>         On Thu, Jul 19, 2018 at 9:45 AM Daniel D. Daugherty
>      >>         <daniel.daugherty at oracle.com
>     <mailto:daniel.daugherty at oracle.com>
>      >>         <mailto:daniel.daugherty at oracle.com
>     <mailto:daniel.daugherty at oracle.com>>> wrote:
>      >>
>      >>             JDK-8207765 covers two different tests as of yesterday:
>      >>
>      >>           
>       serviceability/jvmti/HeapMonitor/MyPackage/HeapMonitorStatRateTest.java
>      >>
>      >>             and
>      >>
>      >>           
>       serviceability/jvmti/HeapMonitor/MyPackage/HeapMonitorStatIntervalTest.java
>      >>
>      >>             I updated it to add a similar failure mode sighting for
>      >>             HeapMonitorStatIntervalTest.java
>      >>
>      >>
>      >>             Does your fix address both test failures?
>      >>
>      >>             Dan
>      >>
>      >>
>      >>             On 7/19/18 12:39 PM, JC Beyler wrote:
>      >>>             Hi all,
>      >>>
>      >>>             Could I have a few reviews of:
>      >>> http://cr.openjdk.java.net/~jcbeyler/8207765/webrev.00/
>     <http://cr.openjdk.java.net/%7Ejcbeyler/8207765/webrev.00/>
>      >>>           
>       <http://cr.openjdk.java.net/%7Ejcbeyler/8207765/webrev.00/>
>      >>>
>      >>>             The  test assumed the size of a 1-element array but ZGC
>      >>>             changes that assumption. The test now first allocates a
>      >>>             bit of memory and gets the average size of the samples
>      >>>             before assuming the size. This works with/without ZGC.
>      >>>
>      >>>             Webrev:
>      >>> http://cr.openjdk.java.net/~jcbeyler/8207765/webrev.00/
>     <http://cr.openjdk.java.net/%7Ejcbeyler/8207765/webrev.00/>
>      >>>           
>       <http://cr.openjdk.java.net/%7Ejcbeyler/8207765/webrev.00/>
>      >>>             Bug: https://bugs.openjdk.java.net/browse/JDK-8207765
>      >>>
>      >>>             Thanks!
>      >>>             Jc
>      >>
>      >>
>      >>
>      >>         --
>      >>
>      >>         Thanks,
>      >>         Jc
>      >>
>      >>
>      >>
>      >>     --
>      >>
>      >>     Thanks,
>      >>     Jc
>      >
>      >
>      >
>      > --
>      >
>      > Thanks,
>      > Jc
> 
> 
> 
> -- 
> 
> Thanks,
> Jc


More information about the serviceability-dev mailing list