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

[Vladimir Kozlov]

John,

If you are changing flags which were used or will be used by public
you have to file CCC request for approval which takes a time.
I would suggest to keep old flags but it is up to you.

Thanks,
Vladimir

john cuthbertson - Sun Microsystems wrote:
> 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
>