On Mon, 2 Oct 2023 22:17:34 GMT, Calvin Cheung <cche...@openjdk.org> wrote:
> Please review this changeset for adding support for `--module` (-m) option > for CDS. > Changes in the `ModuleBootstrap.java` are needed so that the > `ArchivedModuleGraph.archive` and `ArchivedBootLayer.archive` are called if > the main module is specified. The module name will be stored in the ro region > of the CDS archive. During runtime, the archived module name will be compared > with the runtime module name. If comparison fails, the archived full module > graph won't be used. > > Note: this RFE is a subtask of > [JDK-8266329](https://bugs.openjdk.org/browse/JDK-8266329). More subtask(s) > will be created to support other options such as `--add-modules`. > > Passed tiers 1 - 4 testing. Changes requested by iklam (Reviewer). src/hotspot/share/classfile/modules.cpp line 597: > 595: MetaspaceShared::disable_optimized_module_handling(); > 596: } > 597: } I think this code can be made less verbose: bool disable = false; if (runtime_main_module == nullptr) { if (_archived_main_module_name != nullptr) { log_info(cds)("Module %s specified during dump time but not during runtime", _archived_main_module_name); disable = true; } } else { if (_archived_main_module_name == nullptr) { log_info(cds)("Module %s specified during runtime but not during dump time", runtime_main_module); disable = true; } else if (strcmp(runtime_main_module, _archived_main_module_name) != 0) { log_info(cds)("Mismatched modules: runtime %s dump time %s", runtime_main_module, _archived_main_module_name); disable = true; } } if (disable) { log_info(cds)("Disabling optimized module handling"); MetaspaceShared::disable_optimized_module_handling(); } test/hotspot/jtreg/runtime/cds/appcds/jigsaw/module/ModuleOption.java line 60: > 58: .shouldMatch("cds,module.*Restored from archive: entry.0x.*name > jdk.compiler"); > 59: > 60: // different module specified during runtime Maybe add a comment that jdk.httpserver is special as it specifies its main class, so it causes a different code path to be used in ModuleBootstrap than jdk.compiler? Also, maybe add an additional test case with `-m jdk.httpserver` (without specifying the main class). ------------- PR Review: https://git.openjdk.org/jdk/pull/16016#pullrequestreview-1660271426 PR Review Comment: https://git.openjdk.org/jdk/pull/16016#discussion_r1347686444 PR Review Comment: https://git.openjdk.org/jdk/pull/16016#discussion_r1347690405
