Re: RFR(S) 8207765: HeapMonitorIntervalRateTest fails with ZGC

Thu, 19 Jul 2018 15:21:02 -0700

Hi Jc,

Looks good.
Thank you for the update!

Thanks,
Serguei

On 7/19/18 14:52, JC Beyler wrote:
Hi Serguei,

Done here:

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.spit...@oracle.com <serguei.spit...@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:

It got renamed in jdk11 via:

Thanks!
Jc

On Thu, Jul 19, 2018 at 10:07 AM JC Beyler <jcbey...@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.daughe...@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:

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.




--

Thanks,
Jc


--

Thanks,
Jc



--

Thanks,
Jc

Reply via email to