Re: RFR: 8311302: Allow for jlinking a custom runtime without packaged modules being present [v29]

2024-06-05 Thread Severin Gehwolf
On Tue, 4 Jun 2024 09:04:30 GMT, Magnus Ihse Bursie  wrote:

>>> Does that proposal sound good?
>> 
>> That table is useful, I think it's right. No change to default behavior. If 
>> someone configures with --enable-runtime-image then they get a JDK run-time 
>> image that supports jlink (with some limitations) but is significantly small 
>> as it doesn't include the packaged modules.
>> 
>> I've read through most of the changes now. Overall I think it's looking 
>> good, just a few terminology and minor points that I'll add as comments.
>
>> Does that proposal sound good?
> 
> What you basically is saying is that the default value of `packaged-modules` 
> should be `! runtime-link-image` (i.e. the inverse)?

@magicus @erikj79 Do you mind re-reviewing the build changes?

-

PR Comment: https://git.openjdk.org/jdk/pull/14787#issuecomment-2150590534


Re: RFR: 8311302: Allow for jlinking a custom runtime without packaged modules being present [v29]

2024-06-05 Thread Alan Bateman
On Tue, 4 Jun 2024 14:39:23 GMT, Severin Gehwolf  wrote:

> I've added a couple of `@requires jlink.packagedModules` (new with this 
> patch) so that jlink tests don't start to fail with it. 

Good, the `@requires jlink.packagedModules` make sense.

-

PR Comment: https://git.openjdk.org/jdk/pull/14787#issuecomment-214941


Re: RFR: 8311302: Allow for jlinking a custom runtime without packaged modules being present [v29]

2024-06-04 Thread Severin Gehwolf
On Mon, 3 Jun 2024 19:10:22 GMT, Alan Bateman  wrote:

> > Does that proposal sound good?
> 
> That table is useful, I think it's right. No change to default behavior. If 
> someone configures with --enable-runtime-image then they get a JDK run-time 
> image that supports jlink (with some limitations) but is significantly small 
> as it doesn't include the packaged modules.

https://github.com/openjdk/jdk/pull/14787/commits/5fef297ba63d60452f098193d2cba33a941b
 implements this.

-

PR Comment: https://git.openjdk.org/jdk/pull/14787#issuecomment-2147609799


Re: RFR: 8311302: Allow for jlinking a custom runtime without packaged modules being present [v29]

2024-06-04 Thread Severin Gehwolf
On Sun, 2 Jun 2024 17:41:55 GMT, Alan Bateman  wrote:

> (Doing that would of course mean that several existing jlink tests would need 
> some changes, either to `@requires` or to remove the checks for the `jmods` 
> directory)

I've added a couple of `@requires jlink.packagedModules` (new with this patch) 
so that jlink tests don't start to fail with it. As a follow-up I've filed 
https://bugs.openjdk.org/browse/JDK-8333541. This change is becoming large 
enough as it is and would like to get this in before RDP1 on Thursday. The 
initial work was submitted almost a year ago.

-

PR Comment: https://git.openjdk.org/jdk/pull/14787#issuecomment-2147709606


Re: RFR: 8311302: Allow for jlinking a custom runtime without packaged modules being present [v29]

2024-06-04 Thread Severin Gehwolf
On Mon, 3 Jun 2024 19:10:22 GMT, Alan Bateman  wrote:

> I've read through most of the changes now. Overall I think it's looking good, 
> just a few terminology and minor points that I'll add as comments.

@AlanBateman I don't see those comments. Should I?

-

PR Comment: https://git.openjdk.org/jdk/pull/14787#issuecomment-2147116736


Re: RFR: 8311302: Allow for jlinking a custom runtime without packaged modules being present [v29]

2024-06-04 Thread Severin Gehwolf
On Tue, 4 Jun 2024 09:04:30 GMT, Magnus Ihse Bursie  wrote:

> > Does that proposal sound good?
> 
> What you basically is saying is that the default value of `packaged-modules` 
> should be `! runtime-link-image` (i.e. the inverse)?

Yes. **default** of the `packaged-modules` value being the key.

-

PR Comment: https://git.openjdk.org/jdk/pull/14787#issuecomment-2147004354


Re: RFR: 8311302: Allow for jlinking a custom runtime without packaged modules being present [v29]

2024-06-04 Thread Magnus Ihse Bursie
On Mon, 3 Jun 2024 19:10:22 GMT, Alan Bateman  wrote:

> Does that proposal sound good?

What you basically is saying is that the default value of `packaged-modules` 
should be `! runtime-link-image` (i.e. the inverse)?

-

PR Comment: https://git.openjdk.org/jdk/pull/14787#issuecomment-2146996167


Re: RFR: 8311302: Allow for jlinking a custom runtime without packaged modules being present [v29]

