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

[Vladimir Kozlov]

why you skipped 19?:

ORIG_COMMAND_LINE       = 1 << 20,

Otherwise looks good.

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
>>
>