RFR: 8316969: Improve CDS module graph support for --module option [v3]

Calvin Cheung ccheung at openjdk.org
Tue Oct 31 20:47:36 UTC 2023


On Tue, 31 Oct 2023 09:09:37 GMT, Alan Bateman <alanb at openjdk.org> wrote:

> Thanks, it looks correctly now.

Thanks!

> 
> One small question. At ModuleBootstrap L235 we set canArchive as it's okay to archive under specific restrictions. For completeness, shouldn't this set canArchive to CDS.isDumpingStaticArchive? I don't think it matters right now but might be confusing to have canArchive be true when not dumping.

Do you mean L230?

 226             if (!haveModulePath && addModules.isEmpty() && limitModules.isEmpty()) {
 227                 systemModules = SystemModuleFinders.systemModules(mainModule);
 228                 if (systemModules != null && !isPatched) {
 229                     needResolution = (traceOutput != null);
 230                     canArchive = true;
 231                 }
 232             }

Do you prefer the `canArchive` setting be inside `if (CDS.isDumpingStaticArchive())` like the following?

    if (CDS.isDumpingStaticArchive())
        canArchive = true;

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

PR Review Comment: https://git.openjdk.org/jdk/pull/16016#discussion_r1378146393


More information about the core-libs-dev mailing list