RFR: 8048093 - Explicitly setting := vs = in the -XX:+PrintFlagsFinal output

[Jesper Wilhelmsson]

Den 23/6/16 kl. 20:22, skrev Vladimir Kozlov:
> why you skipped 19?:
>
> ORIG_COMMAND_LINE       = 1 << 20,

No good reason really, just leaving some space since ORIG_COMMAND_LINE is not a 
kind. I can change it to 19 if you think that's better.

>
> Otherwise looks good.

Thanks for reviewing!

/Jesper

>
> Thanks,
> Vladimir
>
> On 6/23/16 8:04 AM, Jesper Wilhelmsson wrote:
>> Den 22/6/16 kl. 21:33, skrev Coleen Phillimore:
>>
>>>
>>> I agree with Vladimir.  There's no way I'm going to remember that a colon
>>> mean that the value was changed.
>> The colon is what we have today to indicate that a value was changed :)
>>
>> I made a different solution in which I explicitly print the origin with words.
>> To make it look better I changed the
>> printing routine somewhat. The origin is currently at the end of the line.
>> This version will also print other origins like config file, environment, and
>> management as suggested by Dan.
>>
>> New webrev:
>> http://cr.openjdk.java.net/~jwilhelm/8048093/webrev.01/
>>
>> Examples of -XX:+PrintFlagsFinal with and without the change:
>>
>> This is the new version:
>>
>>     ccstr AbortVMOnExceptionMessage =
>> {diagnostic} {default}
>>     uintx AdaptiveSizeDecrementScaleFactor         =
>> 4                                        {product} {default}
>>      intx AliasLevel                               =
>> 3                                     {C2 product} {default}
>>      intx CMSAbortablePrecleanWaitMillis           =
>> 100                                   {manageable} {default}
>>      bool CMSClassUnloadingEnabled                 =
>> false                                    {product} {command line}
>>      intx ConditionalMoveLimit                     =
>> 3                                  {C2 pd product} {default}
>>    size_t InitialHeapSize                          =
>> 134217728                                {product} {ergonomic}
>>    size_t MaxMetaspaceSize                         =
>> 18446744073709547520                     {product} {default}
>>    size_t NewSize                                  =
>> 1966080                                  {product} {command line,
>> ergonomic}
>> ccstrlist OnError =                                          {product} {default}
>>      intx PSAdaptiveSizePolicyResizeVirtualSpaceAlot =
>> -1                                     {develop} {default}
>>      bool UseAdaptiveGenerationSizePolicyAtMajorCollection =
>> true                             {product} {default}
>>      bool UseCISCSpill                             =
>> true                               {C2 pd develop} {default}
>>      bool UseCLMUL                                 =
>> true                                {ARCH product} {default}
>>      bool UseCompressedOops                        =
>> true                                {lp64_product} {ergonomic}
>>      bool UseConcMarkSweepGC                       =
>> true                                     {product} {command line}
>>
>> This is the old (current) version:
>>
>>     ccstr AbortVMOnExceptionMessage =
>> {diagnostic}
>>     uintx AdaptiveSizeDecrementScaleFactor          =
>> 4                                   {product}
>>      intx AliasLevel                                =
>> 3                                   {C2 product}
>>      intx CMSAbortablePrecleanWaitMillis            =
>> 100                                 {manageable}
>>      bool CMSClassUnloadingEnabled                 :=
>> false                               {product}
>>      intx ConditionalMoveLimit                      =
>> 3                                   {C2 pd product}
>>    size_t InitialHeapSize                          :=
>> 134217728                           {product}
>>    size_t MaxMetaspaceSize                          =
>> 18446744073709547520                    {product}
>>    size_t NewSize                                  :=
>> 1966080                             {product}
>> ccstrlist OnError =                                     {product}
>>      intx PSAdaptiveSizePolicyResizeVirtualSpaceAlot  =
>> -1                                  {develop}
>>      bool UseAdaptiveGenerationSizePolicyAtMajorCollection  =
>> true                                {product}
>>      bool UseCISCSpill                              =
>> true                                {C2 pd develop}
>>      bool UseCompressedOops                        :=
>> true                                {lp64_product}
>>      bool UseConcMarkSweepGC                       :=
>> true                                {product}
>>
>> /Jesper
>>
>>> Coleen
>>>
>>>
>>> On 6/21/16 4:15 PM, Vladimir Kozlov wrote:
>>>> Nobody will remember the meaning of such marking. You should use normal words.
>>>> The output already have flag's type as, for example, "{C2 product}". You can
>>>> add an other word there to indicate type
>>>> of initialization: default|command|ergo.
>>>>
>>>> Thanks,
>>>> Vladimir
>>>>
>>>> On 6/21/16 12:09 PM, Jesper Wilhelmsson wrote:
>>>>> Hi,
>>>>>
>>>>> Please review this change to make -XX:+PrintFlagsFinal distinguish between
>>>>> flags changed on the command line and flags
>>>>> changed by the ergonomics.
>>>>>
>>>>> To enable us to remember if a flag was set on the command line or not after
>>>>> having been changed by the ergonomics I use
>>>>> another bit in the Flag struct.
>>>>>
>>>>> The actual symbols printed by PrintFlagsFinal are chosen somewhat arbitrary
>>>>> and I'm open to suggestions (bike sheding)
>>>>> about how to visualize this the best.
>>>>>
>>>>> The old decorations are:
>>>>>
>>>>> ' =' - Default value (no decoration)
>>>>> ':=' - Value was changed, either by command line or ergonomics or other
>>>>>
>>>>> The new decorations are:
>>>>>
>>>>> ' =' - Default value (no decoration)
>>>>> ':=' - Set on the command line
>>>>> '?=' - Changed by the ergonomics
>>>>> '!=' - Set on the command line AND changed by the ergonomics
>>>>> '-=' - Any other origin, like changed by management API or taken from a
>>>>> config file
>>>>>
>>>>> My reasoning behind selecting these characters are that ':=' looks like a
>>>>> solid assignment and will therefore represent
>>>>> the command line. You never know what you get from the ergonomics, so '?='
>>>>> seems appropriate. '!=' because you'd
>>>>> want to
>>>>> know that the ergonomics changed a value you had set on the command line.
>>>>>
>>>>> Another option could be to use a colon ':=' for the command line, and a
>>>>> comma ',=' for the ergonomics. That would
>>>>> naturally give us the semi-colon ';=' for something set on the command line
>>>>> and changed by the ergonomics. I didn't go
>>>>> with this because the colon and semi-colon can be hard to distinguish from
>>>>> each other.
>>>>>
>>>>> As mentioned above there are other origins, the enum contains eight values.
>>>>> There is no mention of the other types in
>>>>> the bug and there are no is_...() for the other types in globals.cpp, so
>>>>> they didn't seem as important. If the general
>>>>> opinion is that these other origins are as important, or we might as
>>>>> well..., I'd suggest that we use letters as
>>>>> decorations. Let me know if you have an opinion.
>>>>>
>>>>>
>>>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8048093
>>>>> Webrev: http://cr.openjdk.java.net/~jwilhelm/8048093/webrev.00/
>>>>>
>>>>> Thanks,
>>>>> /Jesper
>>>
>>