<html><head><meta http-equiv="Content-Type" content="text/html charset=utf-8"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class="">Hi Jon,<div class=""><br class=""></div><div class="">Are you able to calculate allocation rates?</div><div class=""><br class=""></div><div class="">Kind regards,</div><div class="">Kirk</div><div class=""><br class=""><div><blockquote type="cite" class=""><div class="">On May 3, 2016, at 8:07 PM, Jon Masamitsu <<a href="mailto:jon.masamitsu@oracle.com" class="">jon.masamitsu@oracle.com</a>> wrote:</div><br class="Apple-interchange-newline"><div class=""><span style="font-family: Helvetica, Arial; font-size: 13px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; background-color: rgb(255, 255, 255); float: none; display: inline !important;" class="">Performance results are back. If anything I see a decrease in ParNew pause times</span><br style="font-family: Helvetica, Arial; font-size: 13px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; background-color: rgb(255, 255, 255);" class=""><span style="font-family: Helvetica, Arial; font-size: 13px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; background-color: rgb(255, 255, 255); float: none; display: inline !important;" class="">and an increase in the number of ParNew GC's. Is that the expected results? Faster</span><br style="font-family: Helvetica, Arial; font-size: 13px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; background-color: rgb(255, 255, 255);" class=""><span style="font-family: Helvetica, Arial; font-size: 13px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; background-color: rgb(255, 255, 255); float: none; display: inline !important;" class="">GC's so more time for the benchmark to make progress (so more allocations and more</span><br style="font-family: Helvetica, Arial; font-size: 13px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; background-color: rgb(255, 255, 255);" class=""><span style="font-family: Helvetica, Arial; font-size: 13px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; background-color: rgb(255, 255, 255); float: none; display: inline !important;" class="">GC's)?</span><br style="font-family: Helvetica, Arial; font-size: 13px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; background-color: rgb(255, 255, 255);" class=""><br style="font-family: Helvetica, Arial; font-size: 13px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; background-color: rgb(255, 255, 255);" class=""><span style="font-family: Helvetica, Arial; font-size: 13px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; background-color: rgb(255, 255, 255); float: none; display: inline !important;" class="">Jon</span><br style="font-family: Helvetica, Arial; font-size: 13px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; background-color: rgb(255, 255, 255);" class=""><br style="font-family: Helvetica, Arial; font-size: 13px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; background-color: rgb(255, 255, 255);" class=""><div class="moz-cite-prefix" style="font-family: Helvetica, Arial; font-size: 13px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; background-color: rgb(255, 255, 255);">On 5/2/2016 2:59 PM, Tony Printezis wrote:<br class=""></div><blockquote cite="mid:etPan.5727cdd7.2a17fe53.4be@tw-mbp-tprintezis" type="cite" style="font-family: Helvetica, Arial; font-size: 13px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; background-color: rgb(255, 255, 255);" class=""><div id="bloop_customfont" style="font-family: Helvetica, Arial; font-size: 13px; margin: 0px;" class="">Sounds good, thanks Jon!!!</div><br class=""><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" class="">jon.masamitsu@oracle.com</a>) wrote:</p><blockquote type="cite" class="clean_bq"><span class=""><div bgcolor="#FFFFFF" text="#000000" class=""><div class=""><br class=""><br class=""><div class="moz-cite-prefix">On 05/02/2016 08:02 AM, Tony Printezis wrote:<br class=""></div><blockquote cite="mid:etPan.57276c01.248d3d4c.4be@tw-mbp-tprintezis" type="cite" class=""><div id="bloop_customfont" style="font-family: Helvetica, Arial; font-size: 13px; margin: 0px;" class="">Jon,</div><div id="bloop_customfont" style="font-family: Helvetica, Arial; font-size: 13px; margin: 0px;" class=""><br class=""></div><div id="bloop_customfont" style="font-family: Helvetica, Arial; font-size: 13px; margin: 0px;" class="">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).</div><div id="bloop_customfont" style="font-family: Helvetica, Arial; font-size: 13px; margin: 0px;" class=""><br class=""></div><div id="bloop_customfont" style="font-family: Helvetica, Arial; font-size: 13px; margin: 0px;" class="">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.</div></blockquote><br class="">I'll do some performance runs with .2 and push if there are no<br class="">regressions.<br class=""><br class="">Jon<br class=""><br class=""><blockquote cite="mid:etPan.57276c01.248d3d4c.4be@tw-mbp-tprintezis" type="cite" class=""><div id="bloop_customfont" style="font-family: Helvetica, Arial; font-size: 13px; margin: 0px;" class=""><br class=""></div><div id="bloop_customfont" style="font-family: Helvetica, Arial; font-size: 13px; margin: 0px;" class="">Tony</div><br class=""><p class="airmail_on">On April 29, 2016 at 2:49:11 PM, Jon Masamitsu (<a moz-do-not-send="true" href="mailto:jon.masamitsu@oracle.com" class="">jon.masamitsu@oracle.com</a>) wrote:</p><blockquote type="cite" class="clean_bq"><div bgcolor="#FFFFFF" text="#000000" class=""><div class=""><p class=""><span class=""><br class=""></span></p><span class=""><br class=""></span><div class="moz-cite-prefix"><span class="">On 4/29/2016 8:03 AM, Tony Printezis wrote:<br class=""></span></div><blockquote cite="mid:etPan.572377b1.236ee83a.367@tw-mbp-tprintezis" type="cite" class=""><div id="bloop_customfont" style="font-family: Helvetica, Arial; font-size: 13px; margin: 0px;" class=""><span class="">Ramki and Jon,</span></div><div id="bloop_customfont" style="font-family: Helvetica, Arial; font-size: 13px; margin: 0px;" class=""><span class=""><br class=""></span></div><div id="bloop_customfont" style="font-family: Helvetica, Arial; font-size: 13px; margin: 0px;" class=""><span class="">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 class=""><br class="">Tony,<br class=""><br class="">I can see how nicely this works with the implementation but adding the new method<br class="">par_oop_since_save_marks_iterate_start() to Generation causes me some concern.<br class="">If there were another collector that used par_oop_since_save_marks_iterate_start(),<br class="">it would make more sense to me.<br class=""><br class="">Going with your previous version where the CMSGen epilogue/prologue is<br class="">responsible for this does make sense from the point of view that it is done<br class="">by the generation that needs it.<br class=""><br class="">Alternatively, we could just all admit that ParNew only works with CMS and<br class="">make the _old_gen in ParNewGeneration a ConcurrentMarkSweepGeneration*<br class="">and avoid adding the method to Generation. This might not work so easily with<br class="">full GC's and ScavengeBeforeFullGC though.<br class=""><br class="">I think this is more important for you guys then for us so don't hesitant to<br class="">push for the solution you like.<br class=""><br class="">Jon<br class=""></span><blockquote cite="mid:etPan.572377b1.236ee83a.367@tw-mbp-tprintezis" type="cite" class=""><div id="bloop_customfont" style="font-family: Helvetica, Arial; font-size: 13px; margin: 0px;" class=""><span class=""><br class=""></span></div><div id="bloop_customfont" style="font-family: Helvetica, Arial; font-size: 13px; margin: 0px;" class=""><span class="">Here’s an alternative webrev for your consideration:</span></div><div id="bloop_customfont" style="font-family: Helvetica, Arial; font-size: 13px; margin: 0px;" class=""><span class=""><br class=""></span></div><div id="bloop_customfont" style="font-family: Helvetica, Arial; font-size: 13px; margin: 0px;" class=""><span class=""><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; margin: 0px;" class=""><span class=""><br class=""></span></div><div id="bloop_customfont" style="font-family: Helvetica, Arial; font-size: 13px; margin: 0px;" class=""><span class="">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; margin: 0px;" class=""><span class=""><br class=""></span></div><div id="bloop_customfont" style="font-family: Helvetica, Arial; font-size: 13px; margin: 0px;" class=""><span class="">Tony</span></div><span class=""><br class=""></span><p class="airmail_on"><span class="">On April 29, 2016 at 10:13:27 AM, Tony Printezis (<a moz-do-not-send="true" href="mailto:tprintezis@twitter.com" class="">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;" class=""><div class=""><div id="bloop_customfont" style="font-family: Helvetica, Arial; font-size: 13px; margin: 0px;" class=""><span class=""><span class="">Ramki,</span></span></div><div id="bloop_customfont" style="font-family: Helvetica, Arial; font-size: 13px; margin: 0px;" class=""><span class=""><br class=""></span></div><div id="bloop_customfont" style="font-family: Helvetica, Arial; font-size: 13px; margin: 0px;" class=""><span class="">Thanks for the feedback. Latest webrev:</span></div><div id="bloop_customfont" style="font-family: Helvetica, Arial; font-size: 13px; margin: 0px;" class=""><span class=""><br class=""></span></div><div id="bloop_customfont" style="font-family: Helvetica, Arial; font-size: 13px; margin: 0px;" class=""><span class=""><a moz-do-not-send="true" href="http://cr.openjdk.java.net/%7Etonyp/8155257/webrev.2/" class="">http://cr.openjdk.java.net/~tonyp/8155257/webrev.2/</a></span></div><div id="bloop_customfont" style="font-family: Helvetica, Arial; font-size: 13px; margin: 0px;" class=""><span class=""><br class=""></span></div><div id="bloop_customfont" style="font-family: Helvetica, Arial; font-size: 13px; margin: 0px;" class=""><span class="">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; margin: 0px;" class=""><span class=""><br class=""></span></div><div id="bloop_customfont" style="font-family: Helvetica, Arial; font-size: 13px; margin: 0px;" class=""><span class="">Tony</span></div><span class=""><br class=""></span><p class="airmail_on"><span class="">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><a class="moz-txt-link-abbreviated" href="mailto:ysr1729@gmail.com">ysr1729@gmail.com</a>) wrote:</span></p><blockquote type="cite" class="clean_bq"><div class=""><div class=""><div dir="ltr" class=""><div style="font-size: 12.8000001907349px;" class=""><span class=""><span class="">(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;" class=""><span class=""><br class=""></span></div><div style="font-size: 12.8000001907349px;" class=""><span class="">Looks good to me too. Minor remarks regarding the documentation comments:</span></div><div style="font-size: 12.8000001907349px;" class=""><span class=""><br class=""></span></div><div style="font-size: 12.8000001907349px;" class=""><pre style="white-space: pre-wrap;" class=""><span class=""><span style="color: blue; font-weight: bold;" class="">1098 // The par_oop_since_save_marks_iterate_done() method should be</span>
<span style="color: blue; font-weight: bold;" class="">1099 // called at the end of the main ParNew parallel phase to collapse</span>
<span style="color: blue; font-weight: bold;" class="">1100 // the promoted object lists. Given that we don't want promoted</span>
<span style="color: blue; font-weight: bold;" class="">1101 // objects to be tracked in future phases (e.g., during reference</span>
<span style="color: blue; font-weight: bold;" class="">1102 // processing) we disable promoted object tracking.</span>
</span>
</pre><pre style="white-space: pre-wrap;" class=""><span style="color: blue; font-weight: bold;" class="">
</span>
</pre><pre style="white-space: pre-wrap;" class=""><font face="arial, helvetica, sans-serif" class="">Perhaps just say:</font>
</pre><pre style="white-space: pre-wrap;" class=""><font face="arial, helvetica, sans-serif" class="">// Because card-scanning has been completed, further promotions, if any,</font>
</pre><pre style="white-space: pre-wrap;" class=""><font face="arial, helvetica, sans-serif" style="font-size: 12.8000001907349px;" class="">// e.g. during reference processing, </font><span style="font-size: 12.8000001907349px; font-family: arial, helvetica, sans-serif;" class="">will not need to keep track of promoted objects.</span>
</pre><pre style="white-space: pre-wrap;" class=""><span style="font-family: arial, helvetica, sans-serif;" class="">Also instead of:</span>
</pre><pre style="white-space: pre-wrap;" class=""><span style="color: blue; font-weight: bold;" class="">// Enable promotion tracking for the main parallel ParNew phase.</span>
</pre><pre style="white-space: pre-wrap;" class=""><font face="arial, helvetica, sans-serif" class="">perhaps:</font>
</pre><pre style="white-space: pre-wrap;" class=""><font face="arial, helvetica, sans-serif" class="">// We track promoted objects to allow card-scanning to skip them.</font>
</pre><pre style="white-space: pre-wrap;" class=""><font face="arial, helvetica, sans-serif" class="">and instead of:</font>
</pre><pre style="white-space: pre-wrap;" class=""><font face="arial, helvetica, sans-serif" class="">But leaving them as is fine too.</font>
</pre><pre style="white-space: pre-wrap;" class=""><font face="arial, helvetica, sans-serif" class="">reviewed!</font>
</pre><pre style="white-space: pre-wrap;" class=""><span class=""><font color="#888888" class=""><font face="arial, helvetica, sans-serif" class="">-- ramki</font></font></span>
</pre><pre style="white-space: pre-wrap;" class=""><span style="color: blue;" class="">2133 // Promotion tracking should be disabled at the end of the ParNew</span>
<span style="color: blue;" class="">2134 // parallel phase....</span>
</pre><pre style="white-space: pre-wrap;" class=""><font face="arial, helvetica, sans-serif" class="">perhaps:</font>
</pre><pre style="white-space: pre-wrap;" class=""><font face="arial, helvetica, sans-serif" class="">// When using ParNew, promoted object tracking would already have been disabled, however, ...</font>
</pre></div><div class="gmail_extra"><span class=""><font color="#888888" class=""><br class=""></font></span><div class="gmail_quote"><span class=""><font color="#888888" class="">On Thu, Apr 28, 2016 at 2:06 PM, Tony Printezis<span class="Apple-converted-space"> </span><span dir="ltr" class=""><<a moz-do-not-send="true" class="moz-txt-link-abbreviated" href="mailto:tprintezis@twitter.com"></a><a class="moz-txt-link-abbreviated" href="mailto:tprintezis@twitter.com">tprintezis@twitter.com</a>></span><span class="Apple-converted-space"> </span>wrote:<br class=""></font></span><blockquote class="gmail_quote" style="margin: 0px 0px 0px 0.8ex; border-left-width: 1px; border-left-color: rgb(204, 204, 204); border-left-style: solid; padding-left: 1ex;"><div style="word-wrap: break-word;" class=""><div style="font-family: Helvetica, Arial; font-size: 13px; margin: 0px;" class=""><span class="">Thanks Jon! New webrev here:</span></div><div style="font-family: Helvetica, Arial; font-size: 13px; margin: 0px;" class=""><span class=""><br class=""></span></div><div style="font-family: Helvetica, Arial; font-size: 13px; margin: 0px;" class=""><span class=""><a moz-do-not-send="true" class="moz-txt-link-freetext" href="http://cr.openjdk.java.net/%7Etonyp/8155257/webrev.1/"></a><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></span></div><div style="font-family: Helvetica, Arial; font-size: 13px; margin: 0px;" class=""><span class=""><br class=""></span></div><div style="font-family: Helvetica, Arial; font-size: 13px; margin: 0px;" class=""><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<span class="Apple-converted-space"> </span> <span class="Apple-converted-space"> </span><span class="Apple-converted-space"> </span> <span class="Apple-converted-space"> </span><span class="Apple-converted-space"> </span> <span class="Apple-converted-space"> </span><span class="Apple-converted-space"> </span> <span class="Apple-converted-space"> </span><span class="Apple-converted-space"> </span> <span class="Apple-converted-space"> </span><span class="Apple-converted-space"> </span> <span class="Apple-converted-space"> </span><span class="Apple-converted-space"> </span> <span class="Apple-converted-space"> </span><span class="Apple-converted-space"> </span> <span class="Apple-converted-space"> </span><span class="Apple-converted-space"> </span> <span class="Apple-converted-space"> </span><span class="Apple-converted-space"> </span> <span class="Apple-converted-space"> </span><span class="Apple-converted-space"> </span> <span class="Apple-converted-space"> </span><span class="Apple-converted-space"> </span> <span class="Apple-converted-space"> </span><span class="Apple-converted-space"> </span> <span class="Apple-converted-space"> </span><span class="Apple-converted-space"> </span> <span class="Apple-converted-space"> </span><span class="Apple-converted-space"> </span> <span class="Apple-converted-space"> </span><span class="Apple-converted-space"> </span> <span class="Apple-converted-space"> </span><span class="Apple-converted-space"> </span> <span class="Apple-converted-space"> </span>another set of tests overnight. </span></div><div style="font-family: Helvetica, Arial; font-size: 13px; margin: 0px;" class=""><span class=""><font color="#888888" class=""><br class=""></font></span></div><div style="font-family: Helvetica, Arial; font-size: 13px; margin: 0px;" class=""><span class=""><font color="#888888" class="">Tony</font></span></div><div class=""><div class="h5"><br class=""><p class="">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><a class="moz-txt-link-abbreviated" href="mailto:jon.masamitsu@oracle.com">jon.masamitsu@oracle.com</a>) wrote:</p><blockquote type="cite" class=""><div bgcolor="#FFFFFF" text="#000000" class=""><div class=""><span class=""><br class=""><br class=""></span><div class=""><span class="">On 04/28/2016 01:51 PM, Tony Printezis wrote:<br class=""></span></div><blockquote type="cite" class=""><div style="font-family: Helvetica, Arial; font-size: 13px; margin: 0px;" class=""><span class="">Jon (CCing Ramki),</span></div><div style="font-family: Helvetica, Arial; font-size: 13px; margin: 0px;" class=""><span class=""><br class=""></span></div><div style="font-family: Helvetica, Arial; font-size: 13px; margin: 0px;" class=""><span class="">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 class=""><br class="">That sounds good. I like the symmetry of putting the stopPromotionTracking in the epilogue. <br class=""><br class="">And yes, I'll run some addition tests when the latest patch comes out.<br class=""><br class="">Jon<br class=""><br class=""></span><blockquote type="cite" class=""><div style="font-family: Helvetica, Arial; font-size: 13px; margin: 0px;" class=""><span class=""><br class=""></span></div><div style="font-family: Helvetica, Arial; font-size: 13px; margin: 0px;" class=""><span class="">Tony</span></div><span class=""><br class=""></span><p class=""><span class="">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><a class="moz-txt-link-abbreviated" href="mailto:tprintezis@twitter.com">tprintezis@twitter.com</a>) wrote:</span></p><blockquote type="cite" class=""><div style="word-wrap: break-word;" class=""><div class=""><div style="font-family: Helvetica, Arial; font-size: 13px; margin: 0px;" class=""><span class=""><span class="">Hi Jon,</span></span></div><div style="font-family: Helvetica, Arial; font-size: 13px; margin: 0px;" class=""><span class=""><br class=""></span></div><div style="font-family: Helvetica, Arial; font-size: 13px; margin: 0px;" class=""><span class="">Thanks for looking at it! </span></div><div style="font-family: Helvetica, Arial; font-size: 13px; margin: 0px;" class=""><span class=""><br class=""></span></div><div style="font-family: Helvetica, Arial; font-size: 13px; margin: 0px;" class=""><span class="">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; margin: 0px;" class=""><span class=""><br class=""></span></div><div style="font-family: Helvetica, Arial; font-size: 13px; margin: 0px;" class=""><span class="">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; margin: 0px;" class=""><span class=""><br class=""></span></div><div style="font-family: Helvetica, Arial; font-size: 13px; margin: 0px;" class=""><span class="">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; margin: 0px;" class=""><span class=""><br class=""></span></div><div style="font-family: Helvetica, Arial; font-size: 13px; margin: 0px;" class=""><span class="">Tony</span></div><span class=""><br class=""></span><p class=""><span class="">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><a class="moz-txt-link-abbreviated" href="mailto:jon.masamitsu@oracle.com">jon.masamitsu@oracle.com</a>) wrote:</span></p><blockquote type="cite" class=""><div bgcolor="#FFFFFF" text="#000000" class=""><div class=""><span class=""><span class=""><font face="Times
New Roman,
Times, serif" class="">Tony,<br class=""><br class="">Changes look good. Couple of small points.<br class=""><br class=""><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><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><br class=""></font><br class=""></span></span><pre class=""><span class=""><span class="">1097 // This method should be called at the end of the main ParNew</span>
<span class="">1098 // parallel phase to collapse the promoted object lists. Given that</span>
<span class="">1099 // we don't want promoted objects to be tracked in future phases</span>
<span class="">1100 // (e.g., during reference processing) we also disable promote</span>
<span class="">1101 // object tracking here.</span>
</span>
</pre><font face="Times
New Roman,
Times, serif" class="">The placement of this block comment was slightly confusing. I wasn't<br class="">sure if "This method ..." applied specirfically to the stopTrackingPromotions()<br class="">until the last part "also disable ...". Would it be better placed before the method.<br class=""></font><br class=""><pre class="">2132 // Also reset promotion tracking in par gc thread states.
<span class="">2133 // I don't think this is really needed, as promotion tracking should</span>
<span class="">2134 // have already been disabled. However, it sanity checks that the</span>
<span class="">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 class="">sanity check you can use? Comments sometimes get lost in the fog of code<br class="">churn and calling a method just for sanity checkinf seems wasteful and confusing<br class="">(Why is stopTrackingPromotions() call twice? Is there something that the<br class="">first all didn't do? Yes, you comment explains it but I have to read the comment.)<br class="">If you agree you can fix it at a later time. If you disagree, I'll silently forget about it :-).<br class=""><br class="">Jon<br class=""><br class=""><div class="">On 04/27/2016 01:40 PM, Tony Printezis wrote:<br class=""></div><blockquote type="cite" class=""><div style="font-family: Helvetica, Arial; font-size: 13px; margin: 0px;" class="">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; margin: 0px;" class=""><br class=""></div><div style="font-family: Helvetica, Arial; font-size: 13px; margin: 0px;" class=""><a moz-do-not-send="true" class="moz-txt-link-freetext" href="http://cr.openjdk.java.net/%7Etonyp/8155257/webrev.0/"></a><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></div><div style="font-family: Helvetica, Arial; font-size: 13px; margin: 0px;" class=""><br class=""></div><div style="font-family: Helvetica, Arial; font-size: 13px; margin: 0px;" class="">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; margin: 0px;" class=""><br class=""></div><div style="font-family: Helvetica, Arial; font-size: 13px; margin: 0px;" class="">I’ll run more testing overnight.</div><div style="font-family: Helvetica, Arial; font-size: 13px; margin: 0px;" class=""><br class=""></div><div style="font-family: Helvetica, Arial; font-size: 13px; margin: 0px;" class="">Tony</div><br class=""><div class=""><div style="font-family: helvetica, arial; font-size: 13px;" class=""><div class="">-----</div><div class=""><br class=""></div><div class="">Tony Printezis | JVM/GC Engineer / VM Team | Twitter</div><div class=""><br class=""></div><div class="">@TonyPrintezis</div><div class=""><a moz-do-not-send="true" class="moz-txt-link-abbreviated" href="mailto:tprintezis@twitter.com"></a><a class="moz-txt-link-abbreviated" href="mailto:tprintezis@twitter.com">tprintezis@twitter.com</a></div><div class=""><br class=""></div></div></div></blockquote><br class=""></div></div></blockquote><div class=""><div style="font-family: helvetica, arial; font-size: 13px;" class=""><div class="">-----</div><div class=""><br class=""></div><div class="">Tony Printezis | JVM/GC Engineer / VM Team | Twitter</div><div class=""><br class=""></div><div class="">@TonyPrintezis</div><div class=""><a moz-do-not-send="true" class="moz-txt-link-abbreviated" href="mailto:tprintezis@twitter.com"></a><a class="moz-txt-link-abbreviated" href="mailto:tprintezis@twitter.com">tprintezis@twitter.com</a></div><div class=""><br class=""></div></div></div></div></div></blockquote><div class=""><div style="font-family: helvetica, arial; font-size: 13px;" class=""><div class="">-----</div><div class=""><br class=""></div><div class="">Tony Printezis | JVM/GC Engineer / VM Team | Twitter</div><div class=""><br class=""></div><div class="">@TonyPrintezis</div><div class=""><a moz-do-not-send="true" class="moz-txt-link-abbreviated" href="mailto:tprintezis@twitter.com"></a><a class="moz-txt-link-abbreviated" href="mailto:tprintezis@twitter.com">tprintezis@twitter.com</a></div><div class=""><br class=""></div></div></div></blockquote><br class=""></div></div></blockquote><div class=""><div style="font-family: helvetica, arial; font-size: 13px;" class=""><div class="">-----</div><div class=""><br class=""></div><div class="">Tony Printezis | JVM/GC Engineer / VM Team | Twitter</div><div class=""><br class=""></div><div class="">@TonyPrintezis</div><div class=""><a moz-do-not-send="true" class="moz-txt-link-abbreviated" href="mailto:tprintezis@twitter.com"></a><a class="moz-txt-link-abbreviated" href="mailto:tprintezis@twitter.com">tprintezis@twitter.com</a></div><div class=""><br class=""></div></div></div></div></div></div></blockquote></div><br class=""></div></div></div></div></blockquote><div id="bloop_sign_1461938962388339968" class="bloop_sign"><div style="font-family: helvetica, arial; font-size: 13px;" class=""><div class="">-----</div><div class=""><br class=""></div><div class="">Tony Printezis | JVM/GC Engineer / VM Team | Twitter</div><div class=""><br class=""></div><div class="">@TonyPrintezis</div><div class=""><a moz-do-not-send="true" href="mailto:tprintezis@twitter.com" class="">tprintezis@twitter.com</a></div><div class=""><br class=""></div></div></div></div></div></blockquote><div id="bloop_sign_1461941952758243072" class="bloop_sign"><div style="font-family: helvetica, arial; font-size: 13px;" class=""><div class="">-----</div><div class=""><br class=""></div><div class="">Tony Printezis | JVM/GC Engineer / VM Team | Twitter</div><div class=""><br class=""></div><div class="">@TonyPrintezis</div><div class=""><a moz-do-not-send="true" href="mailto:tprintezis@twitter.com" class="">tprintezis@twitter.com</a></div><div class=""><br class=""></div></div></div></blockquote><br class=""></div></div></blockquote><div id="bloop_sign_1462200730444526848" class="bloop_sign"><div style="font-family: helvetica, arial; font-size: 13px;" class=""><div class="">-----</div><div class=""><br class=""></div><div class="">Tony Printezis | JVM/GC Engineer / VM Team | Twitter</div><div class=""><br class=""></div><div class="">@TonyPrintezis</div><div class=""><a moz-do-not-send="true" href="mailto:tprintezis@twitter.com" class="">tprintezis@twitter.com</a></div><div class=""><br class=""></div></div></div></blockquote><br class=""></div></div></span></blockquote><div id="bloop_sign_1462226381911459840" class="bloop_sign"><div style="font-family: helvetica, arial; font-size: 13px;" class=""><div class="">-----</div><div class=""><br class=""></div><div class="">Tony Printezis | JVM/GC Engineer / VM Team | Twitter</div><div class=""><br class=""></div><div class="">@TonyPrintezis</div><div class=""><a moz-do-not-send="true" href="mailto:tprintezis@twitter.com" class="">tprintezis@twitter.com</a></div><div class=""><br class=""></div></div></div></blockquote><br style="font-family: Helvetica, Arial; font-size: 13px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; background-color: rgb(255, 255, 255);" class=""><br class="Apple-interchange-newline"></div></blockquote></div><br class=""></div></body></html>