JDK-8247449 : Revisit the argument processing logic for MetaspaceShared::disable_optimized_module_handling()
  • Type: Enhancement
  • Component: hotspot
  • Sub-Component: runtime
  • Affected Version: 16
  • Priority: P4
  • Status: Resolved
  • Resolution: Fixed
  • Submitted: 2020-06-12
  • Updated: 2024-04-04
  • Resolved: 2024-03-28
The Version table provides details related to the release that this issue/RFE will be addressed.

Unresolved : Release in which this issue/RFE will be addressed.
Resolved: Release in which this issue/RFE has been resolved.
Fixed : Release in which this issue/RFE has been fixed. The release containing this fix may be available for download as an Early Access Release or a General Availability Release.

To download the current JDK release, click here.
JDK 23
23 b17Fixed
Related Reports
Relates :  
JDK-8266329 - Improve CDS module graph support for command-line options
Relates :  
JDK-8240245 - Avoid calling is_shared_class_visible() in SystemDictionary::load_shared_class()
Relates :  
JDK-8247388 - Minimal build broken after JDK-8240245 (undefined reference to `MetaspaceShared::_use_optimized_module_handling')
Description
JDK-8240245 added an optimization for module handling which has to be disabled if certain command-line flags are specified in relation to the module system or the boot classpath. For the module system flags the following code was added to Arguments::add_property:

   if (is_internal_module_property(key) ||
       strcmp(key, "jdk.module.main") == 0) {
     MetaspaceShared::disable_optimized_module_handling();
     log_info(cds)("Using optimized module handling disabled due to incompatible property: %s=%s", key, value);
   }

There are a couple of issues with this code placement:

1. We've added module specific property handling to general purpose code, when we already have module specific methods:  Arguments::create_module_property and Arguments::create_numbered_module_property. The logic should be handled in one or both of those methods as applicable.

2. The check is_internal_module_property(key) will be true for all module related properties, but it is far from clear that the intent is to disable the optimisation if any module related flag is provided. So this check seems far too broad. We may be getting away with this because when called from Arguments::create_numbered_module_property we have already appended a number to the key and so AFAICS we will fail to match.

This new code is also specific to a build with INCLUDE_CDS defined and we should ensure we use the most effective mechanism to remove this code in non-CDS builds (ref: JDK-8247388).
Comments
Changeset: 85cb4a99 Author: Calvin Cheung <ccheung@openjdk.org> Date: 2024-03-28 15:03:09 +0000 URL: https://git.openjdk.org/jdk/commit/85cb4a9942f31ec99dae9f6a8af4f1fd4e8195bd
28-03-2024
A pull request was submitted for review. URL: https://git.openjdk.org/jdk/pull/18495 Date: 2024-03-26 18:58:55 +0000
26-03-2024
Since the fix for JDK-8316969, we allow -Djdk.module.main for CDS optimized module handling. So we just need to enhance the argument processing of other module properties in this RFE.
26-03-2024
-Djdk.module.main can be special-cased higher-up when we process "-D", where we already have a couple of special cases. But for the other module properties they would be better handled inside the module specific functions, otherwise you waste time calling is_internal_module_property(key) on every -Dxxx value. If you know which module options are a problem you should do the check where those flags are explicitly processed: } else if (match_option(option, "--add-reads=", &tail)) { if (!create_numbered_module_property("jdk.module.addreads", tail, addreads_count++)) { return JNI_ENOMEM; } } else if (match_option(option, "--add-exports=", &tail)) { if (!create_numbered_module_property("jdk.module.addexports", tail, addexports_count++)) { return JNI_ENOMEM; } } else if (match_option(option, "--add-opens=", &tail)) { if (!create_numbered_module_property("jdk.module.addopens", tail, addopens_count++)) { return JNI_ENOMEM; } etc.
12-06-2020
One reason to do it inside Arguments::add_property is when we specify a main module with something like "java -m jdk.compiler/com.sun.tools.javac.Main", we are called with this stack: #0 Arguments::add_property (prop=0x555555571852 "jdk.module.main=jdk.compiler", writeable=Arguments::WriteableProperty, internal=Arguments::ExternalProperty) #1 Arguments::parse_each_vm_init_arg (args=..., patch_mod_javabase=..., origin=JVMFlag::COMMAND_LINE) #2 Arguments::parse_vm_init_args (vm_options_args=..., java_tool_options_args=..., java_options_args=..., #3 Arguments::parse (initial_cmd_args=...) #4 Threads::create_vm (args=..., canTryAgain=...)cpp:3789 #5 JNI_CreateJavaVM_inner (vm=..., penv=..., args=...) ... which bypasses all the Arguments::create_xxx_module_property() functions. The "-m" is handled by the Java launcher and decomposed to -Djdk.module.main=jdk.compiler and -Dsun.java.command=jdk.compiler/com.sun.tools.javac.Main http://hg.openjdk.java.net/jdk/jdk/file/1d45272feec9/src/java.base/share/native/libjli/java.c#l1308 Also, these two properties are not considered is_internal_module_property(), as you can legally specify them in the command-line $ java -Djdk.module.main=jdk.compiler -Dsun.java.command=jdk.compiler/com.sun.tools.javac.Main Do we must cover the case when -Djdk.module.main=jdk.compiler is specified by the user on command-line. ========== The goal of MetaspaceShared::disable_optimized_module_handling in JDK-8247449 is to really disable the optimizations when *any* module-related options are specified during either dump time or run time. It's hard to analyze the effect when different combinations of these options are specified. For now, the only "optimized_module_handling" we do is to skip the SystemDictionary::is_shared_class_visible call. We are also planning to use the same check with JDK-8244778 Archive full module graph in CDS. ========== In the future, we can allow "optimized_module_handling" in more cases. For example, when the exact same set of module-related options are specified at both dump time and run time.
12-06-2020