RFR (S) 8212931 HeapMonitorStatIntervalTest.java fails due average calculation

classic Classic list List threaded Threaded
5 messages Options
Reply | Threaded
Open this post in threaded view
|

RFR (S) 8212931 HeapMonitorStatIntervalTest.java fails due average calculation

JC Beyler
Hi all,

Could I get a review for a bug in the calculation of the average size of an allocation? The solution is actually not to do the calculation at all. Instead, I just go in the cache and find an object with the right stacktrace and get its size (since all sizes should be equal because they are allocating every time 1-element arrays).

This removes the risk of a problem and simplifies the test.

Webrev: http://cr.openjdk.java.net/~jcbeyler/8212931/webrev.00/
Reply | Threaded
Open this post in threaded view
|

Re: RFR (S) 8212931 HeapMonitorStatIntervalTest.java fails due average calculation

JC Beyler
Hi all,

Any chance I can get a review on this (technically two reviews :-))?

Thanks,
Jc

On Fri, Nov 2, 2018 at 3:55 PM JC Beyler <[hidden email]> wrote:
Hi all,

Could I get a review for a bug in the calculation of the average size of an allocation? The solution is actually not to do the calculation at all. Instead, I just go in the cache and find an object with the right stacktrace and get its size (since all sizes should be equal because they are allocating every time 1-element arrays).

This removes the risk of a problem and simplifies the test.

Webrev: http://cr.openjdk.java.net/~jcbeyler/8212931/webrev.00/


--

Thanks,
Jc
Reply | Threaded
Open this post in threaded view
|

Re: RFR (S) 8212931 HeapMonitorStatIntervalTest.java fails due average calculation

serguei.spitsyn@oracle.com
Hi Jc,

Looks good.

One minor comment.

http://cr.openjdk.java.net/%7Ejcbeyler/8212931/webrev.00/test/hotspot/jtreg/serviceability/jvmti/HeapMonitor/MyPackage/HeapMonitor.java.frames.html

 The "average" in comments is not needed anymore:
 112   // Calculate the size of a 1-element array in order to assess average sampling interval
 113   // via the HeapMonitorStatIntervalTest.
 ...
 125     // Get the actual average size.
 126     oneElementSize = getSize(frames);
 127     System.out.println("Element size is: " + oneElementSize);

Thanks,
Serguei


On 11/9/18 19:42, JC Beyler wrote:
Hi all,

Any chance I can get a review on this (technically two reviews :-))?

Thanks,
Jc

On Fri, Nov 2, 2018 at 3:55 PM JC Beyler <[hidden email]> wrote:
Hi all,

Could I get a review for a bug in the calculation of the average size of an allocation? The solution is actually not to do the calculation at all. Instead, I just go in the cache and find an object with the right stacktrace and get its size (since all sizes should be equal because they are allocating every time 1-element arrays).

This removes the risk of a problem and simplifies the test.

Webrev: http://cr.openjdk.java.net/~jcbeyler/8212931/webrev.00/


--

Thanks,
Jc

Reply | Threaded
Open this post in threaded view
|

Re: RFR (S) 8212931 HeapMonitorStatIntervalTest.java fails due average calculation

JC Beyler
Thanks Serguei, I fixed that in my local version, could I get a second review from someone?

Thanks,
Jc

On Sat, Nov 10, 2018 at 3:24 PM [hidden email] <[hidden email]> wrote:
Hi Jc,

Looks good.

One minor comment.

http://cr.openjdk.java.net/%7Ejcbeyler/8212931/webrev.00/test/hotspot/jtreg/serviceability/jvmti/HeapMonitor/MyPackage/HeapMonitor.java.frames.html

 The "average" in comments is not needed anymore:
 112   // Calculate the size of a 1-element array in order to assess average sampling interval
 113   // via the HeapMonitorStatIntervalTest.
 ...
 125     // Get the actual average size.
 126     oneElementSize = getSize(frames);
 127     System.out.println("Element size is: " + oneElementSize);

Thanks,
Serguei


On 11/9/18 19:42, JC Beyler wrote:
Hi all,

Any chance I can get a review on this (technically two reviews :-))?

Thanks,
Jc

On Fri, Nov 2, 2018 at 3:55 PM JC Beyler <[hidden email]> wrote:
Hi all,

Could I get a review for a bug in the calculation of the average size of an allocation? The solution is actually not to do the calculation at all. Instead, I just go in the cache and find an object with the right stacktrace and get its size (since all sizes should be equal because they are allocating every time 1-element arrays).

This removes the risk of a problem and simplifies the test.

Webrev: http://cr.openjdk.java.net/~jcbeyler/8212931/webrev.00/


--

Thanks,
Jc



--

Thanks,
Jc
Reply | Threaded
Open this post in threaded view
|

Re: RFR (S) 8212931 HeapMonitorStatIntervalTest.java fails due average calculation

Alex Menkov-2
LGTM

--alex

On 11/15/2018 08:27, JC Beyler wrote:

> Thanks Serguei, I fixed that in my local version, could I get a second
> review from someone?
>
> Thanks,
> Jc
>
> On Sat, Nov 10, 2018 at 3:24 PM [hidden email]
> <mailto:[hidden email]> <[hidden email]
> <mailto:[hidden email]>> wrote:
>
>     Hi Jc,
>
>     Looks good.
>
>     One minor comment.
>
>     http://cr.openjdk.java.net/%7Ejcbeyler/8212931/webrev.00/test/hotspot/jtreg/serviceability/jvmti/HeapMonitor/MyPackage/HeapMonitor.java.frames.html
>
>       The "average" in comments is not needed anymore:
>
>       112   // Calculate the size of a 1-element array in order to assess average sampling interval
>     113 // via the HeapMonitorStatIntervalTest.
>       ...
>
>       125     // Get the actual average size.
>     126 oneElementSize = getSize(frames);
>     127 System.out.println("Element size is: " + oneElementSize);
>
>
>     Thanks,
>     Serguei
>
>
>     On 11/9/18 19:42, JC Beyler wrote:
>>     Hi all,
>>
>>     Any chance I can get a review on this (technically two reviews :-))?
>>
>>     Thanks,
>>     Jc
>>
>>     On Fri, Nov 2, 2018 at 3:55 PM JC Beyler <[hidden email]
>>     <mailto:[hidden email]>> wrote:
>>
>>         Hi all,
>>
>>         Could I get a review for a bug in the calculation of the
>>         average size of an allocation? The solution is actually not to
>>         do the calculation at all. Instead, I just go in the cache and
>>         find an object with the right stacktrace and get its size
>>         (since all sizes should be equal because they are allocating
>>         every time 1-element arrays).
>>
>>         This removes the risk of a problem and simplifies the test.
>>
>>         Webrev:
>>         http://cr.openjdk.java.net/~jcbeyler/8212931/webrev.00/
>>         <http://cr.openjdk.java.net/%7Ejcbeyler/8212931/webrev.00/>
>>         Bug: https://bugs.openjdk.java.net/browse/JDK-8212931?filter=-1
>>
>>         Thanks,
>>         Jc
>>
>>
>>
>>     --
>>
>>     Thanks,
>>     Jc
>
>
>
> --
>
> Thanks,
> Jc