<html><head><style>body{font-family:Helvetica,Arial;font-size:13px}</style></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;"><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 Bengt,</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 for looking at it. Inline.</div> <br><p class="airmail_on" style="color:#000;">On June 18, 2015 at 3:02:45 AM, Bengt Rutisson (<a href="mailto:bengt.rutisson@oracle.com">bengt.rutisson@oracle.com</a>) wrote:</p> <div><blockquote type="cite" class="clean_bq" style="color: rgb(0, 0, 0); font-family: Helvetica, Arial; font-size: 13px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: 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;"><span><div bgcolor="#FFFFFF" text="#000000"><div></div><div><div class="moz-cite-prefix"><br>Hi Tony,<br><br>NIce to hear from you!<br><br><br>On 18/06/15 00:30, Tony Printezis wrote:<br></div><blockquote cite="mid:etPan.5581f51b.699a1ea0.1215a@tw-mbp-tprintezis" type="cite"><div id="bloop_customfont" style="font-family: Helvetica, Arial; font-size: 13px; color: rgb(0, 0, 0); margin: 0px;">Hi all,</div><div id="bloop_customfont" style="font-family: Helvetica, Arial; font-size: 13px; color: rgb(0, 0, 0); margin: 0px;"><br></div><div id="bloop_customfont" style="font-family: Helvetica, Arial; font-size: 13px; color: rgb(0, 0, 0); margin: 0px;">A small patch for your consideration:</div><div id="bloop_customfont" style="font-family: Helvetica, Arial; font-size: 13px; color: rgb(0, 0, 0); margin: 0px;"><br></div><div id="bloop_customfont" style="font-family: Helvetica, Arial; font-size: 13px; color: rgb(0, 0, 0); margin: 0px;"><a moz-do-not-send="true" href="http://cr.openjdk.java.net/%7Etonyp/8086056/webrev.0/">http://cr.openjdk.java.net/~tonyp/8086056/webrev.0/</a></div></blockquote><br>A couple of style comments:<br><br> 891 const size_t MinOldGenCapacity = G;<br> 892 const size_t MaxOldGenCapacity = 16 * G;<br> 893 assert(MinOldGenCapacity <= MaxOldGenCapacity, "sanity");<br><br>Local variables are normmaly named using lower case and underscore.<span class="Apple-converted-space"> </span></div></div></span></blockquote></div><p><br></p><p>Yeah, I of course usually do that. But as they are constants I thought I’d make them look like constants. :-)</p><p><br></p><div><div><div><blockquote type="cite" class="clean_bq" style="color: rgb(0, 0, 0); font-family: Helvetica, Arial; font-size: 13px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: 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;"><span><div bgcolor="#FFFFFF" text="#000000"><div>So, min_old_gen_capacity and max_old_gen_capacity. Same for the rest of the local variables in ParNewGeneration::adjust_cards_per_stride().<br><br>Having said that, I think it would be good to turn some of these constants into flags (which means that the naming should be as it is now ;) ).<span class="Apple-converted-space"> </span></div></div></span></blockquote></div><p><br></p><p>Hang on! Are you and Masa encourating me to add 5 (!!!) more cmd line args to HotSpot? ;-)</p><p><br></p><div><blockquote type="cite" class="clean_bq" style="color: rgb(0, 0, 0); font-family: Helvetica, Arial; font-size: 13px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: 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;"><span><div bgcolor="#FFFFFF" text="#000000"><div>Especially since you say that you haven't done much performance testing of these values.</div></div></span></blockquote></div></div><p><br></p><p>To be clear: I did performance testing with chunks sizes in the 256-8k range and with three old gen sizes (2g, 10g, and 20g) and auto-tuning comes pretty close to optimal; I have numbers on the JIRA. But, yes, that was only with a couple of (synthetic) tests and only on one architecture (Linux/x64). If someone there could try this on a couple more apps and maybe on sparc, as we don’t happen to have any sparc boxes lying around :-), it’d be helpful.</p><p>But, yes, your suggestion of making the chunk size / old gen capacity limits into cmd line args definitely makes sense and I had also considered that myself. But I had decided against it as I thought you wouldn’t want more cmd line args. :-) How about:</p><p>+UseDynamicParGCStrides</p><p>DynamicParGCStrides{Min,Max}OldGenCapacity</p><p>DynamicParGCStrides{Min,Max}Size</p><p>I should also make them all manageable.</p><p><br></p><div><div><blockquote type="cite" class="clean_bq" style="color: rgb(0, 0, 0); font-family: Helvetica, Arial; font-size: 13px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: 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;"><span><div bgcolor="#FFFFFF" text="#000000"><div><span class="Apple-converted-space"> </span>I would prefer that we could adjust them at runtime. Best would of course be if you could take the time to do the performance testing.</div></div></span></blockquote></div><p><br></p><p>So, as I said earlier, I did. The numbers are on the JIRA. Is there anything else I could maybe run? Maybe larger old gen than 20g? (but that’s reaching the memory limits of my workstation, FWIW)</p><p><br></p><div><div><blockquote type="cite" class="clean_bq" style="color: rgb(0, 0, 0); font-family: Helvetica, Arial; font-size: 13px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: 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;"><span><div bgcolor="#FFFFFF" text="#000000"><div><br>I realize that the assert on line 893 might have been useful if you were playing around with the numbers when you were experimenting with the patch. But now it looks mostly like line noise to me. Same with the assert on line 898.</div></div></span></blockquote></div><p><br></p><p>Actually, it as basically stating the invariant for those parameters (in case someone wants to play around with them). But if we make the constants into cmd line args this will go away...</p><p><br></p><div><div><blockquote type="cite" class="clean_bq" style="color: rgb(0, 0, 0); font-family: Helvetica, Arial; font-size: 13px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: 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;"><span><div bgcolor="#FFFFFF" text="#000000"><div><br>I would also consider factoring the actual computation out into a separate helper method.</div></div></span></blockquote></div><p><br></p><p>You mean the interpolation? Like:</p><p>(base_min, base_max, value_min, value_max, value) -> result in range [base_min, base_max]?</p><p>Tony</p><p><br></p><div><blockquote type="cite" class="clean_bq" style="color: rgb(0, 0, 0); font-family: Helvetica, Arial; font-size: 13px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: 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;"><span><div bgcolor="#FFFFFF" text="#000000"><div><br>I also second Jon's comment on that it would be good with a more explicit way of turning this feature off.<br><br>Thanks,<br>Bengt<br><br><blockquote cite="mid:etPan.5581f51b.699a1ea0.1215a@tw-mbp-tprintezis" type="cite"><div id="bloop_customfont" style="font-family: Helvetica, Arial; font-size: 13px; color: rgb(0, 0, 0); margin: 0px;"><br></div><div id="bloop_customfont" style="font-family: Helvetica, Arial; font-size: 13px; color: rgb(0, 0, 0); margin: 0px;">(BTW, for some reason some of the webrev output is a bit messed up. Not sure why, maybe some hg incompatibility I guess. The diffs look OK though. I also attached the patch to this e-mail.)</div><div id="bloop_customfont" style="font-family: Helvetica, Arial; font-size: 13px; color: rgb(0, 0, 0); margin: 0px;"><br></div><div id="bloop_customfont" style="font-family: Helvetica, Arial; font-size: 13px; color: rgb(0, 0, 0); margin: 0px;">There’s a bit of info on the JIRA on the rationale for the patch:</div><div id="bloop_customfont" style="font-family: Helvetica, Arial; font-size: 13px; color: rgb(0, 0, 0); margin: 0px;"><br></div><div id="bloop_customfont" style="font-family: Helvetica, Arial; font-size: 13px; color: rgb(0, 0, 0); margin: 0px;"><a moz-do-not-send="true" href="https://bugs.openjdk.java.net/browse/JDK-8086056">https://bugs.openjdk.java.net/browse/JDK-8086056</a></div><div id="bloop_customfont" style="font-family: Helvetica, Arial; font-size: 13px; color: rgb(0, 0, 0); margin: 0px;"><br></div><div id="bloop_customfont" style="font-family: Helvetica, Arial; font-size: 13px; color: rgb(0, 0, 0); margin: 0px;">The min / max values for the old gen capacity and ParGCCardsPerStrideChunk were chosen empircally after running a few (mostly synthetic tests) on Linux x64. If someone has the cycles to do a more extensive performance study, I’d be happy to revise them accordingly.</div><div id="bloop_customfont" style="font-family: Helvetica, Arial; font-size: 13px; color: rgb(0, 0, 0); margin: 0px;"><br></div><div id="bloop_customfont" style="font-family: Helvetica, Arial; font-size: 13px; color: rgb(0, 0, 0); margin: 0px;">Regards,</div><div id="bloop_customfont" style="font-family: Helvetica, Arial; font-size: 13px; color: rgb(0, 0, 0); margin: 0px;"><br></div><div id="bloop_customfont" style="font-family: Helvetica, Arial; font-size: 13px; color: rgb(0, 0, 0); margin: 0px;">Tony</div><br><div id="bloop_sign_1434579909530055936" 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><br class="Apple-interchange-newline"></div><br class="Apple-interchange-newline"></div><br class="Apple-interchange-newline"></div><br class="Apple-interchange-newline"></div><br class="Apple-interchange-newline"></div> <div id="bloop_sign_1434634117464453888" 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 href="mailto:tprintezis@twitter.com">tprintezis@twitter.com</a></div><div><br></div></div></div></body></html>