<div class="xam_msg_class">
<div >Hi Mikael,<br><br>Find a new webrev which is addressing your small (mostly stylistic) comments. If you still find something missing we can quickly create new webrev. <br><br>Here is a new webrev:<br>
    <a target="_blank" class="moz-txt-link-freetext" href="http://cr.openjdk.java.net/%7Ebrutisso/8038265/webrev.01/">http://cr.openjdk.java.net/~brutisso/8038265/webrev.01/</a><br>
    <br>
    Here is a diff between the old and the new webrev:<br>
    <a target="_blank" class="moz-txt-link-freetext" href="http://cr.openjdk.java.net/%7Ebrutisso/8038265/webrev.00-01.diff/">http://cr.openjdk.java.net/~brutisso/8038265/webrev.00-01.diff/</a><br><br>Best regards,<br>Michal<br><br>
<div><span >Od</span><span >: "Michal Frajt" michal@frajt.eu</span></div>
<div><span >Komu</span><span >: bengt.rutisson@oracle.com</span></div>
<div><span >Kopie</span><span >: mikael.gerdin@oracle.com, hotspot-gc-dev@openjdk.java.net</span></div>
<div><span >Datum</span><span >: Wed, 16 Apr 2014 16:36:38 +0200</span></div>
<div><span >Předmet</span><span >: Re: Request for review: 8038265: CMS: enable time based triggering of concurrent cycles</span></div>
<br>

<div class="xam_msg_class">

<div class="xam_msg_class">
<div>> Hi Bengt,<br><br>> We will provide you the new patch next week. All Mikael's comments will be addressed.<br><br>> Best regards,<br>> Michal<br><br>
<div><span>> Od</span><span>: "hotspot-gc-dev" hotspot-gc-dev-bounces@openjdk.java.net</span></div>
<div><span>> Komu</span><span>: "Mikael Gerdin" mikael.gerdin@oracle.com, hotspot-gc-dev@openjdk.java.net</span></div>
<div><span>> Kopie</span><span>: </span></div>
<div><span>> Datum</span><span>: Mon, 14 Apr 2014 17:24:16 +0200</span></div>
<div><span>> Předmet</span><span>: Re: Request for review: 8038265: CMS: enable time based triggering of concurrent cycles</span></div>
<br>
<div>> > </div><div>> > Hi Michal,</div><div>> > </div><div>> > I think the change looks good modulo the comments that Mikael has. If </div><div>> > you fix those I can help you with a new webrev and then push this change.</div><div>> > </div><div>> > Thanks,</div><div>> > Bengt</div><div>> > </div><div>> > </div><div>> > On 2014-04-01 11:52, Mikael Gerdin wrote:</div><div>> > > Hi Michal,</div><div>> > ></div><div>> > > On Monday 31 March 2014 14.32.01 Michal Frajt wrote:</div><div>> > >> Hi all,could you please review the following change we prepared together</div><div>> > >> with Bengt? The change adds new CMSTriggerInterval flag which allows to</div><div>> > >> invoke CMS collections periodically. The periodically running CMS</div><div>> > >> collections are helping us with replacing the deprecated incremental CMS</div><div>> > >> collector. We believe it might be useful for other customers currently</div><div>> > >> using the CMS in the incremental mode.Bugs:</div><div>> > >> https://bugs.openjdk.java.net/browse/JDK-8038265</div><div>> > >></div><div>> > >> Webrev:</div><div>> > >> http://cr.openjdk.java.net/~brutisso/8038265/webrev.00/</div><div>> > > If the addition of this functionality helps you replace iCMS then that's</div><div>> > > great! I think this code change is simple and easy to understand.</div><div>> > ></div><div>> > > I just have some small (mostly stylistic) comments:</div><div>> > ></div><div>> > >     The other time output in PrintCMSInitiationStatistics seems to use      </div><div>> > >   %3.7f as the format specifier instead of %g.</div><div>> > ></div><div>> > > 1515     gclog_or_tty->print_cr("cms_time_since_begin=%g",</div><div>> > > stats().cms_time_since_begin());</div><div>> > > 1516     gclog_or_tty->print_cr("cms_time_since_end=%g",</div><div>> > > stats().cms_time_since_end());</div><div>> > ></div><div>> > ></div><div>> > >      The HotSpot style is to have a space between if and the opening brace,</div><div>> > >     can you please change the if here:</div><div>> > ></div><div>> > > 1588     if(stats().cms_time_since_begin() >= (CMSTriggerInterval / ((double)</div><div>> > > MILLIUNITS))) {</div><div>> > > 1589       if (Verbose && PrintGCDetails) {</div><div>> > ></div><div>> > >   And here:</div><div>> > ></div><div>> > > 1590         if(stats().valid()) {</div><div>> > ></div><div>> > >        We usually align the parameters on the second line with the opening     </div><div>> > >   brace on the function call, something like:</div><div>> > ></div><div>> > > gclog_or_tty->print_cr(foo, bar,</div><div>> > >                         stats().baz());</div><div>> > ></div><div>> > > 1591           gclog_or_tty->print_cr("CMSCollector: collect because of</div><div>> > > trigger interval (time since last begin %g secs)",</div><div>> > > 1592             stats().cms_time_since_begin());</div><div>> > > 1593         } else {</div><div>> > ></div><div>> > >       You pass in cms_time_since_begin() but don't actually use that value    </div><div>> > >   in the else branch.</div><div>> > ></div><div>> > > 1594           gclog_or_tty->print_cr("CMSCollector: collect because of</div><div>> > > trigger interval (first collection)",</div><div>> > > 1595             stats().cms_time_since_begin());</div><div>> > > 1596         }</div><div>> > > 1597       }</div><div>> > ></div><div>> > > /Mikael</div><div>> > ></div><div>> > ></div><div>> > >> Thanks,</div><div>> > >> Michal</div><div>> > </div></div>
</div>

</div>
</div>
</div>