Re: RFR: 8277013: Allow creation of module graphs without defining modules to the VM
On Thu, 11 Nov 2021 14:53:13 GMT, Ivan Ristović wrote: > Related JBS issue: https://bugs.openjdk.java.net/browse/JDK-8277013 > > The GraalVM Native Image module support must create the runtime boot-module > layer at image build time; module instances created at build time need to > reflect module relations at runtime, i.e., their opens, reads, and exports. > To achieve this, we had to duplicate and modify a significant portion of the > module synthesis methods from the JDK. The GraalVM PR that demonstrates this > duplication can be found at: > > https://github.com/oracle/graal/blob/9bfa3a312d7d0309eb014d52e49afd7136d9e77e/substratevm/src/com.oracle.svm.hosted.jdk11/src/com/oracle/svm/hosted/ModuleLayerFeature.java#L269 > > The hard-to-maintain code duplication is necessary as synthesizing module > graphs in the JDK unconditionally defines modules to the VM which causes an > error due to module redefinition. For example, methods `System.defineModule`, > `ModuleLayer` factories and constructor, and `Module` constructor > unconditionally update the VM state. All these methods eventually reach the > `Module` constructor or the `Modules.defineModules` method. > > We propose that the `Module` constructor and the `Modules.defineModules` > conditionally update the VM state (similarly to 'Module.implAddReads`). The > JDK would call these methods so they update the VM state and GraalVM Native > Image would call them without updating the state. The internal API for modules already allows conditional VM updates used for white-box testing (e.g., `Module.implAddReadsNoSync()` and `Module.implAddExportsNoSync()`). This PR adds similar functionality to the module constructor and the module graph synthesizer and as such is quite consistent with the existing code. Would the appropriate cleanup be to add white-box tests for the newly added methods? This would increase the test coverage, and at the same time avoid bit rot of the methods that Native Image requires? - PR: https://git.openjdk.java.net/jdk/pull/6356
RFR: 8277013: Allow creation of module graphs without defining modules to the VM
Related JBS issue: https://bugs.openjdk.java.net/browse/JDK-8277013 The GraalVM Native Image module support must create the runtime boot-module layer at image build time; module instances created at build time need to reflect module relations at runtime, i.e., their opens, reads, and exports. To achieve this, we had to duplicate and modify a significant portion of the module synthesis methods from the JDK. The GraalVM PR that demonstrates this duplication can be found at: https://github.com/oracle/graal/blob/9bfa3a312d7d0309eb014d52e49afd7136d9e77e/substratevm/src/com.oracle.svm.hosted.jdk11/src/com/oracle/svm/hosted/ModuleLayerFeature.java#L269 The hard-to-maintain code duplication is necessary as synthesizing module graphs in the JDK unconditionally defines modules to the VM which causes an error due to module redefinition. For example, methods `System.defineModule`, `ModuleLayer` factories and constructor, and `Module` constructor unconditionally update the VM state. All these methods eventually reach the `Module` constructor or the `Modules.defineModules` method. We propose that the `Module` constructor and the `Modules.defineModules` conditionally update the VM state (similarly to 'Module.implAddReads`). The JDK would call these methods so they update the VM state and GraalVM Native Image would call them without updating the state. - Commit messages: - Update parameter names; Add constructor overload for backward compatibility - Enhance Module.defineModules to accept switches to update VM state Changes: https://git.openjdk.java.net/jdk/pull/6356/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=6356=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8277013 Stats: 52 lines in 1 file changed: 36 ins; 0 del; 16 mod Patch: https://git.openjdk.java.net/jdk/pull/6356.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/6356/head:pull/6356 PR: https://git.openjdk.java.net/jdk/pull/6356
