On Tue, 17 Sep 2024 23:44:40 GMT, Calvin Cheung <cche...@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
