RFR(S) 8207765: HeapMonitorIntervalRateTest fails with ZGC

[Per Liden]

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/
> 
> 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> <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