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