RFR: 8048093 - Explicitly setting := vs = in the -XX:+PrintFlagsFinal output
Jesper Wilhelmsson
jesper.wilhelmsson at oracle.com
Wed Jul 6 15:22:09 UTC 2016
Den 6/7/16 kl. 17:09, skrev Gerard Ziemski:
> hi Jesper,
>
> I’m currently in the process of reviewing your change.
Thanks!
>
> Can you help me and provide me with a test case, where a command line option is later overridden by ergonomic code (ie. where we see “command line,” followed by different origin in the output)?
The two compiler tests that I change in this patch both exhibit this behavior.
If you want something to test on the command line you can set some heap flag to
an unaligned value for instance:
$ java -XX:+PrintFlagsFinal -XX:InitialHeapSize=3000000 -version
size_t InitialHeapSize = 4194304
{product} {command line, ergonomic}
Thanks,
/Jesper
>
>
> cheers
>
>> On Jun 29, 2016, at 11:55 AM, Jesper Wilhelmsson <jesper.wilhelmsson at oracle.com> 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