[prev in list] [next in list] [prev in thread] [next in thread]
List: openjdk-serviceability-dev
Subject: Re: RFR(S) 8215113: Sampling interval not always correct
From: Jean Christophe Beyler <jcbeyler () google ! com>
Date: 2019-01-29 17:21:13
Message-ID: CAF9BGByMWH_QyauagEOjP1A56MP+AmuCNA+03gdJWUacWr=BFw () mail ! gmail ! com
[Download RAW message or body]
Hi Serguei,
Sorry it took so long to answer, I just got submit repo to work with it. It
passes there, I think that should be enough for now but we could do a mach5
testing if you prefer (and could you/someone do it please?)
I also opened https://bugs.openjdk.java.net/browse/JDK-8217267 to really
study the bias and accuracy of the sampling a bit more. So this might get
revisited in the near future but this particular webrev is "important" as
it fixes the tests to be doing what we want them to do.
Let me know what you think,
Jc
On Tue, Jan 22, 2019 at 1:14 PM <serguei.spitsyn@oracle.com> wrote:
>
>
> On 1/22/19 12:54 PM, serguei.spitsyn@oracle.com wrote:
>
> Hi Jc,
>
> It looks good to me too.
>
>
> Forgot to say that I hope it was tested well.
>
> Thanks,
> Serguei
>
>
> Thanks,
> Serguei
>
>
> On 1/11/19 5:07 PM, Hohensee, Paul wrote:
>
> Nits:
>
>
>
> memAllocator.cpp:
>
>
>
> Copyright line should read
>
>
>
> * Copyright (c) 2018, 2019, Oracle and/or its affiliates. All rights
> reserved.
>
>
>
> The comment
>
>
>
> // Tell tlab to forget the bytes_since if we passed it to the heap sampler.
>
>
>
> would be clearer if it read
>
>
>
> // Tell tlab to forget bytes_since_last if we passed it to the heap sampler.
>
> Same copyright line comment for HeapMonitor.java,
> HeapMonitorArrayAllSampledTest.java and HeapMonitorStatArrayCorrectnessTest.java
> as for memAllocator.cpp.
>
> Otherwise lgtm, no need for another webrev on my account.
>
>
> Thanks,
>
>
>
> Paul
>
>
>
> *From: *serviceability-dev <serviceability-dev-bounces@openjdk.java.net>
> <serviceability-dev-bounces@openjdk.java.net> on behalf of JC Beyler
> <jcbeyler@google.com> <jcbeyler@google.com>
> *Date: *Thursday, January 10, 2019 at 11:28 AM
> *To: *OpenJDK Serviceability <serviceability-dev@openjdk.java.net>
> <serviceability-dev@openjdk.java.net>
> *Subject: *RFR(S) 8215113: Sampling interval not always correct
>
>
>
> Hi all,
>
>
>
> Could I get a review for this:
>
> Webrev: http://cr.openjdk.java.net/~jcbeyler/8215113/webrev.00/
>
> Bug: https://bugs.openjdk.java.net/browse/JDK-8215113
>
>
>
> This fixes the code to do the right accounting in sampling and get the
> right sample counts and sampled objects. My error percentage calculation
> was wrong so the two tests I had added were false positives (my apologies).
>
>
>
> Thanks,
>
> Jc
>
>
>
>
--
Thanks,
Jc
[Attachment #3 (text/html)]
<div dir="ltr"><div dir="ltr">Hi Serguei,<br><div><br></div><div>Sorry it took so \
long to answer, I just got submit repo to work with it. It passes there, I think that \
should be enough for now but we could do a mach5 testing if you prefer (and could \
you/someone do it please?)</div><div><br></div><div>I also opened <a \
href="https://bugs.openjdk.java.net/browse/JDK-8217267">https://bugs.openjdk.java.net/browse/JDK-8217267</a> \
to really study the bias and accuracy of the sampling a bit more. So this might get \
revisited in the near future but this particular webrev is "important" as \
it fixes the tests to be doing what we want them to do.</div><div><br></div><div>Let \
me know what you think,</div><div>Jc</div></div></div><br><div \
class="gmail_quote"><div dir="ltr" class="gmail_attr">On Tue, Jan 22, 2019 at 1:14 PM \
<<a href="mailto:serguei.spitsyn@oracle.com">serguei.spitsyn@oracle.com</a>> \
wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px \
0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<div bgcolor="#FFFFFF">
<br>
<br>
<div class="gmail-m_3485049885793048121moz-cite-prefix">On 1/22/19 12:54 PM,
<a class="gmail-m_3485049885793048121moz-txt-link-abbreviated" \
href="mailto:serguei.spitsyn@oracle.com" \
target="_blank">serguei.spitsyn@oracle.com</a> wrote:<br> </div>
<blockquote type="cite">
Hi Jc,<br>
<br>
It looks good to me too.<br>
</blockquote>
<br>
Forgot to say that I hope it was tested well.<br>
<br>
Thanks,<br>
Serguei<br>
<br>
<blockquote type="cite"> <br>
Thanks,<br>
Serguei<br>
<br>
<br>
<div class="gmail-m_3485049885793048121moz-cite-prefix">On 1/11/19 5:07 PM, \
Hohensee, Paul wrote:<br>
</div>
<blockquote type="cite">
<div class="gmail-m_3485049885793048121WordSection1">
<p class="MsoNormal">Nits:<u></u><u></u></p>
<p class="MsoNormal"><u></u> <u></u></p>
<p class="MsoNormal">memAllocator.cpp:<u></u><u></u></p>
<p class="MsoNormal"><u></u> <u></u></p>
<p class="MsoNormal">Copyright line should read<u></u><u></u></p>
<p class="MsoNormal"><u></u> <u></u></p>
<p class="MsoNormal"><span>* Copyright (c) 2018, 2019, Oracle and/or its
affiliates. All rights reserved.<u></u><u></u></span></p>
<p class="MsoNormal"><u></u> <u></u></p>
<p class="MsoNormal">The comment<u></u><u></u></p>
<p class="MsoNormal"><u></u> <u></u></p>
<pre><span class="gmail-m_3485049885793048121new">// Tell tlab to forget \
the bytes_since if we passed it to the heap sampler.</span><u></u><u></u></pre> <p \
class="MsoNormal"><u></u> <u></u></p>
<p class="MsoNormal">would be clearer if it read<u></u><u></u></p>
<p class="MsoNormal"><u></u> <u></u></p>
<pre><span class="gmail-m_3485049885793048121new">// Tell tlab to forget \
bytes_since_last if we passed it to the heap sampler.<u></u><u></u></span></pre> \
<h2><span style="font-size:11pt;font-weight:normal">Same copyright line comment for \
HeapMonitor.java,
HeapMonitorArrayAllSampledTest.java and </span><span \
style="font-size:11pt;font-weight:normal">HeapMonitorStatArrayCorrectnessTest.java \
as for memAllocator.cpp.<u></u><u></u></span></h2> <p class="MsoNormal">Otherwise \
lgtm, no need for another webrev on my account.<u></u><u></u></p>
<p class="MsoNormal"><br>
Thanks,<u></u><u></u></p>
<p class="MsoNormal"><u></u> <u></u></p>
<p class="MsoNormal">Paul<u></u><u></u></p>
<p class="MsoNormal"><u></u> <u></u></p>
<div style="border-right:none;border-bottom:none;border-left:none;border-top:1pt \
solid rgb(181,196,223);padding:3pt 0in 0in">
<p class="MsoNormal"><b><span style="font-size:12pt;color:black">From: \
</span></b><span style="font-size:12pt;color:black">serviceability-dev <a \
class="gmail-m_3485049885793048121moz-txt-link-rfc2396E" \
href="mailto:serviceability-dev-bounces@openjdk.java.net" \
target="_blank"><serviceability-dev-bounces@openjdk.java.net></a> on behalf of \
JC Beyler <a class="gmail-m_3485049885793048121moz-txt-link-rfc2396E" \
href="mailto:jcbeyler@google.com" target="_blank"><jcbeyler@google.com></a><br> \
<b>Date: </b>Thursday, January 10, 2019 at 11:28 AM<br> <b>To: </b>OpenJDK \
Serviceability <a class="gmail-m_3485049885793048121moz-txt-link-rfc2396E" \
href="mailto:serviceability-dev@openjdk.java.net" \
target="_blank"><serviceability-dev@openjdk.java.net></a><br> <b>Subject: \
</b>RFR(S) 8215113: Sampling interval not always correct<u></u><u></u></span></p>
</div>
<div>
<p class="MsoNormal"><u></u> <u></u></p>
</div>
<div>
<div>
<div>
<div>
<p class="MsoNormal">Hi all,<u></u><u></u></p>
</div>
<div>
<p class="MsoNormal"><u></u> <u></u></p>
</div>
<div>
<p class="MsoNormal">Could I get a review for \
this:<u></u><u></u></p> </div>
<div>
<p class="MsoNormal">Webrev: <a \
href="http://cr.openjdk.java.net/%7Ejcbeyler/8215113/webrev.00/" \
target="_blank">http://cr.openjdk.java.net/~jcbeyler/8215113/webrev.00/</a><u></u><u></u></p>
<div>
<p class="MsoNormal">Bug: <a \
href="https://bugs.openjdk.java.net/browse/JDK-8215113" \
target="_blank">https://bugs.openjdk.java.net/browse/JDK-8215113</a><u></u><u></u></p>
</div>
<div>
<div>
<div>
<p class="MsoNormal"><u></u> <u></u></p>
</div>
<div>
<p class="MsoNormal">This fixes the code to do
the right accounting in sampling and get the
right sample counts and sampled objects. My
error percentage calculation was wrong so the
two tests I had added were false positives (my
apologies).<u></u><u></u></p>
</div>
<div>
<p class="MsoNormal"><u></u> <u></u></p>
</div>
<p class="MsoNormal">Thanks, <u></u><u></u></p>
<div>
<p class="MsoNormal">Jc<u></u><u></u></p>
</div>
</div>
</div>
</div>
</div>
</div>
</div>
</div>
</blockquote>
<br>
</blockquote>
<br>
</div>
</blockquote></div><br clear="all"><div><br></div>-- <br><div dir="ltr" \
class="gmail_signature"><div \
dir="ltr"><div><br></div>Thanks,<div>Jc</div></div></div>
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic