RFR: 8312585: Rename DisableTHPStackMitigation flag to THPStackMitigation

David Holmes dholmes at openjdk.org
Mon Jul 24 23:29:40 UTC 2023


On Mon, 24 Jul 2023 09:02:59 GMT, Aleksey Shipilev <shade at openjdk.org> wrote:

> The flag newly added by [JDK-8312182](https://bugs.openjdk.org/browse/JDK-8312182) is prompting the use of `-XX:+DisableTHPStackMitigation` to disable the THP stack mitigation, thus allowing THP in thread stacks. This double negation does not read well, and not in line with other mitigation flags like `IntelJccErratumMitigation`.
> 
> It would be better to rename the flag to avoid double-negation, before it proliferates to other JDK releases.
> 
> (I would have the same comment during the original review, but missed it :P)
> 
> Additional testing: 
>  - [ ] GHA
>  - [x] Linux x86_64 fastdebug, affected test

I understand how you see it this way but that is, I presume, because you read the +/- as enable/disable. But `+Foo` simply means "set Foo to true". `-Foo` means "set Foo to false". If you read it that way there is no confusion: `-DisableTHPStackMitigation` means `DisableTHPStackMitigation==false` ie we are not disabling the mitigation, hence it is enabled.

To me the use of Enable/Disable can convey additional information about the nature of the flag - if the flag is DisableX then I would expect X to normally be enabled but in some rare cases we may need to disable it, hence the flag conveys that. And yes you can abuse that by having the flag and the default value of the flag the wrong way around - such is life. To me if the flag is just X then it suggests, albeit there must be a default for X, that there is some ambivalence about whether X should be enabled or not - "hey X is enabled by default, but if you want to turn it off that's fine, your call."

I fully understand both points of view here, but there is plenty of precedence for using Use/Enable/Disable prefixes on flags. And Use in particular is far too entrenched to be displaced.

-------------

PR Comment: https://git.openjdk.org/jdk/pull/14992#issuecomment-1648754807


More information about the hotspot-runtime-dev mailing list