RFR: 8048093 - Explicitly setting := vs = in the -XX:+PrintFlagsFinal output
Jesper Wilhelmsson
jesper.wilhelmsson at oracle.com
Wed Jun 29 16:55:15 UTC 2016
After some more testing I made a few smaller updates:
* Using jio_snprintf instead of snprintf in Flag::print_kind_and_origin()
Looking at other code, it seemed to be the right thing to do.
* Using size_t instead of int for the size and used counter in
Flag::print_kind_and_origin()
Windows compiler complained.
* ORIG_COMMAND_LINE = 1 << 19
* Fixed some tests that was surprised by the new output from PrintFlagsFinal.
New webrev:
http://cr.openjdk.java.net/~jwilhelm/8048093/webrev.02/
Thanks,
/Jesper
Den 23/6/16 kl. 20:54, skrev 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
>>>>
>>>
More information about the hotspot-dev
mailing list