Re: RFR: 8311302: Allow for jlinking a custom runtime without packaged modules being present [v24]
On Fri, 5 Apr 2024 09:31:18 GMT, Severin Gehwolf wrote: >> Kind of aligning with the "Donaudampfschiffahrtsgesellschaftskapitän" >> prejudice of German. ;-) >> >> >> >> (In Sweden, we have "flaggstångsknoppsförgyllare" so you are not alone) > > Hah! My kids just recently informed me about > "Donaudampfschiffahrtsgesellschaftskapitänsmützenproductionsstätte"... or > whatever else you can come up with :) Hmm, that's nothing. Look up Rhabarberbarbara on YouTube. - PR Review Comment: https://git.openjdk.org/jdk/pull/14787#discussion_r1553444805
Re: RFR: 8311302: Allow for jlinking a custom runtime without packaged modules being present [v24]
On Fri, 5 Apr 2024 08:21:09 GMT, Severin Gehwolf wrote: >> Just to clarify: I did not say the name needed to be long, just that many >> (if not all) tools has used the convention of using the package name >> `build.tools.` and the class name `.java`. >> >> I think the new name sounds . > > Thanks. Yes, the long name was my doing. Sorry. Kind of aligning with the "Donaudampfschiffahrtsgesellschaftskapitän" prejudice of German. ;-) (In Sweden, we have "flaggstångsknoppsförgyllare" so you are not alone) - PR Review Comment: https://git.openjdk.org/jdk/pull/14787#discussion_r1553253017
Re: RFR: 8311302: Allow for jlinking a custom runtime without packaged modules being present [v24]
On Fri, 5 Apr 2024 09:25:49 GMT, Magnus Ihse Bursie wrote: >> Thanks. Yes, the long name was my doing. Sorry. > > Kind of aligning with the "Donaudampfschiffahrtsgesellschaftskapitän" > prejudice of German. ;-) > > > > (In Sweden, we have "flaggstångsknoppsförgyllare" so you are not alone) Hah! My kids just recently informed me about "Donaudampfschiffahrtsgesellschaftskapitänsmützenproductionsstätte"... or whatever else you can come up with :) - PR Review Comment: https://git.openjdk.org/jdk/pull/14787#discussion_r1553260930
Re: RFR: 8311302: Allow for jlinking a custom runtime without packaged modules being present [v24]
On Thu, 4 Apr 2024 20:56:02 GMT, Magnus Ihse Bursie wrote: >> I was not aware of such a convention and I can't say I agree with it. It >> just seems redundant and unnecessary, but I suppose we should wait for >> Magnus to respond. > > Just to clarify: I did not say the name needed to be long, just that many (if > not all) tools has used the convention of using the package name > `build.tools.` and the class name `.java`. > > I think the new name sounds . Thanks. Yes, the long name was my doing. Sorry. - PR Review Comment: https://git.openjdk.org/jdk/pull/14787#discussion_r1553173105
Re: RFR: 8311302: Allow for jlinking a custom runtime without packaged modules being present [v24]
On Thu, 4 Apr 2024 15:38:36 GMT, Erik Joelsson wrote: >> FYI: This was trying to address a comment from @magicus >> https://github.com/openjdk/jdk/pull/14787#discussion_r1514894494 Happy to >> not follow that convention and/or rename the tool. > > I was not aware of such a convention and I can't say I agree with it. It just > seems redundant and unnecessary, but I suppose we should wait for Magnus to > respond. Just to clarify: I did not say the name needed to be long, just that many (if not all) tools has used the convention of using the package name `build.tools.` and the class name `.java`. I think the new name sounds . - PR Review Comment: https://git.openjdk.org/jdk/pull/14787#discussion_r1552430689
Re: RFR: 8311302: Allow for jlinking a custom runtime without packaged modules being present [v24]
On Thu, 4 Apr 2024 15:24:53 GMT, Severin Gehwolf wrote: >> make/CompileToolsJdk.gmk line 50: >> >>> 48: EXCLUDES := \ >>> 49: build/tools/classlist \ >>> 50: build/tools/runtimeimagelinkdeltaproducer \ >> >> This directory name is comically long. I guess that's not really a problem, >> but perhaps "linkdelta" would be descriptive enough given that the class has >> the full name anyway? > > FYI: This was trying to address a comment from @magicus > https://github.com/openjdk/jdk/pull/14787#discussion_r1514894494 Happy to not > follow that convention and/or rename the tool. I was not aware of such a convention and I can't say I agree with it. It just seems redundant and unnecessary, but I suppose we should wait for Magnus to respond. - PR Review Comment: https://git.openjdk.org/jdk/pull/14787#discussion_r1551940327
Re: RFR: 8311302: Allow for jlinking a custom runtime without packaged modules being present [v24]
On Thu, 4 Apr 2024 12:56:41 GMT, Erik Joelsson wrote: >> Severin Gehwolf has updated the pull request with a new target base due to a >> merge or a rebase. The pull request now contains 100 commits: >> >> - Fix coment >> - Fix comment >> - Fix typo >> - Revert some now unneded build changes >> - Follow build tools naming convention >> - Update to new build-time approach with delta in lib >> - Make generation of fs_$module_files unconditional >> - Merge branch 'master' into jdk-8311302-jmodless-link >> - Fix copyright year >> - Move CreateLinkableRuntimePlugin to build folder >> >>Keep runtime link supporting classes in package >>jdk.tools.jlink.internal.runtimelink >> - ... and 90 more: https://git.openjdk.org/jdk/compare/6ae1cf12...ce04f42a > > make/CompileToolsJdk.gmk line 50: > >> 48: EXCLUDES := \ >> 49: build/tools/classlist \ >> 50: build/tools/runtimeimagelinkdeltaproducer \ > > This directory name is comically long. I guess that's not really a problem, > but perhaps "linkdelta" would be descriptive enough given that the class has > the full name anyway? FYI: This was trying to address a comment from @magicus https://github.com/openjdk/jdk/pull/14787#discussion_r1514894494 Happy to not follow that convention and/or rename the tool. - PR Review Comment: https://git.openjdk.org/jdk/pull/14787#discussion_r1551909878
Re: RFR: 8311302: Allow for jlinking a custom runtime without packaged modules being present [v24]
On Thu, 4 Apr 2024 13:00:33 GMT, Erik Joelsson wrote: >> Severin Gehwolf has updated the pull request with a new target base due to a >> merge or a rebase. The pull request now contains 100 commits: >> >> - Fix coment >> - Fix comment >> - Fix typo >> - Revert some now unneded build changes >> - Follow build tools naming convention >> - Update to new build-time approach with delta in lib >> - Make generation of fs_$module_files unconditional >> - Merge branch 'master' into jdk-8311302-jmodless-link >> - Fix copyright year >> - Move CreateLinkableRuntimePlugin to build folder >> >>Keep runtime link supporting classes in package >>jdk.tools.jlink.internal.runtimelink >> - ... and 90 more: https://git.openjdk.org/jdk/compare/6ae1cf12...ce04f42a > > make/Images.gmk line 208: > >> 206: WARN := Creating CDS$$($1_$2_DUMP_TYPE) archive for jdk image for >> $1, \ >> 207: INFO := Using CDS flags for $1: $$($1_$2_CDS_DUMP_FLAGS), \ >> 208: DEPS := $$(FINAL_JDK_JLINK), \ > > Does this actually interfere with the cds archive creation? I would assume > they output to different files and they aren't even running `java` from the > same image. If not, I would skip the whole `FINAL_JDK_JLINK` and just add > `$(diff_runtime_jdk)` to `JDK_TARGETS`. You are right. It no longer does (the old approach did, though). I'll clean it up. - PR Review Comment: https://git.openjdk.org/jdk/pull/14787#discussion_r1551898463
Re: RFR: 8311302: Allow for jlinking a custom runtime without packaged modules being present [v24]
On Thu, 4 Apr 2024 11:12:43 GMT, Severin Gehwolf wrote: >> Please review this patch which adds a jlink mode to the JDK which doesn't >> need the packaged modules being present. A.k.a run-time image based jlink. >> 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 `JRTArchive` >> 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 jdk.jlink) >> <(../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.jmodjava.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.jmodjava.prefs.jmod java.transaction.xa.jmod >> jdk.dynalink.jmod jdk.internal.vm.ci.jmod >> jdk.jdwp.agent.jmod jdk.management.jfr.jmodjdk.security.jgss.jmod >> java.datatransfer.jmodjava.rmi.jmodjava.xml.crypto.jmod >> jdk.editpad.jmod jdk.internal.vm.compiler.jmod >> jdk.jfr.jmod jdk.management.jmodjdk.unsupported.desktop.jmod >> java.desktop.jmod java.scripting.jmod java.xml.jmod >> jdk.hotspot.agent.jmod jdk.i... > > Severin Gehwolf has updated the pull request with a new target base due to a > merge or a rebase. The pull request now contains 100 commits: > > - Fix coment > - Fix comment > - Fix typo > - Revert some now unneded build changes > - Follow build tools naming convention > - Update to new build-time approach with delta in lib > - Make generation of fs_$module_files unconditional > - Merge branch 'master' into jdk-8311302-jmodless-link > - Fix copyright year > - Move CreateLinkableRuntimePlugin to build folder > >Keep runtime link supporting classes in package >jdk.tools.jlink.internal.runtimelink > - ... and 90 more: https://git.openjdk.org/jdk/compare/6ae1cf12...ce04f42a The new approach certainly makes the build part simpler, which I appreciate. Left some polishing comments. You don't need to address them until the general approach is accepted. make/CompileToolsJdk.gmk line 50: > 48: EXCLUDES := \ > 49: build/tools/classlist \ > 50: build/tools/runtimeimagelinkdeltaproducer \ This directory name is comically long. I guess that's not really a problem, but perhaps "linkdelta" would be descriptive enough given that the class has the full name anyway? make/Images.gmk line 114: > 112: ifeq ($(JLINK_PRODUCE_RUNTIME_LINK_JDK), true) > 113: RL_BUILD_CLASSES := runtimeimagelinkdeltaproducer-classes > 114: RL_DELTA_GEN_CLASSES := $(BUILDTOOLS_OUTPUTDIR)/$(RL_BUILD_CLASSES) With a shorter name, this could be just one line. make/Images.gmk line 119: > 117: RL_DIFFS_OUTPUT_FILE_ARG := > $(JDK_IMAGE_DIR)/lib/runtime-image-link.delta > 118: RL_MOD_PATH_ARG := $(IMAGES_OUTPUTDIR)/jmods > 119: TOOL_RUNTIME_IMAGE_LINK_DELTA_PRODUCER := $(BUILD_JAVA) --add-modules > jdk.jlink \ All of these are only used once so should rather be inlined. I think that makes it easier to understand and read. make/Images.gmk line 122: > 120: > --add-exports=jdk.jlink/jdk.tools.jlink.internal.runtimelink=ALL-UNNAMED \ > 121: --add-exports=java.base/jdk.internal.module=ALL-UNNAMED \ > 122: --add-exports=java.base/jdk.internal.jimage=ALL-UNNAMED \ These three are repeated in both compilation and runtime so could potentially be set in a variable to avoid the duplication. make/Images.gmk line 208: > 206: WARN
Re: RFR: 8311302: Allow for jlinking a custom runtime without packaged modules being present [v24]
> Please review this patch which adds a jlink mode to the JDK which doesn't > need the packaged modules being present. A.k.a run-time image based jlink. > 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 b eing 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 `JRTArchive` > 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 jdk.jlink) > <(../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.jmodjava.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.jmodjava.prefs.jmod java.transaction.xa.jmod > jdk.dynalink.jmod jdk.internal.vm.ci.jmod > jdk.jdwp.agent.jmod jdk.management.jfr.jmodjdk.security.jgss.jmod > java.datatransfer.jmodjava.rmi.jmodjava.xml.crypto.jmod > jdk.editpad.jmod jdk.internal.vm.compiler.jmod > jdk.jfr.jmod jdk.management.jmodjdk.unsupported.desktop.jmod > java.desktop.jmod java.scripting.jmod java.xml.jmod > jdk.hotspot.agent.jmod jdk.internal.vm.compiler.manage... Severin Gehwolf has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 100 commits: - Fix coment - Fix comment - Fix typo - Revert some now unneded build changes - Follow build tools naming convention - Update to new build-time approach with delta in lib - Make generation of fs_$module_files unconditional - Merge branch 'master' into jdk-8311302-jmodless-link - Fix copyright year - Move CreateLinkableRuntimePlugin to build folder Keep runtime link supporting classes in package jdk.tools.jlink.internal.runtimelink - ... and 90 more: https://git.openjdk.org/jdk/compare/6ae1cf12...ce04f42a - Changes: https://git.openjdk.org/jdk/pull/14787/files Webrev: https://webrevs.openjdk.org/?repo=jdk=14787=23 Stats: 3384 lines in 36 files changed: 3247 ins; 106 del; 31 mod Patch: https://git.openjdk.org/jdk/pull/14787.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/14787/head:pull/14787 PR: https://git.openjdk.org/jdk/pull/14787
