RFR: 8328313: Archived module graph should allow identical --module-path to be specified during dump time and run time
David Holmes
dholmes at openjdk.org
Wed Sep 18 01:37:04 UTC 2024
On Tue, 17 Sep 2024 23:44:40 GMT, Calvin Cheung <ccheung at openjdk.org> wrote:
> Prior to this patch, if `--module-path` is specified in the command line:
> during CDS dump time, full module graph will not be included in the CDS archive;
> during run time, full module graph will not be used.
>
> With this patch, the full module graph will be included in the CDS archive with the `--module-path` option. During run time, if the same `--module-path` option is specified, the archived module graph will be used.
>
> The checking of module paths between dump time and run time is more lenient compared with the checking of class paths; the ordering of the modules is unimportant, duplicate module names are ignored.
> E.g. the following is considered a match:
> dump time runtime
> m1,m2 m2,m1
> m1,m2 m1,m2,m2
>
> I included some [notes](https://bugs.openjdk.org/browse/JDK-8328313?focusedId=14699275&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-14699275) in the bug report regarding some changes in the corelib classes.
I've taken an initial pass through and this seems reasonable (a bit more complicated than the description suggested :) ).
Thanks
src/hotspot/share/cds/filemap.cpp line 931:
> 929: bool FileMapInfo::is_jar_suffix(const char* filename) {
> 930: const char* dot = strrchr(filename, '.');
> 931: if (strcmp(dot + 1, "jar") == 0) {
What if there is no dot? We need a null check.
src/hotspot/share/cds/filemap.hpp line 558:
> 556: unsigned int dumptime_prefix_len,
> 557: unsigned int runtime_prefix_len) NOT_CDS_RETURN_(false);
> 558: bool is_jar_suffix(const char* filename);
Suggestion: has_jar_suffix
src/hotspot/share/cds/heapShared.cpp line 884:
> 882: ClassLoaderExt::num_module_paths() > 0) {
> 883: log_info(cds, heap)(" is_using_optimized_module_handling %d num_module_paths %d jdk.module.main %s",
> 884: CDSConfig::is_using_optimized_module_handling(), ClassLoaderExt::num_module_paths(), Arguments::get_property("jdk.module.main"));
Why are you printing a bool value as an int? I'm surprised one of the format checkers doesn't complain about it.
-------------
PR Review: https://git.openjdk.org/jdk/pull/21048#pullrequestreview-2311418590
PR Review Comment: https://git.openjdk.org/jdk/pull/21048#discussion_r1764261758
PR Review Comment: https://git.openjdk.org/jdk/pull/21048#discussion_r1764267214
PR Review Comment: https://git.openjdk.org/jdk/pull/21048#discussion_r1764268462
More information about the hotspot-dev
mailing list