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

Thu, 19 Jul 2018 14:34:05 -0700

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

Reply via email to