Request for review (XS): JDK-8006430: TraceTypeProfile is a product flag while it should be a diagnostic flag

[Staffan Larsen]

Thanks for explaining.

/Staffan

On 14 feb 2013, at 18:45, Vladimir Kozlov <vladimir.kozlov at oracle.com> wrote:

> It is done to separate flags per component (where a flag is defined) when we print them (-XX:+PrintFlagsInitial, -XX:+PrintFlagsFinal):
> 
>  bool TraceTypeProfile    = false    {C2 diagnostic}
> 
> Yes, we can extend Flag struct to separate 'kind' and 'component' (or whatever). Could be good task for new members of Hotspot group.
> 
> Vladimir
> 
> On 2/14/13 4:04 AM, Staffan Larsen wrote:
>> Yes, exactly.
>> 
>> /Staffan
>> 
>> On 14 feb 2013, at 03:02, Krystal Mo <krystal.mo at oracle.com> wrote:
>> 
>>> Hi Chris,
>>> 
>>> I think Staffan is asking why we differentiate {C2 diagnostic} and friends from {diagnostic}.
>>> 
>>> - Kris
>>> 
>>> On 02/13/2013 12:26 PM, Christian Thalinger wrote:
>>>> On Feb 11, 2013, at 12:57 AM, Staffan Larsen <staffan.larsen at oracle.com> wrote:
>>>> 
>>>>> Forgive my ignorance, but why do diagnostic and experimental flags have different "kinds" depending on what part of the VM they belong to? Can't they all be just "{experimental}" or "{diagnostic}"? I don't see where the additional C1/C2/ARCH/Shark information is used.
>>>> Are you asking why we have experimental and diagnostic flags?
>>>> 
>>>> -- Chris
>>>> 
>>>>> Thanks,
>>>>> /Staffan
>>>>> 
>>>>> On 9 feb 2013, at 02:23, Krystal Mo <krystal.mo at oracle.com> wrote:
>>>>> 
>>>>>> Hi all,
>>>>>> 
>>>>>> Could I have a couple of reviews for this fix, please?
>>>>>> 
>>>>>> Webrev: http://cr.openjdk.java.net/~kmo/8006430/webrev.00/
>>>>>> Bug: http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=8006430
>>>>>> 
>>>>>> This bug is actually not a regression from JDK-7198499, but a long time issue that C2 diagnostic flags were never treated as being "diagnostic" since the OpenJDK Mercurial repo history began (duke at 0).
>>>>>> 
>>>>>> The problem is, C2 diagnostic flags have the kind "{C2 diagnostic}", but Flag::is_unlocked() only cared to test "{diagnostic}".
>>>>>> So the fix is straighforward -- find out how many other "diagnostic" and "experimental" flag kinds there are, and check them.
>>>>>> 
>>>>>> I didn't just check if the string ends with "diagnostic}" or "experimental}" because I think that's too permissive, but I don't have a strong opinion on that.
>>>>>> If reviews form a consensus that checking just the postfix is good enough, I can make the change that way.
>>>>>> 
>>>>>> Care has to be taken if new diagnostic/experimental flag kinds are added to other components in the future, e.g. C1.
>>>>>> 
>>>>>> Tested with JPRT and these flags by hand:
>>>>>> C2 diagnostic: TraceTypeProfile, LoopLimitCheck, RangeLimitCheck
>>>>>> ARCH diagnostic: UseIncDec
>>>>>> 
>>>>>> Thanks,
>>>>>> Kris
>>> 
>>