RFR: 8316969: Improve CDS module graph support for --module option

Ioi Lam iklam at openjdk.org
Tue Oct 17 22:51:49 UTC 2023


On Mon, 2 Oct 2023 22:17:34 GMT, Calvin Cheung <ccheung at 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


More information about the core-libs-dev mailing list