RFR: 8048093 - Explicitly setting := vs = in the -XX:+PrintFlagsFinal output
Vladimir Kozlov
vladimir.kozlov at oracle.com
Tue Jul 5 18:56:55 UTC 2016
In CheckCompileThresholdScaling.java fix spacing for "double
CompileThresholdScaling" lines (it was wrong in original code too).
Otherwise looks good. No need for updated webrev.
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