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

[David Holmes]

On Tue, 14 Nov 2023 05:15:24 GMT, Ioi Lam <iklam at openjdk.org> wrote:

>> src/hotspot/share/cds/cdsConfig.cpp line 34:
>> 
>>> 32: bool CDSConfig::_is_dumping_dynamic_archive = false;
>>> 33: bool CDSConfig::_enable_dumping_full_module_graph = true;
>>> 34: bool CDSConfig::_enable_loading_full_module_graph = true;
>> 
>> The "enable" naming isn't quite right - these fields hold the enabled state e.g. `_dumping_full_module_graph_enabled`.
>
> I like the variable name to be the same as the function name, so it's easier to search. We also have `bool _enable_preview;` in arguments.hpp

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.

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

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