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

[john cuthbertson - Sun Microsystems]

Hi Vladimir,

Thanks for the review. I have removed the bug id from the code and 
changed the warning messages in arguments.cpp. The question I have is: 
should the the old flag name emit the warning and silently set the new 
flag, or emit the warning and be ignored? We seem to do both in 
arguments.cpp.

Thanks,

JohnC

Vladimir Kozlov wrote:
> John,
>
> We usually don't put bug's id as a comment,
> you can get it from the changeset.
> It is better to have a real comment explaining the code.
> And if you have it, as in runtimeService.cpp, you don't need bug's id.
>
> In arguments.cpp, I think, warnings should have what should be used:
>
> warning("-XX:+PrintGCApplicationConcurrentTime is obsolete. Use 
> -XX:+PrintApplicationExecutionTimes instead.");
>
> Thanks,
> Vladimir
>
> john cuthbertson - Sun Microsystems wrote:
>> Hi Everyone,
>>
>> Can I have a couple of volunteers look over the attached changes for 
>> this CR? The changes can be found at:
>>
>> http://cr.openjdk.java.net/~johnc/6782663/webrev.00/
>>
>> The issue here was that the code that was printing this information 
>> was only executed for synchronous safepoints and ignored forced 
>> safepoints. The user would see missing chunks of time (especially 
>> with also displaying GC time stamps) giving the impression that the 
>> JVM and app were idle. I was able to see this in the first few 
>> minutes of SPECjbb2005.
>>
>> The simple solution was to move the printing into the routines that 
>> record safepoint starts and ends, capturing guaranteed safepoints. I 
>> have also changed the names of the flags to more accurately describe 
>> that GCs are not the only things we take a safepoint for (and 
>> deprecated the old flag names).
>>
>> With these changes I see a delta between (application time + 
>> safepoint time) and the GC timestamp of around 0.1s instead of around 
>> 5 and 7 seconds previously.
>>
>> Testing: SPECjbb2005 (exhibited behavior), IBM UK's own evaluation, 
>> and JPRT.
>>
>> Thanks,
>>
>> JohnC