Discussion:
RFR (S) 8212931 HeapMonitorStatIntervalTest.java fails due average calculation
(too old to reply)
JC Beyler
2018-11-02 22:55:11 UTC
Permalink
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/
Bug: https://bugs.openjdk.java.net/browse/JDK-8212931?filter=-1

Thanks,
Jc
JC Beyler
2018-11-10 03:42:03 UTC
Permalink
Hi all,

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

Thanks,
Jc
Post by 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/
Bug: https://bugs.openjdk.java.net/browse/JDK-8212931?filter=-1
Thanks,
Jc
--
Thanks,
Jc
s***@oracle.com
2018-11-10 23:24:51 UTC
Permalink
<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=utf-8">
</head>
<body text="#000000" bgcolor="#FFFFFF">
<div class="moz-cite-prefix">Hi Jc,<br>
<br>
Looks good.<br>
<br>
One minor comment.<br>
<br>
<a class="moz-txt-link-freetext" href="http://cr.openjdk.java.net/%7Ejcbeyler/8212931/webrev.00/test/hotspot/jtreg/serviceability/jvmti/HeapMonitor/MyPackage/HeapMonitor.java.frames.html">http://cr.openjdk.java.net/%7Ejcbeyler/8212931/webrev.00/test/hotspot/jtreg/serviceability/jvmti/HeapMonitor/MyPackage/HeapMonitor.java.frames.html</a><br>
<br>
 The "average" in comments is not needed anymore:<br>
<pre> 112 // Calculate the size of a 1-element array in order to assess average sampling interval
<span class="changed"> 113 // via the HeapMonitorStatIntervalTest.</span>
...
</pre>
<pre> 125 // Get the actual average size.
<span class="changed"> 126 oneElementSize = getSize(frames);</span>
<span class="changed"> 127 System.out.println("Element size is: " + oneElementSize);</span></pre>
<br>
Thanks,<br>
Serguei<br>
<br>
<br>
On 11/9/18 19:42, JC Beyler wrote:<br>
</div>
<blockquote type="cite"
cite="mid:***@mail.gmail.com">
<div dir="ltr">Hi all,<br>
<div><br>
</div>
<div>Any chance I can get a review on this (technically two
reviews :-))?</div>
<div><br>
</div>
<div>Thanks,</div>
<div>Jc</div>
</div>
<br>
<div class="gmail_quote">
<div dir="ltr">On Fri, Nov 2, 2018 at 3:55 PM JC Beyler &lt;<a
href="mailto:***@google.com" moz-do-not-send="true">***@google.com</a>&gt;
wrote:<br>
</div>
<blockquote class="gmail_quote">
<div dir="ltr">
<div dir="ltr">
<div dir="ltr">Hi all,
<div><br>
</div>
<div>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).</div>
<div><br>
</div>
<div>This removes the risk of a problem and simplifies
the test.<br>
<div><br>
</div>
Webrev: <a
href="http://cr.openjdk.java.net/%7Ejcbeyler/8212931/webrev.00/"
target="_blank" moz-do-not-send="true">http://cr.openjdk.java.net/~jcbeyler/8212931/webrev.00/</a></div>
<div>Bug: <a
href="https://bugs.openjdk.java.net/browse/JDK-8212931?filter=-1"
target="_blank" moz-do-not-send="true">https://bugs.openjdk.java.net/browse/JDK-8212931?filter=-1</a><br>
<div dir="ltr"
class="m_2535327701763435043gmail_signature">
<div dir="ltr">
<div><br>
</div>
Thanks,
<div>Jc</div>
</div>
</div>
</div>
</div>
</div>
</div>
</blockquote>
</div>
<br>
<div><br>
</div>
-- <br>
<div dir="ltr" class="gmail_signature"
data-smartmail="gmail_signature">
<div dir="ltr">
<div><br>
</div>
Thanks,
<div>Jc</div>
</div>
</div>
</blockquote>
<br>
</body>
</html>
JC Beyler
2018-11-15 16:27:41 UTC
Permalink
Thanks Serguei, I fixed that in my local version, could I get a second
review from someone?

Thanks,
Jc
Post by s***@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
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
Hi all,
Any chance I can get a review on this (technically two reviews :-))?
Thanks,
Jc
Post by 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/
Bug: https://bugs.openjdk.java.net/browse/JDK-8212931?filter=-1
Thanks,
Jc
--
Thanks,
Jc
--
Thanks,
Jc
Alex Menkov
2018-11-16 01:06:36 UTC
Permalink
LGTM

--alex
Post by JC Beyler
Thanks Serguei, I fixed that in my local version, could I get a second
review from someone?
Thanks,
Jc
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
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
Post by JC Beyler
Hi all,
Any chance I can get a review on this (technically two reviews :-))?
Thanks,
Jc
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.
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
Loading...