RFR(S) 8207765: HeapMonitorIntervalRateTest fails with ZGC
JC Beyler
jcbeyler at google.com
Wed Aug 8 18:07:42 UTC 2018
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).
Thanks for pointing out the actual reason why there were 8 extra bytes!
Jc
On Wed, Aug 8, 2018 at 6:19 AM Per Liden <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/
> >
> > 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> <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>> 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>> 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/
> >
> >>>
> >>> 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/
> >
> >>> Bug: https://bugs.openjdk.java.net/browse/JDK-8207765
> >>>
> >>> Thanks!
> >>> Jc
> >>
> >>
> >>
> >> --
> >>
> >> Thanks,
> >> Jc
> >>
> >>
> >>
> >> --
> >>
> >> Thanks,
> >> Jc
> >
> >
> >
> > --
> >
> > Thanks,
> > Jc
>
--
Thanks,
Jc
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/serviceability-dev/attachments/20180808/dc6bb03d/attachment-0001.html>
More information about the serviceability-dev
mailing list