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

[Jesper Wilhelmsson]

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
>