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

[john cuthbertson - Sun Microsystems]

Hi John and David,

John Coomes wrote:
> We're interested in providing sensible data; the current flags don't.
> And the names are terrible.
>
> In any approach we take, we're going to at least
>
> 	- add new options that track all safepoints
> 	- deprecate the old option names, and issue a warning or message
> 	  if they're used
>
> The question becomes what data to produce when the old option names
> are used.  You are recommending (a) leave the buggy behavior intact.
> I am advocating (b) print the same data that the new options print
> (data on all safepoints).  But there is also (c) change the behavior
> to accurately track GC and only GC safepoints.
>
> I still prefer (b), with (c) as a second choice.  But we have to at
> the very least get rid of the buggy behavior.
>
> -John
>   
I do not have a strong preference about whether to remove or keep the 
previous behavior. I slightly favor removing the printing of the old 
buggy data (which was done originally). If these flags are being 
extensively used to profile application behavior then I'm certain that 
more people would have noticed the mis-accounting.

In the alternatives listed above I would rather not do (c) as that would 
probably mean having an additional 2 time stamps maintained by the 
RuntimeService class. The code in 
RuntimeService::record_safepoint_{begin|end} would start to get rather 
untidy: we would be unconditionally updating one set of TimeStamps (the 
non-GC ones) and the other conditionally on whether the current 
VMOperation is a GC op. Another downside to (c) that we would be telling 
the user that their application ran for x seconds when in actual fact it 
ran for (x-y) seconds (where y would be the  sum of the non-GC safepoint 
durations). The times reported would match those of the GC timestamps 
but we would be misleading the user about the length of the time 
interval that their app was running for. A better description for this 
would be PrintInterGCIntervals rather than PrintGCApplicationConcurrentTime.

JohnC