RFR: 8247449: Revisit the argument processing logic for MetaspaceShared::disable_optimized_module_handling()

[Ioi Lam]

On Tue, 26 Mar 2024 18:58:55 GMT, Calvin Cheung <ccheung at openjdk.org> wrote:

> The changes include:
> 
> - refactor` CDSConfig::check_system_property()` into `CDSConfig:check_internal_module_property()` and `CDSConfig::handle_incompatible_property()`;
> - `CDSConfig:check_internal_module_property()` will be called from `Arguments::create_module_property()` and `Arguments::create_numbered_module_property()`;
> - `CDSConfig::handle_incompatible_property()` will be called from `Arguments::parse_each_vm_init_arg()` during the processing of "-D" properties.
> 
> Passed tiers 1 - 3 testing.

Changes requested by iklam (Reviewer).

src/hotspot/share/runtime/arguments.cpp line 2499:

> 2497:           match_option(option, "-Djdk.module.validation", &value)) {
> 2498:         CDSConfig::handle_incompatible_property((const char*)option->optionString + 2);
> 2499:       }

Since add_property can be called in multiple places, I think this check should be put inside add_property().

Also, for better modularity, the set of properties should be listed inside CDSConfig.cpp. Maybe something like this:


bool Arguments::add_property(...) {
  ...
  CDSConfig::check_incompatible_property(key, value); // checks the 3 properties inside
}

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

PR Review: https://git.openjdk.org/jdk/pull/18495#pullrequestreview-1961821530
PR Review Comment: https://git.openjdk.org/jdk/pull/18495#discussion_r1540163472