2024-06-03 Thread Alan Bateman
On Mon, 3 Jun 2024 15:40:10 GMT, Severin Gehwolf  wrote:

> Does that proposal sound good?

That table is useful, I think it's right. No change to default behavior. If 
someone configures with --enable-runtime-image then they get a JDK run-time 
image that supports jlink (with some limitations) but is significantly small as 
it doesn't include the packaged modules.

I've read through most of the changes now. Overall I think it's looking good, 
just a few terminology and minor points that I'll add as comments.

-

PR Comment: https://git.openjdk.org/jdk/pull/14787#issuecomment-2145932080


Re: RFR: 8311302: Allow for jlinking a custom runtime without packaged modules being present [v29]

2024-06-03 Thread Severin Gehwolf
On Mon, 3 Jun 2024 12:55:54 GMT, Alan Bateman  wrote:

> So I think we may have the wrong default. Yes, they are separate configure 
> and jlink options but I'm wondering if it would be more sensible to 
> opt-in(not opt-out) to keep the packaged modules when configured with 
> --enable-runtime-link-image.

OK. I'll rework it so that we'll have:

| config opts | effect | equivalent to |
| -|--|-|
| `--enable-runtime-link-image` | produces a linkable runtime **without** 
packaged modules even though the default of  `--enable-packaged-modules` is 
otherwise `true`.| `--enable-runtime-link-image --disable-packaged-modules`|
| `--enable-runtime-link-image --enable-packaged-modules`| produces a linkable 
runtime **with** packaged modules, overriding the default of packaged modules 
not being enabled when `--enable-runtime-link-image`  is being used otherwise | 
N/A|
| `--disable-runtime-link-image` | Default as of today. Adds packaged modules, 
with no run time link supporting jimage | `--disable-runtime-link-image 
--enable-packaged-modules`|
| `--disable-runtime-link-image --disable-packaged-modules` | No linkable 
jimage runtime, no packaged modules in the resulting JDK | N/A |

Does that proposal sound good?

-

PR Comment: https://git.openjdk.org/jdk/pull/14787#issuecomment-2145540168


Re: RFR: 8311302: Allow for jlinking a custom runtime without packaged modules being present [v29]

2024-06-03 Thread Alan Bateman
On Sun, 2 Jun 2024 17:41:55 GMT, Alan Bateman  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 110 commits:
>> 
>>  - Merge branch 'master' into jdk-8311302-jmodless-link
>>  - Fix new line in jlink.properties
>>  - Merge branch 'master' into jdk-8311302-jmodless-link
>>  - Simplify JLINK_JDK_EXTRA_OPTS var handling
>>  - Only add runtime track files for linkable runtimes
>>  - Generate the runtime image link diff at jlink time
>>
>>But only do that once the plugin-pipeline has run. The generation is
>>enabled with the hidden option '--generate-linkable-runtime' and
>>the resource pools available at jlink time are being used to generate
>>the diff.
>>  - Merge branch 'master' into jdk-8311302-jmodless-link
>>  - Use shorter name for the build tool
>>  - Review feedback from Erik J.
>>  - Remove dependency on CDS which isn't needed anymore
>>  - ... and 100 more: https://git.openjdk.org/jdk/compare/4f1a10f8...e1e3f02f
>
> I've been looking through the changes. One thing that I'm wondering about is 
> whether --generate-runtime-link-image should disable the keeping of packaged 
> modules (set JLINK_KEEP_PACKAGED_MODULES to false). It seems surprising to 
> use this configure option and have the jlink command in the build also copy 
> the packaged modules into the image.
> 
> (Doing that would of course mean that several existing jlink tests would need 
> some changes, either to `@requires` or to remove the checks for the `jmods` 
> directory)

> @AlanBateman IMO those are orthogonal concepts. 

The purpose of --enable-runtime-link-image is to produce a run-time image that 
is capable of running jlink without the packaged modules. The benefit is is a 
reducing size on the file system but you don't get that benefit if the packages 
modules are copied in the image.

So I think we may have the wrong default. Yes, they are separate configure and 
jlink options but I'm wondering if it would be more sensible to opt-in(not 
opt-out)  to keep the packaged modules when configured with 
--enable-runtime-link-image.

-

PR Comment: https://git.openjdk.org/jdk/pull/14787#issuecomment-2145136017


Re: RFR: 8311302: Allow for jlinking a custom runtime without packaged modules being present [v29]

2024-06-03 Thread Severin Gehwolf
On Sun, 2 Jun 2024 17:41:55 GMT, Alan Bateman  wrote:

> I've been looking through the changes. One thing that I'm wondering about is 
> whether --generate-runtime-link-image should disable the keeping of packaged 
> modules (set JLINK_KEEP_PACKAGED_MODULES to false). It seems surprising to 
> use this configure option and have the jlink command in the build also copy 
> the packaged modules into the image.

