RFR: 8319999: Refactor MetaspaceShared::use_full_module_graph() [v2]

[Ioi Lam]

On Tue, 14 Nov 2023 05:42:56 GMT, David Holmes <dholmes at openjdk.org> wrote:

>> But the context is different. `enable_xxx()` indicates that the function enables xxx. The variable that holds the enabled/disabled state of xxx should reflect in its name that it holds that state so `_xxx_enabled` Or `_has_xxx`, or `_is_xxx`, depending on what `xxx` actually is.
>
> Or `_use_xxx` as was originally used here. I guess I just find it particularly irksome that `enable` is often used where `enabled` would be the grammatically correct choice.

I don't think enable_xxx is grammatically wrong, when it's read as "(we should) enable xxx". We have hundreds of global flags that start with Use/Enable/Disable, like EnableDynamicAgentLoading and UseTLAB.

Anyway, the reason I didn't use `_is_loading_full_module_graph` is that I want a flag that unconditionally disables the feature, but this flag alone doesn't guarantee that the feature exists.

I changed the flag to `_loading_full_module_graph_disabled` and added comments to make it clear.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/16646#discussion_r1392036140