Hi JC,
On 2018-08-08 20:07, JC Beyler wrote:
Hi Per,
You are correct, my test shows that an int[1] will use 32 bytes
with -XX:-UseCompressedClassPointers under G1. My apologies for not
digging more into why with ZGC :-)
In any case, my test was fragile since using that flag would break it,
the fix was not a ZGC only fix, I basically made it first calculate the
size and then use that for the test. So, it is at least a better test
and should be stable now (famous last words).
Sounds good.
Thanks for pointing out the actual reason why there were 8 extra bytes!
No problem.
cheers,
Per
Jc
On Wed, Aug 8, 2018 at 6:19 AM Per Liden <per.li...@oracle.com
<mailto:per.li...@oracle.com>> wrote:
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/
<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.
>
>
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.spit...@oracle.com
<mailto:serguei.spit...@oracle.com>
> <mailto:serguei.spit...@oracle.com
<mailto:serguei.spit...@oracle.com>> <serguei.spit...@oracle.com
<mailto:serguei.spit...@oracle.com>
> <mailto: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>
>> <mailto: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>
>> <mailto: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/>
>>>
<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/>
>>>
<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