@AlanBateman IMO those are orthogonal concepts. Note that the configure options 
are `--enable-packaged-modules` and `--enable-runtime-link-image`. The 
corresponding `jlink` options are `--keep-packaged-modules` and 
`--generate-linkable-runtime`. My mental model is that with this patch it 
allows a more flexible distribution of the JDK build. The testing story also 
seems easier in its current form. All non-linkable runtime tests run as-is - 
with a linkable runtime build - but also run linkable runtime tests (those have 
appropriate `@requires` tags). We've had some discussion around this already in 
this review thread. I'm arguing that both configure options make sense 
independently and in combination. The user can configure it to their liking. 
What I'd try to avoid is needing to produce two different builds whether or not 
the JDK is runtime linkable or not. That is because our prime use-case is to 
make `jmods` optional when `jlink` is being used post build and test.

I've tried to explain it earlier 
[here](https://github.com/openjdk/jdk/pull/14787#issuecomment-1999307995) and 
[here](https://github.com/openjdk/jdk/pull/14787#issuecomment-2003848668). 
@mlchung seemed OK with it 
[here](https://github.com/openjdk/jdk/pull/14787#issuecomment-2004605507) and 
@erikj79 was ok with it as well 
[here](https://github.com/openjdk/jdk/pull/14787#issuecomment-2004761747).

Is there a specific reason this needs to be done? With the current patch 
`--enable-runtime-link-image` influences how the `jimage` in `lib/modules` 
looks like (adds some metadata) nothing else. `--enable-packaged-modules` 
influences copying of the packaged modules.

Right now, what you are suggesting could be achieved with these configure 
options:
`--enable-runtime-link-image --disable-packaged-modules`

Is that not sufficient?

Thoughts?

-

PR Comment: https://git.openjdk.org/jdk/pull/14787#issuecomment-2144808992


Re: RFR: 8311302: Allow for jlinking a custom runtime without packaged modules being present [v29]

2024-06-02 Thread Alan Bateman
On Wed, 22 May 2024 13:23:25 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 110 commits:
> 
>  - Merge branch 'master' into jdk-8311302-jmodless-link
>  - Fix new line in jlink.properties
>  - Merge branch 'master' into jdk-8311302-jmodless-link
>  - Simplify JLINK_JDK_EXTRA_OPTS var handling
>  - Only add runtime track files for linkable runtimes
>  - Generate the runtime image link diff at jlink time
>
>But only do that once the plugin-pipeline has run. The generation is
>enabled with the hidden option '--generate-linkable-runtime' and
>the resource pools available at jlink time are being used to generate
>the diff.
>  - Merge branch 'master' into jdk-8311302-jmodless-link
>  - Use shorter name for the build tool
>  - Review feedback from Erik J.
>  - Remove dependency on CDS which isn't needed anymore
>  - ... and 100 more: https://git.openjdk.org/jdk/compare/4f1a10f8...e1e3f02f

I've been looking through the changes. One thing that I'm wondering about is 
whether --generate-runtime-link-image should disable the keeping of packaged 
modules (set JLINK_KEEP_PACKAGED_MODULES to false). It seems surprising to use 
this configure option and have the jlink command in the build also copy the 
packaged modules into the image.

(Doing that would of course mean that several existing jlink tests would need 
some changes, either to `@requires` or to remove the checks for the `jmods` 
directory)

-

PR Comment: https://git.openjdk.org/jdk/pull/14787#issuecomment-2143963280


Re: RFR: 8311302: Allow for jlinking a custom runtime without packaged modules being present [v29]

2024-05-22 Thread Severin Gehwolf
> 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 110 commits:

 - Merge branch 'master' into jdk-8311302-jmodless-link
 - Fix new line in jlink.properties
 - Merge branch 'master' into jdk-8311302-jmodless-link
 - Simplify JLINK_JDK_EXTRA_OPTS var handling
 - Only add runtime track files for linkable runtimes
 - Generate the runtime image link diff at jlink time
   
   But only do that once the plugin-pipeline has run. The generation is
   enabled with the hidden option '--generate-linkable-runtime' and
   the resource pools available at jlink time are being used to generate
   the diff.
 - Merge branch 'master' into jdk-8311302-jmodless-link
 - Use shorter name for the build tool
 - Review feedback from Erik J.
 - Remove dependency on CDS which isn't needed anymore
 - ... and 100 more: https://git.openjdk.org/jdk/compare/4f1a10f8...e1e3f02f

-

Changes: https://git.openjdk.org/jdk/pull/14787/files
  Webrev: https://webrevs.openjdk.org/?repo=jdk=14787=28
  Stats: 3424 lines in 36 files changed: 3272 ins; 110 del; 42 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