<html>
<head>
<meta content="text/html; charset=utf-8" http-equiv="Content-Type">
</head>
<body bgcolor="#FFFFFF" text="#000000">
<p><br>
</p>
<br>
<div class="moz-cite-prefix">On 5/3/2016 12:47 PM, Tony Printezis
wrote:<br>
</div>
<blockquote cite="mid:etPan.57290042.1d27cf4b.4be@tw-mbp-tprintezis"
type="cite">
<style>body{font-family:Helvetica,Arial;font-size:13px}</style>
<div id="bloop_customfont"
style="font-family:Helvetica,Arial;font-size:13px; color:
rgba(0,0,0,1.0); margin: 0px; line-height: auto;">Hi Jon,</div>
<div id="bloop_customfont"
style="font-family:Helvetica,Arial;font-size:13px; color:
rgba(0,0,0,1.0); margin: 0px; line-height: auto;"><br>
</div>
<div id="bloop_customfont"
style="font-family:Helvetica,Arial;font-size:13px; color:
rgba(0,0,0,1.0); margin: 0px; line-height: auto;">Thanks again
for evaluating the change. Yes, I’m expecting ParNew pause time
improvements with this change but only if a) there’s a
non-trivial amount of promotions during the ParNews and b) there
are more than a couple of GC threads available. For benchmarks
that run for a fixed amount of time, shorter pause times will of
course mean more GCs during the timing interval.</div>
<div id="bloop_customfont"
style="font-family:Helvetica,Arial;font-size:13px; color:
rgba(0,0,0,1.0); margin: 0px; line-height: auto;"><br>
</div>
<div id="bloop_customfont"
style="font-family:Helvetica,Arial;font-size:13px; color:
rgba(0,0,0,1.0); margin: 0px; line-height: auto;">Do you need
anything else from me?</div>
</blockquote>
<br>
Not at this point. I just need to look at some of the logs to be
sure there<br>
are no surprises.<br>
<br>
Jon<br>
<br>
<blockquote cite="mid:etPan.57290042.1d27cf4b.4be@tw-mbp-tprintezis"
type="cite">
<div id="bloop_customfont"
style="font-family:Helvetica,Arial;font-size:13px; color:
rgba(0,0,0,1.0); margin: 0px; line-height: auto;"><br>
</div>
<div id="bloop_customfont"
style="font-family:Helvetica,Arial;font-size:13px; color:
rgba(0,0,0,1.0); margin: 0px; line-height: auto;">Tony</div>
<br>
<p class="airmail_on">On May 3, 2016 at 2:07:57 PM, Jon Masamitsu
(<a moz-do-not-send="true"
href="mailto:jon.masamitsu@oracle.com">jon.masamitsu@oracle.com</a>)
wrote:</p>
<blockquote type="cite" class="clean_bq"><span>
<div bgcolor="#FFFFFF" text="#000000">
<div>
<title></title>
Performance results are back. If anything I see a
decrease in
ParNew pause times<br>
and an increase in the number of ParNew GC's. Is that the
expected results? Faster<br>
GC's so more time for the benchmark to make progress (so
more
allocations and more<br>
GC's)?<br>
<br>
Jon<br>
<br>
<div class="moz-cite-prefix">On 5/2/2016 2:59 PM, Tony
Printezis
wrote:<br>
</div>
<blockquote
cite="mid:etPan.5727cdd7.2a17fe53.4be@tw-mbp-tprintezis"
type="cite">
<div id="bloop_customfont"
style="font-family:Helvetica,Arial;font-size:13px;
color: rgba(0,0,0,1.0); margin: 0px; line-height:
auto;">
Sounds good, thanks Jon!!!</div>
<br>
<p class="airmail_on">On May 2, 2016 at 5:35:14 PM, Jon
Masamitsu
(<a moz-do-not-send="true"
href="mailto:jon.masamitsu@oracle.com">jon.masamitsu@oracle.com</a>)
wrote:</p>
<blockquote type="cite" class="clean_bq">
<div bgcolor="#FFFFFF" text="#000000">
<div><span><br>
<br>
</span>
<div class="moz-cite-prefix"><span>On 05/02/2016
08:02 AM, Tony
Printezis wrote:<br>
</span></div>
<blockquote
cite="mid:etPan.57276c01.248d3d4c.4be@tw-mbp-tprintezis"
type="cite">
<div id="bloop_customfont"
style="font-family:Helvetica,Arial;font-size:13px;
color: rgba(0,0,0,1.0); margin: 0px;
line-height: auto;">
<span>Jon,</span></div>
<div id="bloop_customfont"
style="font-family:Helvetica,Arial;font-size:13px;
color: rgba(0,0,0,1.0); margin: 0px;
line-height: auto;">
<span><br>
</span></div>
<div id="bloop_customfont"
style="font-family:Helvetica,Arial;font-size:13px;
color: rgba(0,0,0,1.0); margin: 0px;
line-height: auto;">
<span>I also didn’t like having to add a new
method to Generation
just for ParNew (but note that ParNew is the
only GC that uses the
existing
par_oop_since_save_marks_iterate_done()
method). On the
other hand, I also didn’t like having to
enable / disable something
in the epilogue / prologue unnecessarily
(i.e., when the GC is not
going to be done by ParNew).</span></div>
<div id="bloop_customfont"
style="font-family:Helvetica,Arial;font-size:13px;
color: rgba(0,0,0,1.0); margin: 0px;
line-height: auto;">
<span><br>
</span></div>
<div id="bloop_customfont"
style="font-family:Helvetica,Arial;font-size:13px;
color: rgba(0,0,0,1.0); margin: 0px;
line-height: auto;">
<span>Anyway, it looks as if the original fix
where we enable /
disable promotion tracking in the prologue /
epilogue seems the
least controversial. :-) So, let’s go with
that. Latest is
webrev.2. and I withdraw
webrev.3.suggestion.</span></div>
</blockquote>
<span><br>
I'll do some performance runs with .2 and push
if there are
no<br>
regressions.<br>
<br>
Jon<br>
<br>
</span>
<blockquote
cite="mid:etPan.57276c01.248d3d4c.4be@tw-mbp-tprintezis"
type="cite">
<div id="bloop_customfont"
style="font-family:Helvetica,Arial;font-size:13px;
color: rgba(0,0,0,1.0); margin: 0px;
line-height: auto;">
<span><br>
</span></div>
<div id="bloop_customfont"
style="font-family:Helvetica,Arial;font-size:13px;
color: rgba(0,0,0,1.0); margin: 0px;
line-height: auto;">
<span>Tony</span></div>
<span><br>
</span>
<p class="airmail_on"><span>On April 29, 2016 at
2:49:11 PM, Jon
Masamitsu (<a moz-do-not-send="true"
href="mailto:jon.masamitsu@oracle.com">jon.masamitsu@oracle.com</a>)
wrote:</span></p>
<blockquote type="cite" class="clean_bq">
<div bgcolor="#FFFFFF" text="#000000">
<div>
<p><span><span><br>
</span></span></p>
<span><br>
</span>
<div class="moz-cite-prefix"><span>On
4/29/2016 8:03 AM, Tony
Printezis wrote:<br>
</span></div>
<blockquote
cite="mid:etPan.572377b1.236ee83a.367@tw-mbp-tprintezis"
type="cite">
<div id="bloop_customfont"
style="font-family:Helvetica,Arial;font-size:13px;
color: rgba(0,0,0,1.0); margin: 0px;
line-height: auto;">
<span>Ramki and Jon,</span></div>
<div id="bloop_customfont"
style="font-family:Helvetica,Arial;font-size:13px;
color: rgba(0,0,0,1.0); margin: 0px;
line-height: auto;">
<span><br>
</span></div>
<div id="bloop_customfont"
style="font-family:Helvetica,Arial;font-size:13px;
color: rgba(0,0,0,1.0); margin: 0px;
line-height: auto;">
<span>I hate to confused things
further. :-) I just realized that
if only ParNew needs the promotion
tracking to be enabled for the
parallel workers, aren’t we better
off just letting ParNew enable /
disable it when it needs to instead
of always doing so in the
CMSGeneration prologue / epilogue?</span></div>
</blockquote>
<span><br>
Tony,<br>
<br>
I can see how nicely this works with the
implementation but adding
the new method<br>
par_oop_since_save_marks_iterate_start()
to Generation causes me
some concern.<br>
If there were another collector that
used
par_oop_since_save_marks_iterate_start(),<br>
it would make more sense to me.<br>
<br>
Going with your previous version where
the CMSGen epilogue/prologue
is<br>
responsible for this does make sense
from the point of view that it
is done<br>
by the generation that needs it.<br>
<br>
Alternatively, we could just all admit
that ParNew only works with
CMS and<br>
make the _old_gen in ParNewGeneration a
ConcurrentMarkSweepGeneration*<br>
and avoid adding the method to
Generation. This might not
work so easily with<br>
full GC's and ScavengeBeforeFullGC
though.<br>
<br>
I think this is more important for you
guys then for us so don't
hesitant to<br>
push for the solution you like.<br>
<br>
Jon<br>
</span>
<blockquote
cite="mid:etPan.572377b1.236ee83a.367@tw-mbp-tprintezis"
type="cite">
<div id="bloop_customfont"
style="font-family:Helvetica,Arial;font-size:13px;
color: rgba(0,0,0,1.0); margin: 0px;
line-height: auto;">
<span><br>
</span></div>
<div id="bloop_customfont"
style="font-family:Helvetica,Arial;font-size:13px;
color: rgba(0,0,0,1.0); margin: 0px;
line-height: auto;">
<span>Here’s an alternative webrev for
your
consideration:</span></div>
<div id="bloop_customfont"
style="font-family:Helvetica,Arial;font-size:13px;
color: rgba(0,0,0,1.0); margin: 0px;
line-height: auto;">
<span><br>
</span></div>
<div id="bloop_customfont"
style="font-family:Helvetica,Arial;font-size:13px;
color: rgba(0,0,0,1.0); margin: 0px;
line-height: auto;">
<span><a moz-do-not-send="true"
class="moz-txt-link-freetext"
href="http://cr.openjdk.java.net/%7Etonyp/8155257/webrev.3.suggestion/">http://cr.openjdk.java.net/~tonyp/8155257/webrev.3.suggestion/</a></span></div>
<div id="bloop_customfont"
style="font-family:Helvetica,Arial;font-size:13px;
color: rgba(0,0,0,1.0); margin: 0px;
line-height: auto;">
<span><br>
</span></div>
<div id="bloop_customfont"
style="font-family:Helvetica,Arial;font-size:13px;
color: rgba(0,0,0,1.0); margin: 0px;
line-height: auto;">
<span>I think I prefer this approach
but I’m OK with either. (And,
yes, I’ll actually expand the
comments I’ve marked as TODO if you
prefer this one.)</span></div>
<div id="bloop_customfont"
style="font-family:Helvetica,Arial;font-size:13px;
color: rgba(0,0,0,1.0); margin: 0px;
line-height: auto;">
<span><br>
</span></div>
<div id="bloop_customfont"
style="font-family:Helvetica,Arial;font-size:13px;
color: rgba(0,0,0,1.0); margin: 0px;
line-height: auto;">
<span>Tony</span></div>
<span><br>
</span>
<p class="airmail_on"><span>On April 29,
2016 at 10:13:27 AM, Tony
Printezis (<a moz-do-not-send="true"
href="mailto:tprintezis@twitter.com">tprintezis@twitter.com</a>)
wrote:</span></p>
<blockquote type="cite" class="clean_bq">
<div style="word-wrap: break-word;
-webkit-nbsp-mode: space;
-webkit-line-break:
after-white-space;">
<div>
<div id="bloop_customfont"
style="font-family:Helvetica,Arial;font-size:13px;
color: rgba(0,0,0,1.0); margin:
0px; line-height: auto;">
<span><span>Ramki,</span></span></div>
<div id="bloop_customfont"
style="font-family:Helvetica,Arial;font-size:13px;
color: rgba(0,0,0,1.0); margin:
0px; line-height: auto;">
<span><br>
</span></div>
<div id="bloop_customfont"
style="font-family:Helvetica,Arial;font-size:13px;
color: rgba(0,0,0,1.0); margin:
0px; line-height: auto;">
<span>Thanks for the feedback.
Latest webrev:</span></div>
<div id="bloop_customfont"
style="font-family:Helvetica,Arial;font-size:13px;
color: rgba(0,0,0,1.0); margin:
0px; line-height: auto;">
<span><br>
</span></div>
<div id="bloop_customfont"
style="font-family:Helvetica,Arial;font-size:13px;
color: rgba(0,0,0,1.0); margin:
0px; line-height: auto;">
<span><a moz-do-not-send="true"
href="http://cr.openjdk.java.net/%7Etonyp/8155257/webrev.2/">http://cr.openjdk.java.net/~tonyp/8155257/webrev.2/</a></span></div>
<div id="bloop_customfont"
style="font-family:Helvetica,Arial;font-size:13px;
color: rgba(0,0,0,1.0); margin:
0px; line-height: auto;">
<span><br>
</span></div>
<div id="bloop_customfont"
style="font-family:Helvetica,Arial;font-size:13px;
color: rgba(0,0,0,1.0); margin:
0px; line-height: auto;">
<span>I rephrased the three
comments in
concurrentMarkSweepGeneration.cpp before the calls to
{start,stop}TrackingPromotions(). The logic should be the
same.</span></div>
<div id="bloop_customfont"
style="font-family:Helvetica,Arial;font-size:13px;
color: rgba(0,0,0,1.0); margin:
0px; line-height: auto;">
<span><br>
</span></div>
<div id="bloop_customfont"
style="font-family:Helvetica,Arial;font-size:13px;
color: rgba(0,0,0,1.0); margin:
0px; line-height: auto;">
<span>Tony</span></div>
<span><br>
</span>
<p class="airmail_on"><span>On
April 28, 2016 at 9:48:26 PM,
Srinivas Ramakrishna (<a
moz-do-not-send="true"
class="moz-txt-link-abbreviated"
href="mailto:ysr1729@gmail.com"><a class="moz-txt-link-abbreviated" href="mailto:ysr1729@gmail.com">ysr1729@gmail.com</a></a>)
wrote:</span></p>
<blockquote type="cite"
class="clean_bq">
<div>
<div>
<div dir="ltr">
<div
style="font-size:12.8000001907349px"><span><span>(Resent
from
my gmail id: as my
twitter email id
isn't registered
with the
openjdk lists;
please pardon
duplicates.)</span></span></div>
<div
style="font-size:12.8000001907349px"><span><br>
</span></div>
<div
style="font-size:12.8000001907349px"><span>Looks
good to me
too. Minor remarks
regarding the
documentation
comments:</span></div>
<div
style="font-size:12.8000001907349px"><span><br>
</span></div>
<div
style="font-size:12.8000001907349px">
<pre style="white-space:pre-wrap;color:rgb(0,0,0)"><span><span style="color:blue;font-weight:bold">1098 // The par_oop_since_save_marks_iterate_done() method should be</span>
<span style="color:blue;font-weight:bold">1099 // called at the end of the main ParNew parallel phase to collapse</span>
<span style="color:blue;font-weight:bold">1100 // the promoted object lists. Given that we don't want promoted</span>
<span style="color:blue;font-weight:bold">1101 // objects to be tracked in future phases (e.g., during reference</span>
<span style="color:blue;font-weight:bold">1102 // processing) we disable promoted object tracking.</span>
</span>
</pre>
<pre style="white-space:pre-wrap;color:rgb(0,0,0)"><span style="color:blue;font-weight:bold">
</span>
</pre>
<pre style="white-space:pre-wrap;color:rgb(0,0,0)"><font face="arial, helvetica, sans-serif">Perhaps just say:</font>
</pre>
<pre style="white-space:pre-wrap;color:rgb(0,0,0)"><font face="arial, helvetica, sans-serif">// Because card-scanning has been completed, further promotions, if any,</font>
</pre>
<pre style="white-space:pre-wrap;color:rgb(0,0,0)"><font style="font-size:12.8000001907349px" face="arial, helvetica, sans-serif">// e.g. during reference processing, </font><span style="font-size:12.8000001907349px;font-family:arial,helvetica,sans-serif">will not need to keep track of promoted objects.</span>
</pre>
<pre style="white-space:pre-wrap;color:rgb(0,0,0)"><span style="font-family:arial,helvetica,sans-serif">Also instead of:</span>
</pre>
<pre style="white-space:pre-wrap;color:rgb(0,0,0)"><span style="color:blue;font-weight:bold">// Enable promotion tracking for the main parallel ParNew phase.</span>
</pre>
<pre style="white-space:pre-wrap"><font face="arial, helvetica, sans-serif">perhaps:</font>
</pre>
<pre style="white-space:pre-wrap"><font face="arial, helvetica, sans-serif">// We track promoted objects to allow card-scanning to skip them.</font>
</pre>
<pre style="white-space:pre-wrap"><font face="arial, helvetica, sans-serif">and instead of:</font>
</pre>
<pre style="white-space:pre-wrap"><font face="arial, helvetica, sans-serif">But leaving them as is fine too.</font>
</pre>
<pre style="white-space:pre-wrap"><font face="arial, helvetica, sans-serif">reviewed!</font>
</pre>
<pre style="white-space:pre-wrap"><span class=""><font color="#888888"><font face="arial, helvetica, sans-serif">-- ramki</font></font></span>
</pre>
<pre style="white-space:pre-wrap;color:rgb(0,0,0)"><span style="color:blue">2133 // Promotion tracking should be disabled at the end of the ParNew</span>
<span style="color:blue">2134 // parallel phase....</span>
</pre>
<pre style="white-space:pre-wrap"><font face="arial, helvetica, sans-serif" color="#000000">perhaps:</font>
</pre>
<pre style="white-space:pre-wrap"><font face="arial, helvetica, sans-serif" color="#000000">// When using ParNew, promoted object tracking would already have been disabled, however, ...</font>
</pre>
</div>
<div class="gmail_extra"><span
class=""><font
color="#888888"><br>
</font></span>
<div class="gmail_quote"><span
class=""><font
color="#888888">On
Thu, Apr 28, 2016
at 2:06 PM, Tony
Printezis <span
dir="ltr"><<a
moz-do-not-send="true" class="moz-txt-link-abbreviated"
href="mailto:tprintezis@twitter.com"><a class="moz-txt-link-abbreviated" href="mailto:tprintezis@twitter.com">tprintezis@twitter.com</a></a>></span>
wrote:<br>
</font></span>
<blockquote
class="gmail_quote"
style="margin:0 0 0
.8ex;border-left:1px
#ccc
solid;padding-left:1ex">
<div
style="word-wrap:break-word">
<div
style="font-family:Helvetica,Arial;font-size:13px;color:rgba(0,0,0,1.0);margin:0px;line-height:auto"><span
class="">Thanks
Jon! New
webrev here:</span></div>
<div
style="font-family:Helvetica,Arial;font-size:13px;color:rgba(0,0,0,1.0);margin:0px;line-height:auto"><span
class=""><br>
</span></div>
<div
style="font-family:Helvetica,Arial;font-size:13px;color:rgba(0,0,0,1.0);margin:0px;line-height:auto"><span
class=""><a
moz-do-not-send="true"
class="moz-txt-link-freetext"
href="http://cr.openjdk.java.net/%7Etonyp/8155257/webrev.1/"><a class="moz-txt-link-freetext" href="http://cr.openjdk.java.net/~tonyp/8155257/webrev.1/">http://cr.openjdk.java.net/~tonyp/8155257/webrev.1/</a></a></span></div>
<div
style="font-family:Helvetica,Arial;font-size:13px;color:rgba(0,0,0,1.0);margin:0px;line-height:auto"><span
class=""><br>
</span></div>
<div
style="font-family:Helvetica,Arial;font-size:13px;color:rgba(0,0,0,1.0);margin:0px;line-height:auto"><span
class="">Compared
to the
previous one,
I only changed
the
comments
before the two
calls to
stopTrackingPromotions().
The
logic should
be the same.
Will also run
another set of
tests
overnight. </span></div>
<div
style="font-family:Helvetica,Arial;font-size:13px;color:rgba(0,0,0,1.0);margin:0px;line-height:auto"><span
class=""><font
color="#888888"><br>
</font></span></div>
<div
style="font-family:Helvetica,Arial;font-size:13px;color:rgba(0,0,0,1.0);margin:0px;line-height:auto"><span
class=""><font
color="#888888">Tony</font></span></div>
<div>
<div class="h5"><br>
<p>On April
28, 2016 at
5:02:17 PM,
Jon Masamitsu
(<a
moz-do-not-send="true"
class="moz-txt-link-abbreviated" href="mailto:jon.masamitsu@oracle.com"><a class="moz-txt-link-abbreviated" href="mailto:jon.masamitsu@oracle.com">jon.masamitsu@oracle.com</a></a>)
wrote:</p>
<blockquote
type="cite">
<div
bgcolor="#FFFFFF"
text="#000000">
<div><span><br>
<br>
</span>
<div><span>On
04/28/2016
01:51 PM, Tony
Printezis
wrote:<br>
</span></div>
<blockquote
type="cite">
<div
style="font-family:Helvetica,Arial;font-size:13px;color:rgba(0,0,0,1.0);margin:0px;line-height:auto"><span>Jon
(CCing Ramki),</span></div>
<div
style="font-family:Helvetica,Arial;font-size:13px;color:rgba(0,0,0,1.0);margin:0px;line-height:auto"><span><br>
</span></div>
<div
style="font-family:Helvetica,Arial;font-size:13px;color:rgba(0,0,0,1.0);margin:0px;line-height:auto"><span>Re:
comment 2:
Actually, it
looks as if we
have to call
stopPromotionTracking in the epilogue. The prologue / epilogue are
called
irrespective
of the type of
GC. So they
are also
called for
Full GCs which
won’t call the
method to tear
down the
lists. I
think it’d be
safer to just
call
stopPromotionTracking()
in the
epilogue, as I
had it before,
with an
amended
comment?</span></div>
</blockquote>
<span><br>
That sounds
good. I like
the symmetry
of putting the
stopPromotionTracking in the epilogue. <br>
<br>
And yes, I'll
run some
addition tests
when the
latest patch
comes
out.<br>
<br>
Jon<br>
<br>
</span>
<blockquote
type="cite">
<div
style="font-family:Helvetica,Arial;font-size:13px;color:rgba(0,0,0,1.0);margin:0px;line-height:auto"><span><br>
</span></div>
<div
style="font-family:Helvetica,Arial;font-size:13px;color:rgba(0,0,0,1.0);margin:0px;line-height:auto"><span>Tony</span></div>
<span><br>
</span>
<p><span>On
April 28, 2016
at 4:33:44 PM,
Tony Printezis
(<a
moz-do-not-send="true"
class="moz-txt-link-abbreviated" href="mailto:tprintezis@twitter.com"><a class="moz-txt-link-abbreviated" href="mailto:tprintezis@twitter.com">tprintezis@twitter.com</a></a>)
wrote:</span></p>
<blockquote
type="cite">
<div
style="word-wrap:break-word">
<div>
<div
style="font-family:Helvetica,Arial;font-size:13px;color:rgba(0,0,0,1.0);margin:0px;line-height:auto"><span><span>Hi
Jon,</span></span></div>
<div
style="font-family:Helvetica,Arial;font-size:13px;color:rgba(0,0,0,1.0);margin:0px;line-height:auto"><span><br>
</span></div>
<div
style="font-family:Helvetica,Arial;font-size:13px;color:rgba(0,0,0,1.0);margin:0px;line-height:auto"><span>Thanks
for looking at
it! </span></div>
<div
style="font-family:Helvetica,Arial;font-size:13px;color:rgba(0,0,0,1.0);margin:0px;line-height:auto"><span><br>
</span></div>
<div
style="font-family:Helvetica,Arial;font-size:13px;color:rgba(0,0,0,1.0);margin:0px;line-height:auto"><span>comment
1 : “this
method” refers
to the
enclosing
method
(i.e.,
par_oop_since_save_marks_iterate_done()).
I’ll clarify.
(I
also spotted a
typo in the
comment! I’ll
fix that
too.)</span></div>
<div
style="font-family:Helvetica,Arial;font-size:13px;color:rgba(0,0,0,1.0);margin:0px;line-height:auto"><span><br>
</span></div>
<div
style="font-family:Helvetica,Arial;font-size:13px;color:rgba(0,0,0,1.0);margin:0px;line-height:auto"><span>comment
2 : You’re
right,
actually. Good
suggestion. I
thought it
might be
awkward
because of the
ParNew-CMS
interaction
going through
the generation
abstractions.
But in this
code I’m
already in the
CMS generation
and I can
access the
promoInfo
directly. So
I’ll just
assert that
tracking is
off and the
list is
empty.</span></div>
<div
style="font-family:Helvetica,Arial;font-size:13px;color:rgba(0,0,0,1.0);margin:0px;line-height:auto"><span><br>
</span></div>
<div
style="font-family:Helvetica,Arial;font-size:13px;color:rgba(0,0,0,1.0);margin:0px;line-height:auto"><span>I’ll
post a new
webrev in a
bit. FWIW,
overnight
testing
didn’t reveal
any issues
(and I’ll run
it again given
the “stop
tracking”
-> “sanity
checks”
change). Any
chance you can
also
run some tests
when I post
the new webrev
just in
case?</span></div>
<div
style="font-family:Helvetica,Arial;font-size:13px;color:rgba(0,0,0,1.0);margin:0px;line-height:auto"><span><br>
</span></div>
<div
style="font-family:Helvetica,Arial;font-size:13px;color:rgba(0,0,0,1.0);margin:0px;line-height:auto"><span>Tony</span></div>
<span><br>
</span>
<p><span>On
April 28, 2016
at 1:20:55 PM,
Jon Masamitsu
(<a
moz-do-not-send="true"
class="moz-txt-link-abbreviated" href="mailto:jon.masamitsu@oracle.com"><a class="moz-txt-link-abbreviated" href="mailto:jon.masamitsu@oracle.com">jon.masamitsu@oracle.com</a></a>)
wrote:</span></p>
<blockquote
type="cite">
<div
bgcolor="#FFFFFF"
text="#000000">
<div><span><span><font
face="Times
New Roman,
Times, serif">Tony,<br>
<br>
Changes look
good. Couple
of small
points.<br>
<br>
<a
moz-do-not-send="true"
class="moz-txt-link-freetext"
href="http://cr.openjdk.java.net/%7Etonyp/8155257/webrev.0/src/share/vm/gc/cms/concurrentMarkSweepGeneration.cpp.frames.html"></a><a
moz-do-not-send="true" class="moz-txt-link-freetext"
href="http://cr.openjdk.java.net/%7Etonyp/8155257/webrev.0/src/share/vm/gc/cms/concurrentMarkSweepGeneration.cpp.frames.html"><a class="moz-txt-link-freetext" href="http://cr.openjdk.java.net/~tonyp/8155257/webrev.0/src/share/vm/gc/cms/concurrentMarkSweepGeneration.cpp.frames.html">http://cr.openjdk.java.net/~tonyp/8155257/webrev.0/src/share/vm/gc/cms/concurrentMarkSweepGeneration.cpp.frames.html</a></a><br>
</font><br>
</span></span>
<pre><span><span>1097 // This method should be called at the end of the main ParNew</span>
<span>1098 // parallel phase to collapse the promoted object lists. Given that</span>
<span>1099 // we don't want promoted objects to be tracked in future phases</span>
<span>1100 // (e.g., during reference processing) we also disable promote</span>
<span>1101 // object tracking here.</span>
</span>
</pre>
<font
face="Times
New Roman,
Times, serif">The
placement of
this
block comment
was slightly
confusing. I
wasn't<br>
sure if "This
method ..."
applied
specirfically
to the
stopTrackingPromotions()<br>
until the last
part "also
disable
...". Would
it be
better placed
before the
method.<br>
</font><br>
<pre>2132 // Also reset promotion tracking in par gc thread states.
<span>2133 // I don't think this is really needed, as promotion tracking should</span>
<span>2134 // have already been disabled. However, it sanity checks that the</span>
<span>2135 // promotion lists are empty so I think it's helpful to leave it in.</span>
</pre>
The comment
makes clear
you intent
but is there
a simpler and
more direct<br>
sanity check
you can use?
Comments
sometimes get
lost in the
fog of code<br>
churn and
calling a
method just
for sanity
checkinf seems
wasteful
and confusing<br>
(Why is
stopTrackingPromotions()
call twice?
Is there
something that
the<br>
first all
didn't do?
Yes, you
comment
explains it
but I have to
read the
comment.)<br>
If you agree
you can fix it
at a later
time. If you
disagree,
I'll silently
forget about
it :-).<br>
<br>
Jon<br>
<br>
<div>On
04/27/2016
01:40 PM, Tony
Printezis
wrote:<br>
</div>
<blockquote
type="cite">
<div
style="font-family:Helvetica,Arial;font-size:13px;color:rgba(0,0,0,1.0);margin:0px;line-height:auto">Small
changes to
clean up when
promoted
object
tracking is
enabled
/ disabled and
when tearing
down the
promoted
object lists
is
done:</div>
<div
style="font-family:Helvetica,Arial;font-size:13px;color:rgba(0,0,0,1.0);margin:0px;line-height:auto"><br>
</div>
<div
style="font-family:Helvetica,Arial;font-size:13px;color:rgba(0,0,0,1.0);margin:0px;line-height:auto"><a
moz-do-not-send="true" class="moz-txt-link-freetext"
href="http://cr.openjdk.java.net/%7Etonyp/8155257/webrev.0/"><a class="moz-txt-link-freetext" href="http://cr.openjdk.java.net/~tonyp/8155257/webrev.0/">http://cr.openjdk.java.net/~tonyp/8155257/webrev.0/</a></a></div>
<div
style="font-family:Helvetica,Arial;font-size:13px;color:rgba(0,0,0,1.0);margin:0px;line-height:auto"><br>
</div>
<div
style="font-family:Helvetica,Arial;font-size:13px;color:rgba(0,0,0,1.0);margin:0px;line-height:auto">I
also had to
amend an
assert which
was too strong
and removed
the
worker_id
argument from
the
stopTrackingPromotions()
method as it
was actually
not used.</div>
<div
style="font-family:Helvetica,Arial;font-size:13px;color:rgba(0,0,0,1.0);margin:0px;line-height:auto"><br>
</div>
<div
style="font-family:Helvetica,Arial;font-size:13px;color:rgba(0,0,0,1.0);margin:0px;line-height:auto">I’ll
run more
testing
overnight.</div>
<div
style="font-family:Helvetica,Arial;font-size:13px;color:rgba(0,0,0,1.0);margin:0px;line-height:auto"><br>
</div>
<div
style="font-family:Helvetica,Arial;font-size:13px;color:rgba(0,0,0,1.0);margin:0px;line-height:auto">Tony</div>
<br>
<div>
<div
style="font-family:helvetica,arial;font-size:13px">
<div>-----</div>
<div><br>
</div>
<div>Tony
Printezis |
JVM/GC
Engineer / VM
Team | Twitter</div>
<div><br>
</div>
<div>@TonyPrintezis</div>
<div><a
moz-do-not-send="true"
class="moz-txt-link-abbreviated" href="mailto:tprintezis@twitter.com"><a class="moz-txt-link-abbreviated" href="mailto:tprintezis@twitter.com">tprintezis@twitter.com</a></a></div>
<div><br>
</div>
</div>
</div>
</blockquote>
<br>
</div>
</div>
</blockquote>
<div>
<div
style="font-family:helvetica,arial;font-size:13px">
<div>-----</div>
<div><br>
</div>
<div>Tony
Printezis |
JVM/GC
Engineer / VM
Team | Twitter</div>
<div><br>
</div>
<div>@TonyPrintezis</div>
<div><a
moz-do-not-send="true"
class="moz-txt-link-abbreviated" href="mailto:tprintezis@twitter.com"><a class="moz-txt-link-abbreviated" href="mailto:tprintezis@twitter.com">tprintezis@twitter.com</a></a></div>
<div><br>
</div>
</div>
</div>
</div>
</div>
</blockquote>
<div>
<div
style="font-family:helvetica,arial;font-size:13px">
<div>-----</div>
<div><br>
</div>
<div>Tony
Printezis |
JVM/GC
Engineer / VM
Team | Twitter</div>
<div><br>
</div>
<div>@TonyPrintezis</div>
<div><a
moz-do-not-send="true"
class="moz-txt-link-abbreviated" href="mailto:tprintezis@twitter.com"><a class="moz-txt-link-abbreviated" href="mailto:tprintezis@twitter.com">tprintezis@twitter.com</a></a></div>
<div><br>
</div>
</div>
</div>
</blockquote>
<br>
</div>
</div>
</blockquote>
<div>
<div
style="font-family:helvetica,arial;font-size:13px">
<div>-----</div>
<div><br>
</div>
<div>Tony
Printezis |
JVM/GC
Engineer / VM
Team | Twitter</div>
<div><br>
</div>
<div>@TonyPrintezis</div>
<div><a
moz-do-not-send="true"
class="moz-txt-link-abbreviated" href="mailto:tprintezis@twitter.com"><a class="moz-txt-link-abbreviated" href="mailto:tprintezis@twitter.com">tprintezis@twitter.com</a></a></div>
<div><br>
</div>
</div>
</div>
</div>
</div>
</div>
</blockquote>
</div>
<br>
</div>
</div>
</div>
</div>
</blockquote>
<div
id="bloop_sign_1461938962388339968"
class="bloop_sign">
<div
style="font-family:helvetica,arial;font-size:13px">
<div>-----</div>
<div><br>
</div>
<div>Tony Printezis | JVM/GC
Engineer / VM Team | Twitter</div>
<div><br>
</div>
<div>@TonyPrintezis</div>
<div><a moz-do-not-send="true"
href="mailto:tprintezis@twitter.com">tprintezis@twitter.com</a></div>
<div><br>
</div>
</div>
</div>
</div>
</div>
</blockquote>
<div id="bloop_sign_1461941952758243072"
class="bloop_sign">
<div
style="font-family:helvetica,arial;font-size:13px">
<div>-----</div>
<div><br>
</div>
<div>Tony Printezis | JVM/GC
Engineer / VM Team | Twitter</div>
<div><br>
</div>
<div>@TonyPrintezis</div>
<div><a moz-do-not-send="true"
href="mailto:tprintezis@twitter.com">tprintezis@twitter.com</a></div>
<div><br>
</div>
</div>
</div>
</blockquote>
<br>
</div>
</div>
</blockquote>
<div id="bloop_sign_1462200730444526848"
class="bloop_sign">
<div
style="font-family:helvetica,arial;font-size:13px">
<div>-----</div>
<div><br>
</div>
<div>Tony Printezis | JVM/GC Engineer / VM
Team | Twitter</div>
<div><br>
</div>
<div>@TonyPrintezis</div>
<div><a moz-do-not-send="true"
href="mailto:tprintezis@twitter.com">tprintezis@twitter.com</a></div>
<div><br>
</div>
</div>
</div>
</blockquote>
<br>
</div>
</div>
</blockquote>
<div id="bloop_sign_1462226381911459840"
class="bloop_sign">
<div
style="font-family:helvetica,arial;font-size:13px">
<div>-----</div>
<div><br>
</div>
<div>Tony Printezis | JVM/GC Engineer / VM Team |
Twitter</div>
<div><br>
</div>
<div>@TonyPrintezis</div>
<div><a moz-do-not-send="true"
href="mailto:tprintezis@twitter.com">tprintezis@twitter.com</a></div>
<div><br>
</div>
</div>
</div>
</blockquote>
<br>
</div>
</div>
</span></blockquote>
<div id="bloop_sign_1462304507952801024" class="bloop_sign">
<div style="font-family:helvetica,arial;font-size:13px">
<div>-----</div>
<div><br>
</div>
<div>Tony Printezis | JVM/GC Engineer / VM Team | Twitter</div>
<div><br>
</div>
<div>@TonyPrintezis</div>
<div><a moz-do-not-send="true"
href="mailto:tprintezis@twitter.com">tprintezis@twitter.com</a></div>
<div><br>
</div>
</div>
</div>
</blockquote>
<br>
</body>
</html>