RFR(S) 8207765: HeapMonitorIntervalRateTest fails with ZGC

[JC Beyler]

Hi Serguei and Alexey,

Thanks both and here you are:
http://cr.openjdk.java.net/~jcbeyler/8207765/webrev.02/

Let me know if you need anything else!
Jc

On Thu, Jul 19, 2018 at 5:21 PM serguei.spitsyn at oracle.com <
serguei.spitsyn at oracle.com> wrote:

> Thanks a lot, Alex!
>
> Jc,
>
> Could you please send me a patch for push?
>
> Thanks,
> Serguei
>
> On 7/19/18 17:06, Alex Menkov wrote:
> > Looks good.
> >
> > --alex
> >
> > On 07/19/2018 14:52, 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.
> >>
> >>
> >> 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/20180719/3399ac92/attachment.html>