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