RFR: 8048093 - Explicitly setting := vs = in the -XX:+PrintFlagsFinal output
Jesper Wilhelmsson
jesper.wilhelmsson at oracle.com
Tue Jul 5 19:09:39 UTC 2016
Den 5/7/16 kl. 20:56, skrev Vladimir Kozlov:
> In CheckCompileThresholdScaling.java fix spacing for "double
> CompileThresholdScaling" lines (it was wrong in original code too).
Are you referring to the number of spaces between the flag and the '='? It
matches the output from PrintFlagsFinal. It looks skewed in the test because in
the test the lines are aligned starting at the first character in the type name,
but in the flag output it is aligned on the space between the type and the flag
name:
intx CompileThreshold = 10000
double CompileThresholdScaling = 1.000000
Please clarify if this was not what you meant.
>
> Otherwise looks good. No need for updated webrev.
Thanks!
/Jesper
>
> Thanks,
> Vladimir
>
> On 6/29/16 9:55 AM, Jesper Wilhelmsson wrote:
>> 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