Request for review: 8038265: CMS: enable time based triggering of concurrent cycles

[Mikael Gerdin]

Hi Michal,

On Monday 31 March 2014 14.32.01 Michal Frajt wrote:
> Hi all,could you please review the following change we prepared together
> with Bengt? The change adds new CMSTriggerInterval flag which allows to
> invoke CMS collections periodically. The periodically running CMS
> collections are helping us with replacing the deprecated incremental CMS
> collector. We believe it might be useful for other customers currently
> using the CMS in the incremental mode.Bugs:
> https://bugs.openjdk.java.net/browse/JDK-8038265
> 
> Webrev:
> http://cr.openjdk.java.net/~brutisso/8038265/webrev.00/

If the addition of this functionality helps you replace iCMS then that's 
great! I think this code change is simple and easy to understand. 

I just have some small (mostly stylistic) comments:

	The other time output in PrintCMSInitiationStatistics seems to use 	
	%3.7f as the format specifier instead of %g.

1515     gclog_or_tty->print_cr("cms_time_since_begin=%g", 
stats().cms_time_since_begin());
1516     gclog_or_tty->print_cr("cms_time_since_end=%g", 
stats().cms_time_since_end());


	The HotSpot style is to have a space between if and the opening brace, 
	can you please change the if here:

1588     if(stats().cms_time_since_begin() >= (CMSTriggerInterval / ((double) 
MILLIUNITS))) {
1589       if (Verbose && PrintGCDetails) {

	And here:

1590         if(stats().valid()) {

	We usually align the parameters on the second line with the opening 	
	brace on the function call, something like:

gclog_or_tty->print_cr(foo, bar,
                       stats().baz());

1591           gclog_or_tty->print_cr("CMSCollector: collect because of 
trigger interval (time since last begin %g secs)",
1592             stats().cms_time_since_begin());
1593         } else {

	You pass in cms_time_since_begin() but don't actually use that value 	
	in the else branch. 

1594           gclog_or_tty->print_cr("CMSCollector: collect because of 
trigger interval (first collection)",
1595             stats().cms_time_since_begin());
1596         }
1597       }

/Mikael


> Thanks,
> Michal