RFR: 8311302: Allow for jlinking a custom runtime without packaged modules being present [v10]
Mandy Chung
mchung at openjdk.org
Tue Nov 28 23:22:22 UTC 2023
On Fri, 24 Nov 2023 13:25:35 GMT, Severin Gehwolf <sgehwolf at openjdk.org> wrote:
>> Please review this patch which adds a "jmodless" jlink mode to the JDK. Fundamentally this patch adds an option to use `jlink` even though your JDK install might not come with the packaged modules (directory `jmods`). This is particularly useful to further reduce the size of a jlinked runtime. After the removal of the concept of a JRE, a common distribution mechanism is still the full JDK with all modules and packaged modules. However, packaged modules can incur an additional size tax. For example in a container scenario it could be useful to have a base JDK container including all modules, but without also delivering the packaged modules. This comes at a size advantage of `~25%`. Such a base JDK container could then be used to `jlink` application specific runtimes, further reducing the size of the application runtime image (App + JDK runtime; as a single image *or* separate bundles, depending on the app being modularized).
>>
>> The basic design of this approach is to add a jlink plugin for tracking non-class and non-resource files of a JDK install. I.e. files which aren't present in the jimage (`lib/modules`). This enables producing a `JmodLessArchive` class which has all the info of what constitutes the final jlinked runtime.
>>
>> Basic usage example:
>>
>> $ diff -u <(./bin/java --list-modules --limit-modules java.se) <(../linux-x86_64-server-release/images/jdk/bin/java --list-modules --limit-modules java.se)
>> $ diff -u <(./bin/java --list-modules --limit-modules java.se) <(../linux-x86_64-server-release/images/jdk/bin/java --list-modules --limit-modules jdk.jlink)
>> $ ls ../linux-x86_64-server-release/images/jdk/jmods
>> java.base.jmod java.net.http.jmod java.sql.rowset.jmod jdk.crypto.ec.jmod jdk.internal.opt.jmod jdk.jdi.jmod jdk.management.agent.jmod jdk.security.auth.jmod
>> java.compiler.jmod java.prefs.jmod java.transaction.xa.jmod jdk.dynalink.jmod jdk.internal.vm.ci.jmod jdk.jdwp.agent.jmod jdk.management.jfr.jmod jdk.security.jgss.jmod
>> java.datatransfer.jmod java.rmi.jmod java.xml.crypto.jmod jdk.editpad.jmod jdk.internal.vm.compiler.jmod jdk.jfr.jmod jdk.management.jmod jdk.unsupported.desktop.jmod
>> java.desktop.jmod java.scripting.jmod java.xml.jmod jdk.hotspot.agent.jmod jdk.internal.vm.compiler.management.jmod jdk.jlink.jmod jdk.naming.dns.j...
>
> Severin Gehwolf has updated the pull request incrementally with one additional commit since the last revision:
>
> Tighten ModifiedFilesExitTest
>
> Ensure the error message is reasonable and doesn't include
> Exceptions presented to the user.
src/jdk.jlink/share/classes/jdk/tools/jlink/internal/TaskHelper.java line 452:
> 450: String addOptionsGlob = "glob:" + AddOptionsPlugin.OPTS_FILE;
> 451: String saveJlinkOptsGlob = "glob:/jdk.jlink/" + JlinkTask.OPTIONS_RESOURCE;
> 452: String additionalPatterns = systemModulesPattern + "," +
Suggest to have each plugin to define `Plugin::excludeResourcesPattern` to return a non-empty string if linking from the run-time image. Construct this pattern using the API.
src/jdk.jlink/share/classes/jdk/tools/jlink/internal/TaskHelper.java line 475:
> 473: // SystemModulesMap class isn't guaranteed to be correct for the
> 474: // current module set.
> 475: if (systemModulesPlugin == null) {
Suggest to fail if system modules plugin does not exist which should not happen. The `disableFastPath` system property was added for testing only.
src/jdk.jlink/share/classes/jdk/tools/jlink/internal/plugins/AddRunImageResourcesPlugin.java line 56:
> 54: * resources. Needed for the the run-time image based jlink.
> 55: */
> 56: public final class AddRunImageResourcesPlugin extends AbstractPlugin {
This resource file is generated if jdk.jlink is linked in the resulting image. Maybe this plugin can be named `GenerateJlinkResourcesListPlugin` or `JlinkResourcesListPlugin`???
src/jdk.jlink/share/classes/jdk/tools/jlink/internal/plugins/AddRunImageResourcesPlugin.java line 67:
> 65: // RunImageArchive for further processing.
> 66: private static final String RESPATH = RESPATH_PREFIX + "%s_resources";
> 67: private static final String JLINK_MOD_NAME = "jdk.jlink";
There are only 2 occurrences of "jdk.jlink". I guess you plan to replace them with the constant variable?
src/jdk.jlink/share/classes/jdk/tools/jlink/internal/plugins/AddRunImageResourcesPlugin.java line 102:
> 100: Platform targetPlatform = getTargetPlatform(in);
> 101: in.transformAndCopy(e -> { ResourcePoolEntry retval = recordAndFilterEntry(e, targetPlatform);
> 102: return retval;}, out);
Suggestion:
in.transformAndCopy(e -> recordAndFilterEntry(e, targetPlatform), out);
src/jdk.jlink/share/classes/jdk/tools/jlink/internal/plugins/AddRunImageResourcesPlugin.java line 116:
> 114: if (platform == null) {
> 115: throw new IllegalStateException("java.base not part of the image?");
> 116: }
Can simply use `orElseThrow`. It should not reach here and so it's ok to use InternalError or AssertionError (which is also used in this code).
Suggestion:
String platform = in.moduleView().findModule("java.base")
.map(ResourcePoolModule::targetPlatform)
..orElseThrow(() -> new AssertionError("java.base not found"));
src/jdk.jlink/share/classes/jdk/tools/jlink/internal/plugins/AddRunImageResourcesPlugin.java line 121:
> 119:
> 120: private void addModuleResourceEntries(ResourcePoolBuilder out) {
> 121: for (String module: keysInSortedOrder()) {
Suggestion:
nonClassResEntries.keySet().stream().sorted().forEach(module -> {
`keysInSortedOrder` can be removed.
src/jdk.jlink/share/classes/jdk/tools/jlink/internal/plugins/AddRunImageResourcesPlugin.java line 126:
> 124: if (mResources == null) {
> 125: throw new AssertionError("Module listed, but no resources?");
> 126: }
Since it is always non-null, this check can be dropped. NPE will be thrown if such bug exists.
src/jdk.jlink/share/classes/jdk/tools/jlink/internal/plugins/AddRunImageResourcesPlugin.java line 159:
> 157: // Filter internal runtime image based link resource file which we
> 158: // create later on-the-fly
> 159: return null;
why this one is not excluded in the same way as other plugins?
src/jdk.jlink/share/classes/jdk/tools/jlink/internal/plugins/AddRunImageResourcesPlugin.java line 184:
> 182: } catch (RunImageLinkException e) {
> 183: // RunImageArchive::RunImageFile.content() may throw this when
> 184: // getting the content(). Propagate this specific exeption.
Suggestion:
// getting the content(). Propagate this specific exception.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/14787#discussion_r1408528283
PR Review Comment: https://git.openjdk.org/jdk/pull/14787#discussion_r1408529843
PR Review Comment: https://git.openjdk.org/jdk/pull/14787#discussion_r1408525936
PR Review Comment: https://git.openjdk.org/jdk/pull/14787#discussion_r1408457185
PR Review Comment: https://git.openjdk.org/jdk/pull/14787#discussion_r1408453177
PR Review Comment: https://git.openjdk.org/jdk/pull/14787#discussion_r1408463342
PR Review Comment: https://git.openjdk.org/jdk/pull/14787#discussion_r1408470820
PR Review Comment: https://git.openjdk.org/jdk/pull/14787#discussion_r1408469356
PR Review Comment: https://git.openjdk.org/jdk/pull/14787#discussion_r1408472411
PR Review Comment: https://git.openjdk.org/jdk/pull/14787#discussion_r1408475136
More information about the core-libs-dev
mailing list