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

[David Holmes - Sun Microsystems]

Hi John,

I advocate leaving the current behaviour in JDK6 as that is the most 
compatible solution - but that is accompanied by a notice to the user 
that the flags are deprecated and may not report what might be expected 
(which is arbitrary because there's no definition of what the flags 
really mean) and telling them to update to the new flags. So I don't see 
that this will continue to cause future problems/bug-reports.

As far as I can see the bug reporter had an issue because these flags 
don't measure what they want to measure (all safepoints) as opposed to 
the flags not measuring what their names suggested - GC related 
safepoint activity.

But this is just my view. I won't lose any sleep over it going the other 
way.

John Cuthbertson - sorry this is making more work for you.

Cheers,
David

John Coomes said the following on 07/23/09 06:14:
> David Holmes - Sun Microsystems (David.Holmes at Sun.COM) wrote:
>> John,
>>
>> John Coomes said the following on 07/22/09 10:57:
>>> Your original webrev (webrev.00) removed the print statements in
>>> vmThread.cpp.  Based upon David Holmes' comments (I believe) this
>>> version restores them, which keeps the existing (buggy) behavior of
>>> missing time in jdk6 when the old names are used.  I think that's
>>> taking compatibility too far; we should get rid of the buggy behavior
>>> completely.
>> The intent was to give JDK6 users a transition path to the new flags 
>> which perform a different (albeit better) measurement than the current 
>> flags, without simply changing the meaning of the existing flags in 
>> mid-stream via a JDK6 update release.
>>
>> If a customer uses these flags to track performance changes across 
>> releases then changing the meaning of them under-the-covers is not a 
>> user-friendly thing to do. This might seem overly conservative but it is 
>> the safer option.
> 
> Hi David,
> 
> The info we are currently providing is inaccurate and misleading.
> Certainly the bug submitter felt that way.  By fixing it, we can avoid
> others wasting time on the problem; to do so we have to give up at
> least some degree of compatibility.
> 
>> If we were only interested in fixing the bug with the current flags then 
>> we'd be tracking only GC safepoint ops, and we'd keep the old flag names.
> 
> 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
>