RFR: 8342089: Require --enable-native-access to be the same between CDS dump time and run time [v4]

David Holmes dholmes at openjdk.org
Mon Nov 25 04:06:16 UTC 2024


On Fri, 22 Nov 2024 20:45:30 GMT, Matias Saavedra Silva <matsaave at openjdk.org> wrote:

>> Applications that use JNI or FFM need to use the `--enable-native-access` flag, or include the Enable-Native-Access attribute in their JAR files. Currently, the CDS archive cannot use optimized module handling when `--enable-native-access` is specified, so such applications do not support CDS. 
>> 
>> This patch no longer disables optimized module graph so long as the `--enable-native-access` is consistent between dump time and runtime. The modules list provided by the option is stored in the RO region of the CDS archive. Verified with tier 1-5 tests and a new regression test.
>
> Matias Saavedra Silva has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Calvin comments

Generally looks good. A couple of nits. 

Thanks

src/hotspot/share/classfile/modules.cpp line 588:

> 586:       disable = true;
> 587:     } else if (strcmp(runtime_flag, *archived_flag) != 0) {
> 588:       log_info(cds)("Mismatched modules for %s: runtime %s dump time %s", property, runtime_flag, *archived_flag);

"Module for" doesn't really seem the right wording here as it can be a list of modules depending on the property. Maybe just say e.g. "Mismatched values for property %s: ..." in each case

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

> 82: #define DEFAULT_JAVA_LAUNCHER _default_java_launcher
> 83: 
> 84: char*  Arguments::_jvm_flags_file                   = nullptr;

This is a great example of why aligning declarations this way is a bad thing - too much incidental change, hiding the real change.

test/hotspot/jtreg/runtime/cds/appcds/jigsaw/module/EnableNativeAccessCDS.java line 69:

> 67:         oa.shouldHaveExitValue(0)
> 68:         .shouldContain("Mismatched modules for jdk.module.enable.native.access: runtime jdk.httpserver dump time java.base")
> 69:         .shouldContain(disabledOptimizedModule);

Please indent to align the dot operator.

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

Changes requested by dholmes (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/22305#pullrequestreview-2457044015
PR Review Comment: https://git.openjdk.org/jdk/pull/22305#discussion_r1855729945
PR Review Comment: https://git.openjdk.org/jdk/pull/22305#discussion_r1855739988
PR Review Comment: https://git.openjdk.org/jdk/pull/22305#discussion_r1855743867


More information about the hotspot-runtime-dev mailing list