Please Review (S): CR 6782663: Data produced by PrintGCApplicationConcurrentTime and PrintGCApplicationStoppedTime is not accurate

David Holmes - Sun Microsystems David.Holmes at Sun.COM
Tue Jul 21 16:07:27 PDT 2009


John,

Mostly looks okay to me. Minor nit:

  429         if (PrintGCApplicationConcurrentTime) {
  430           assert(!JDK_Version::is_gte_jdk17x_version(), "obsolete 
flag");
  431           if (!JDK_Version::is_gte_jdk17x_version())
  432             gclog_or_tty->print_cr("Application time: %3.7f seconds",

if you are going to check the version regardless then just make it a 
guarantee at 430 and delete 431 as in JDK7 the flag should always be false.

Also in the "incompatibility" message you should indicate which is the 
deprecated flag name.

Cheers,
David

Vladimir Kozlov said the following on 07/22/09 07:21:
> John,
> 
> Looks good.
> In vmThread.cpp add flag name into asserts message otherwise
> it is not clear what a flag the asserts complain about.
> 
> Thanks,
> Vladimir
> 
> john cuthbertson - Sun Microsystems wrote:
>> Hi David,
>>
>> Thank you for the review and answering my questions. I have applied 
>> the comments that you and Vladimir have suggested. I went with your 
>> suggestions for the new flag names as they do accurately describe what 
>> is being measured and displayed/logged.
>>
>> The new webrev can be found at:
>>
>> http://cr.openjdk.java.net/~johnc/6782663/webrev.01/
>>
>> Can I have a couple of volunteers look these changes over?
>>
>> Thanks,
>>
>> JohnC



More information about the hotspot-runtime-dev mailing list