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

[Severin Gehwolf]

On Tue, 7 May 2024 16:52:12 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 105 commits:
> 
>  - 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
>  - Fix coment
>  - Fix comment
>  - Fix typo
>  - Revert some now unneded build changes
>  - Follow build tools naming convention
>  - ... and 95 more: https://git.openjdk.org/jdk/compare/23a72a1f...67aea4ca

GHA test failures 
[gc/shenandoah/oom/TestThreadFailure](https://github.com/jerboaa/jdk/actions/runs/8989206765#user-content-gc_shenandoah_oom_testthreadfailure)
 on Mac x86_64 and 
[java/util/zip/ZipFile/ZipSourceCache](https://github.com/jerboaa/jdk/actions/runs/8989206765#user-content-java_util_zip_zipfile_zipsourcecache)
 on Windows x86_64 seem unrelated to this patch.

-

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

Re: RFR: 8331886: Allow markdown src file overrides

[Magnus Ihse Bursie]

On Wed, 8 May 2024 01:02:41 GMT, Erik Joelsson  wrote:

> For c, c++  and java source files, we have a built in system for letting more 
> specific files override if there are multiple files with the same name found, 
> by letting the first found file with a given name override any later found 
> files with that name. This is typically used for OS specific variants or when 
> needing to override a source file with a file from a custom source set. We 
> would like to make it possible to do the same for markdown files used to 
> generate man pages.
> 
> This will not have an immediate use i OpenJDK, but is needed for a custom 
> override in proprietary code.
> 
> The change in Docs.gmk removes unnecessary extra loops so that 
> SetupProcessMarkdown is called only once per module. This is necessary for 
> the override mechanism to kick in for each module src set.
> 
> The logic in ProcessMarkdown.gmk is more or less copied from 
> SetupNativeCompilation.

make/common/ProcessMarkdown.gmk line 43:

> 41:   # Only continue if this target file hasn't been processed already. This 
> lets
> 42:   # the first found source file override any other with the same name.
> 43:   $$(call PrintVar, $1_$2_INPUT_FILE)

Left-over debug code.

-

PR Review Comment: https://git.openjdk.org/jdk/pull/19132#discussion_r1593782745

Re: RFR: 8331886: Allow markdown src file overrides

[Magnus Ihse Bursie]

On Wed, 8 May 2024 01:02:41 GMT, Erik Joelsson  wrote:

> For c, c++  and java source files, we have a built in system for letting more 
> specific files override if there are multiple files with the same name found, 
> by letting the first found file with a given name override any later found 
> files with that name. This is typically used for OS specific variants or when 
> needing to override a source file with a file from a custom source set. We 
> would like to make it possible to do the same for markdown files used to 
> generate man pages.
> 
> This will not have an immediate use i OpenJDK, but is needed for a custom 
> override in proprietary code.
> 
> The change in Docs.gmk removes unnecessary extra loops so that 
> SetupProcessMarkdown is called only once per module. This is necessary for 
> the override mechanism to kick in for each module src set.
> 
> The logic in ProcessMarkdown.gmk is more or less copied from 
> SetupNativeCompilation.

Looks good (apart from the PrintVar)

This was particularly hard to review, since neither Github nor Webrev could 
make any sense of this. For other reviewers, I can add that the only 
substantial change in ProcessMarkdown.gmk is wrapping the entire function body 
in a `ifeq ($$($1_$2_OUTPUT_FILE_PROCESSED), )` check (and updating this 
variable upon entry).

-

Marked as reviewed by ihse (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/19132#pullrequestreview-2045243623
PR Comment: https://git.openjdk.org/jdk/pull/19132#issuecomment-2100237486

Re: RFR: 8331541: [i386] linking with libjvm.so fails after JDK-8283326

[Magnus Ihse Bursie]

On Thu, 2 May 2024 08:23:38 GMT, Vladimir Petko  wrote:

> Hi, 
> 
> The `.type _SafeFetch32_impl,@function` symbol declaration contains a 
> spurious underscore causing an undefined symbol in libjvm.so.
> 
> I am not sure about change in default flags. I have removed the flag that 
> allows linking with undefined symbols
> because having the flag on might cause bugs like this one or 
> https://bugs.openjdk.org/browse/JDK-8329983 as the build succeeds even if 
> some symbols are not defined.
> Openjdk builds successfully without it on amd64, i386, armhf, arm64, s390, 
> ppc64el and riscv64 with this change[1]. riscv64 build was done locally 
> inside vm:
> 
> Finished building target 'images' in configuration 
> 'linux-riscv64-server-release'
> ubuntu@ubuntu:~/jdk$ 
> 
> Unfortunately I do not know why it was introduced, so I might be missing 
> something.
> I am happy to drop it from this one or move it to a separate issue/pr.
>  
> [1] 
> https://launchpad.net/~vpa1977/+archive/ubuntu/october-21/+sourcepub/16076564/+listing-archive-extra

Ok, I think I'm understanding this better now. The error occurs since the 
`.type`, `.globl` and `.hidden` directives do not match -- we define the global 
symbol to be `SafeFetch32_impl` but then we set the type of the non-existing 
symbol `_SafeFetch32_impl`. Somehow this tricks the linker into accepting this 
as an undefined symbol.

The `.type` directive is not without purpose -- it sets the type of the symbol 
to be a function. If omitted, the type will be `NOTYPE`. Apparently this does 
not break the program but there is no reason to remove the `.type` directives.

Instead, we should have a common macro, something like this:

#define DECLARE_FUNC(func) \
.globl func ; \
.hidden func ; \
.type func,@function ; \
func:



in a shared file, and include and use it for all symbols in our hotspot 
assembly files. I was thinking somewhat along those lines last time I was 
poking around there (when introducing .hidden for the removal of the hotspot 
map files), but never really got around to it. This bug really shows why we 
should do that.

-

PR Comment: https://git.openjdk.org/jdk/pull/19048#issuecomment-2100322142

Re: RFR: 8331541: [i386] linking with libjvm.so fails after JDK-8283326

[Magnus Ihse Bursie]

On Thu, 2 May 2024 08:23:38 GMT, Vladimir Petko  wrote:

> Hi, 
> 
> The `.type _SafeFetch32_impl,@function` symbol declaration contains a 
> spurious underscore causing an undefined symbol in libjvm.so.
> 
> I am not sure about change in default flags. I have removed the flag that 
> allows linking with undefined symbols
> because having the flag on might cause bugs like this one or 
> https://bugs.openjdk.org/browse/JDK-8329983 as the build succeeds even if 
> some symbols are not defined.
> Openjdk builds successfully without it on amd64, i386, armhf, arm64, s390, 
> ppc64el and riscv64 with this change[1]. riscv64 build was done locally 
> inside vm:
> 
> Finished building target 'images' in configuration 
> 'linux-riscv64-server-release'
> ubuntu@ubuntu:~/jdk$ 
> 
> Unfortunately I do not know why it was introduced, so I might be missing 
> something.
> I am happy to drop it from this one or move it to a separate issue/pr.
>  
> [1] 
> https://launchpad.net/~vpa1977/+archive/ubuntu/october-21/+sourcepub/16076564/+listing-archive-extra

So, after sleeping on this, here is my analysis/verdict:
1) The fix on renaming the `.type` directive looks good. This is the actual bug 
fix.

2) We can remove `--allow-shlib-undefined`. That just overrides 
`--no-allow-shlib-
undefined`, which has been the default for years and years, and there is no 
good reason to have it. It is not strictly related to the bug fix per se, but 
we can do that in this PR as well to keep things simpler.

3) The reason this could happen was since we used assembler code. A similar 
problem in C++ code would not have been allowed by the compiler.

4) If we introduce a common macro as I suggest, we can avoid this ever 
happening again. So while the linker cannot guarantee the consistency when 
linking libjvm.so (which I really would have liked), by using a stricter scheme 
when defining symbols in assembly code, we can make sure to avoid it.

-

Marked as reviewed by ihse (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/19048#pullrequestreview-2045354117

Re: RFR: 8331541: [i386] linking with libjvm.so fails after JDK-8283326

[Magnus Ihse Bursie]

On Thu, 2 May 2024 08:23:38 GMT, Vladimir Petko  wrote:

> Hi, 
> 
> The `.type _SafeFetch32_impl,@function` symbol declaration contains a 
> spurious underscore causing an undefined symbol in libjvm.so.
> 
> I am not sure about change in default flags. I have removed the flag that 
> allows linking with undefined symbols
> because having the flag on might cause bugs like this one or 
> https://bugs.openjdk.org/browse/JDK-8329983 as the build succeeds even if 
> some symbols are not defined.
> Openjdk builds successfully without it on amd64, i386, armhf, arm64, s390, 
> ppc64el and riscv64 with this change[1]. riscv64 build was done locally 
> inside vm:
> 
> Finished building target 'images' in configuration 
> 'linux-riscv64-server-release'
> ubuntu@ubuntu:~/jdk$ 
> 
> Unfortunately I do not know why it was introduced, so I might be missing 
> something.
> I am happy to drop it from this one or move it to a separate issue/pr.
>  
> [1] 
> https://launchpad.net/~vpa1977/+archive/ubuntu/october-21/+sourcepub/16076564/+listing-archive-extra

I created https://bugs.openjdk.org/browse/JDK-8331921 to track setting up 
proper assembly functions consistently.

-

PR Comment: https://git.openjdk.org/jdk/pull/19048#issuecomment-2100375665

RFR: 8331939: Add custom hook for TestImage

[Erik Joelsson]

We need a custom hook in TestImage.gmk to modify behavior when building with 
custom source.

-

Commit messages:
 - JDK-8331939

Changes: https://git.openjdk.org/jdk/pull/19139/files
  Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=19139&range=00
  Issue: https://bugs.openjdk.org/browse/JDK-8331939
  Stats: 3 lines in 1 file changed: 3 ins; 0 del; 0 mod
  Patch: https://git.openjdk.org/jdk/pull/19139.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/19139/head:pull/19139

PR: https://git.openjdk.org/jdk/pull/19139

Re: RFR: 8331886: Allow markdown src file overrides [v2]

[Erik Joelsson]

> For c, c++  and java source files, we have a built in system for letting more 
> specific files override if there are multiple files with the same name found, 
> by letting the first found file with a given name override any later found 
> files with that name. This is typically used for OS specific variants or when 
> needing to override a source file with a file from a custom source set. We 
> would like to make it possible to do the same for markdown files used to 
> generate man pages.
> 
> This will not have an immediate use i OpenJDK, but is needed for a custom 
> override in proprietary code.
> 
> The change in Docs.gmk removes unnecessary extra loops so that 
> SetupProcessMarkdown is called only once per module. This is necessary for 
> the override mechanism to kick in for each module src set.
> 
> The logic in ProcessMarkdown.gmk is more or less copied from 
> SetupNativeCompilation.

Erik Joelsson has updated the pull request with a new target base due to a 
merge or a rebase. The incremental webrev excludes the unrelated changes 
brought in by the merge/rebase. The pull request contains three additional 
commits since the last revision:

 - Remove debug printing
 - Merge branch 'master' into JDK-8331886-override-markdown
 - JDK-8331886

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/19132/files
  - new: https://git.openjdk.org/jdk/pull/19132/files/0bc0a668..17fee013

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk&pr=19132&range=01
 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=19132&range=00-01

  Stats: 15 lines in 2 files changed: 11 ins; 2 del; 2 mod
  Patch: https://git.openjdk.org/jdk/pull/19132.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/19132/head:pull/19132

PR: https://git.openjdk.org/jdk/pull/19132

RFR: 8331921: Hotspot assembler files should use common logic to setup exported functions

[Magnus Ihse Bursie]

As seen in [JDK-8331541](https://bugs.openjdk.org/browse/JDK-8331541), if we 
are not consistently setting all assembler directives correctly, we can get 
errors that are not detected by the linker.

We should stop duplicating this code and create a unified macro to properly 
setup functions, and use it everywhere.

-

Commit messages:
 - Fix copyright headers
 - Fix building on macos/aarch64
 - Use % instead of @ due to arm assembler
 - 8331921: Hotspot assembler files should use common logic to setup exported 
functions

Changes: https://git.openjdk.org/jdk/pull/19138/files
  Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=19138&range=00
  Issue: https://bugs.openjdk.org/browse/JDK-8331921
  Stats: 604 lines in 22 files changed: 124 ins; 347 del; 133 mod
  Patch: https://git.openjdk.org/jdk/pull/19138.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/19138/head:pull/19138

PR: https://git.openjdk.org/jdk/pull/19138

Re: RFR: 8331921: Hotspot assembler files should use common logic to setup exported functions

[Magnus Ihse Bursie]

On Wed, 8 May 2024 13:30:59 GMT, Magnus Ihse Bursie  wrote:

> As seen in [JDK-8331541](https://bugs.openjdk.org/browse/JDK-8331541), if we 
> are not consistently setting all assembler directives correctly, we can get 
> errors that are not detected by the linker.
> 
> We should stop duplicating this code and create a unified macro to properly 
> setup functions, and use it everywhere.

When making a build comparison, the build result was identical on Windows (of 
course), and macOS. On linux/x64 and linux/aarch64, there was a slight 
difference, in that a few functions did not properly set `.type`, so they 
appeared as `NOTYPE` instead of `FUNC` in the symbol table.

I have not done comparison builds for other linux platforms, but I assume that 
GHA passing is good enough.

-

PR Comment: https://git.openjdk.org/jdk/pull/19138#issuecomment-2100776636

Re: RFR: 8331886: Allow markdown src file overrides [v2]

[Magnus Ihse Bursie]

On Wed, 8 May 2024 14:29:18 GMT, Erik Joelsson  wrote:

>> For c, c++  and java source files, we have a built in system for letting 
>> more specific files override if there are multiple files with the same name 
>> found, by letting the first found file with a given name override any later 
>> found files with that name. This is typically used for OS specific variants 
>> or when needing to override a source file with a file from a custom source 
>> set. We would like to make it possible to do the same for markdown files 
>> used to generate man pages.
>> 
>> This will not have an immediate use i OpenJDK, but is needed for a custom 
>> override in proprietary code.
>> 
>> The change in Docs.gmk removes unnecessary extra loops so that 
>> SetupProcessMarkdown is called only once per module. This is necessary for 
>> the override mechanism to kick in for each module src set.
>> 
>> The logic in ProcessMarkdown.gmk is more or less copied from 
>> SetupNativeCompilation.
>
> Erik Joelsson has updated the pull request with a new target base due to a 
> merge or a rebase. The incremental webrev excludes the unrelated changes 
> brought in by the merge/rebase. The pull request contains three additional 
> commits since the last revision:
> 
>  - Remove debug printing
>  - Merge branch 'master' into JDK-8331886-override-markdown
>  - JDK-8331886

Marked as reviewed by ihse (Reviewer).

-

PR Review: https://git.openjdk.org/jdk/pull/19132#pullrequestreview-2045917930

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

[Magnus Ihse Bursie]

On Tue, 7 May 2024 16:52:12 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 105 commits:
> 
>  - 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
>  - Fix coment
>  - Fix comment
>  - Fix typo
>  - Revert some now unneded build changes
>  - Follow build tools naming convention
>  - ... and 95 more: https://git.openjdk.org/jdk/compare/23a72a1f...67aea4ca

The new build changes look extremely trivial. From a pure build PoV, this is a 
much simpler solution.

-

Marked as reviewed by ihse (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/14787#pullrequestreview-2045924409

RFR: 8331942: On Linux aarch64, CDS archives should be using 64K alignment by default

[Thomas Stuefe]

On Linux aarch64, a JVM may encounter three different page sizes: 4K, 64K, and 
(when run on Mac M1 hardware) 16K.

Since forgetting to specify `--enable-compatible-cds-alignment` is a common 
error that is only noticed when running the produced JVM on hardware with 
different page size, we propose to enable that option by default on Linux 
aarch64. The cost is a moderate increase in CDS archive size (about 300K).

I tested this patch on:
- x64 Linux
- x64 Linux, crossbuilding to aarch64
- building natively on aarch64 Linux

-

Commit messages:
 - aarch64-should-default-to-64k-alignment-for-cds-regions

Changes: https://git.openjdk.org/jdk/pull/19142/files
  Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=19142&range=00
  Issue: https://bugs.openjdk.org/browse/JDK-8331942
  Stats: 12 lines in 1 file changed: 11 ins; 0 del; 1 mod
  Patch: https://git.openjdk.org/jdk/pull/19142.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/19142/head:pull/19142

PR: https://git.openjdk.org/jdk/pull/19142

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

[Severin Gehwolf]

On Thu, 4 Apr 2024 20:56:55 GMT, Magnus Ihse Bursie  wrote:

>> Severin Gehwolf has updated the pull request incrementally with three 
>> additional commits since the last revision:
>> 
>>  - Use shorter name for the build tool
>>  - Review feedback from Erik J.
>>  - Remove dependency on CDS which isn't needed anymore
>
> Overall, I think the build system changes in the current revision of the 
> patch look good, but please let me know for a deeper review when things have 
> stabilized.

Thanks for the review, @magicus!

-

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

Re: RFR: 8331942: On Linux aarch64, CDS archives should be using 64K alignment by default

[Andrew Haley]

On Wed, 8 May 2024 15:14:16 GMT, Thomas Stuefe  wrote:

> On Linux aarch64, a JVM may encounter three different page sizes: 4K, 64K, 
> and (when run on Mac M1 hardware) 16K.
> 
> Since forgetting to specify `--enable-compatible-cds-alignment` is a common 
> error that is only noticed when running the produced JVM on hardware with 
> different page size, we propose to enable that option by default on Linux 
> aarch64. The cost is a moderate increase in CDS archive size (about 300K).
> 
> I tested this patch on:
> - x64 Linux
> - x64 Linux, crossbuilding to aarch64
> - building natively on aarch64 Linux

This is obviously correct. Otherwise, the CDS archive will only work if the JDK 
happens to have been built on a machine with greater or equal page size.

Does the default `compatible-cds-alignment=false` make any sense anywhere on 
other platforms, though? We might simply make compatibility the default.

-

Marked as reviewed by aph (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/19142#pullrequestreview-204614

Re: RFR: 8312425: [vectorapi] AArch64: Optimize vector math operations with SLEEF [v5]

[Hamlin Li]

> Hi,
> Can you help to review the patch?
> This pr is based on previous work and discussion in [pr 
> 16234](https://github.com/openjdk/jdk/pull/16234), [pr 
> 18294](https://github.com/openjdk/jdk/pull/18294).
> 
> Compared with previous prs, the major change in this pr is to integrate the 
> source of sleef (for the steps, please check 
> `src/jdk.incubator.vector/linux/native/libvectormath/README`), rather than 
> depends on external sleef things (header or lib) at build or run time.
> Besides of this change, also modify the previous changes accordingly, e.g. 
> remove some uncessary files or changes especially in make dir of jdk.
> 
> Besides of the code changes, one important task is to handle the legal 
> process.
> 
> Thanks!
> 
> ## Performance
> NOTE: 
> * `Src` means implementation in this pr, i.e. without depenency on external 
> sleef.
> * `Disabled` means disable intrinsics by `-XX:-UseVectorStubs` 
> * `system_sleef` means implementation in [previous pr 
> 18294](https://github.com/openjdk/jdk/pull/18294), i.e. build and run jdk 
> with depenency on external sleef.
> 
> Basically, the perf data below shows that 
> * this implementation has better performance than previous version in [pr 
> 18294](https://github.com/openjdk/jdk/pull/18294), 
> * and both sleef versions has much better performance compared with non-sleef 
> version.
> 
> |Benchmark |(size)|Src  
> |Units|system_sleef|(system_sleef-Src)/Src|Diabled  |(Disable-Src)/Src|
> |--|--|-|-||--|-|-|
> |3472:Double128Vector.ACOS |1024  |8546.842 |ns/op|8516.007|-0.004
> |16799.273|0.966|
> |3473:Double128Vector.ASIN |1024  |6864.656 |ns/op|6987.328|0.018 
> |16602.442|1.419|
> |3474:Double128Vector.ATAN |1024  |11489.255|ns/op|12261.800   |0.067 
> |26329.320|1.292|
> |3475:Double128Vector.ATAN2|1024  |16661.170|ns/op|17234.472   |0.034 
> |42084.100|1.526|
> |3476:Double128Vector.CBRT |1024  |18999.387|ns/op|20298.458   |0.068 
> |35998.688|0.895|
> |3477:Double128Vector.COS  |1024  |14081.857|ns/op|14846.117   |0.054 
> |24420.692|0.734|
> |3478:Double128Vector.COSH |1024  |12202.306|ns/op|12237.772   |0.003 
> |21343.863|0.749|
> |3479:Double128Vector.EXP  |1024  |4553.108 |ns/op|4777.638|0.049 
> |20155.903|3.427|
> |3480:D...

Hamlin Li has updated the pull request incrementally with one additional commit 
since the last revision:

  add inline header file for riscv64

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/18605/files
  - new: https://git.openjdk.org/jdk/pull/18605/files/cbcd4634..bd9c0931

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk&pr=18605&range=04
 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=18605&range=03-04

  Stats: 7073 lines in 1 file changed: 7073 ins; 0 del; 0 mod
  Patch: https://git.openjdk.org/jdk/pull/18605.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/18605/head:pull/18605

PR: https://git.openjdk.org/jdk/pull/18605

Re: RFR: 8312425: [vectorapi] AArch64: Optimize vector math operations with SLEEF [v2]

[Hamlin Li]

On Tue, 9 Apr 2024 20:10:36 GMT, Mikael Vidstedt  wrote:

>> Hamlin Li has updated the pull request incrementally with two additional 
>> commits since the last revision:
>> 
>>  - disable unused-function warnings; add log msg
>>  - minor
>
> Thank you for the update and for working on this in general.
> 
> I've started working on JDK-8329816, preparing the change for the SLEEF 
> specific part of the change. Specifically, I'm currently planning on 
> including the three SLEEF header files, the README and a legal/sleef.md file 
> in that change. Let me know if you have any thoughts/concerns.
> 
> Also, just for my understanding, would love to understand your thoughts on 
> the future here (I apologize if this was already discussed elsewhere):
> 
> It seem like SLEEF is (sort of) limited to linux at this point (the SLEEF 
> README mentions that "Due to limited test capacities, SLEEF is currently only 
> officially supported on Linux with gcc or llvm/clang." ). That same README 
> does, however, indicate good test coverage on several architectures in 
> addition to aarch64 (including x86_64, PPC, RISC-V). With that in mind, it 
> looks like we could potentially use SLEEF for other architectures on linux in 
> the future? And potentially additional operating systems as well?

Hey @vidmik , I just added inline header file for riscv64, hope to help avoid 
go through the legal process for arm and riscv header files separately.

For implementation on riscv64, I will put it in another pr.

-

PR Comment: https://git.openjdk.org/jdk/pull/18605#issuecomment-2101055673

Re: RFR: 8312425: [vectorapi] AArch64: Optimize vector math operations with SLEEF [v6]

[Hamlin Li]

> Hi,
> Can you help to review the patch?
> This pr is based on previous work and discussion in [pr 
> 16234](https://github.com/openjdk/jdk/pull/16234), [pr 
> 18294](https://github.com/openjdk/jdk/pull/18294).
> 
> Compared with previous prs, the major change in this pr is to integrate the 
> source of sleef (for the steps, please check 
> `src/jdk.incubator.vector/linux/native/libvectormath/README`), rather than 
> depends on external sleef things (header or lib) at build or run time.
> Besides of this change, also modify the previous changes accordingly, e.g. 
> remove some uncessary files or changes especially in make dir of jdk.
> 
> Besides of the code changes, one important task is to handle the legal 
> process.
> 
> Thanks!
> 
> ## Performance
> NOTE: 
> * `Src` means implementation in this pr, i.e. without depenency on external 
> sleef.
> * `Disabled` means disable intrinsics by `-XX:-UseVectorStubs` 
> * `system_sleef` means implementation in [previous pr 
> 18294](https://github.com/openjdk/jdk/pull/18294), i.e. build and run jdk 
> with depenency on external sleef.
> 
> Basically, the perf data below shows that 
> * this implementation has better performance than previous version in [pr 
> 18294](https://github.com/openjdk/jdk/pull/18294), 
> * and both sleef versions has much better performance compared with non-sleef 
> version.
> 
> |Benchmark |(size)|Src  
> |Units|system_sleef|(system_sleef-Src)/Src|Diabled  |(Disable-Src)/Src|
> |--|--|-|-||--|-|-|
> |3472:Double128Vector.ACOS |1024  |8546.842 |ns/op|8516.007|-0.004
> |16799.273|0.966|
> |3473:Double128Vector.ASIN |1024  |6864.656 |ns/op|6987.328|0.018 
> |16602.442|1.419|
> |3474:Double128Vector.ATAN |1024  |11489.255|ns/op|12261.800   |0.067 
> |26329.320|1.292|
> |3475:Double128Vector.ATAN2|1024  |16661.170|ns/op|17234.472   |0.034 
> |42084.100|1.526|
> |3476:Double128Vector.CBRT |1024  |18999.387|ns/op|20298.458   |0.068 
> |35998.688|0.895|
> |3477:Double128Vector.COS  |1024  |14081.857|ns/op|14846.117   |0.054 
> |24420.692|0.734|
> |3478:Double128Vector.COSH |1024  |12202.306|ns/op|12237.772   |0.003 
> |21343.863|0.749|
> |3479:Double128Vector.EXP  |1024  |4553.108 |ns/op|4777.638|0.049 
> |20155.903|3.427|
> |3480:D...

Hamlin Li has updated the pull request incrementally with one additional commit 
since the last revision:

  update header files for arm

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/18605/files
  - new: https://git.openjdk.org/jdk/pull/18605/files/bd9c0931..36415c34

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk&pr=18605&range=05
 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=18605&range=04-05

  Stats: 20 lines in 2 files changed: 14 ins; 2 del; 4 mod
  Patch: https://git.openjdk.org/jdk/pull/18605.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/18605/head:pull/18605

PR: https://git.openjdk.org/jdk/pull/18605

Re: RFR: 8331939: Add custom hook for TestImage

[Mikael Vidstedt]

On Wed, 8 May 2024 14:03:48 GMT, Erik Joelsson  wrote:

> We need a custom hook in TestImage.gmk to modify behavior when building with 
> custom source.

Marked as reviewed by mikael (Reviewer).

-

PR Review: https://git.openjdk.org/jdk/pull/19139#pullrequestreview-2046326105

Re: RFR: 8331942: On Linux aarch64, CDS archives should be using 64K alignment by default

[Thomas Stuefe]

On Wed, 8 May 2024 17:01:26 GMT, Andrew Haley  wrote:

> This is obviously correct. Otherwise, the CDS archive will only work if the 
> JDK happens to have been built on a machine with greater or equal page size.
> 
> Does the default `compatible-cds-alignment=false` make any sense anywhere on 
> other platforms, though? We might simply make compatibility the default.

The configure option is also evaluated for MacOS x64 to enforce an alignment of 
16K, to be able to run on MacOS aarch64 in Rosetta. Seeing how MacOS x64 is not 
really important anymore, and losses due to 16K alignment are small, I'd be 
fine with setting the default to compatible. It would be a simpler patch, 
certainly.

-

PR Comment: https://git.openjdk.org/jdk/pull/19142#issuecomment-2101138971

RE: JDK-8170635 -- adding a base library to java

[Suchismith Roy]

Thanks for the reply @Magnus Ihse Bursie
Is there any example of PRs which create such libraries that I can refer to ?
Is OSAL similar to how os.cpp is defined and respective platforms implement 
them ?


From: Magnus Ihse Bursie 
Date: Tuesday, 7 May 2024 at 6:18 PM
To: Thomas Stüfe , Suchismith Roy 

Cc: build-dev@openjdk.org 
Subject: [EXTERNAL] Re: JDK-8170635 -- adding a base library to java
On 2024-05-06 16:36, Thomas Stüfe wrote:

> Not sure if you meant to address this mail to a specific person. I
> assume with proposal you mean this:
> https://mail.openjdk.org/pipermail/build-dev/2016-September/017746.html  ?
>
> If yes, my proposal was to move dladdr out of the OpenJDK code base
> into an independent library that would be maintained by IBM,
> hopefully, and would be a prerequisite for building the JDK.
> If no, whose proposal did you mean?

Oh, this is an old bug you're picking up Suchismith!

I read through the discussion from 2016. It seems that the suggestion to
make an external 3rd party library was only supported by Thomas, and
that the general agreement among the other participants was that we
should have a general, base-level "OSAL" (OS abstraction library) in the
JDK, that could be used by both Hotspot and libjli, as well as other JDK
libraries.

Creating such a library would be a much larger effort than just adding a
AIX implementation of dladdr to it, if it existed. The current structure
of the JDK does not readily lend itself to such a library, neither in
terms of source code layout nor build system.

With that said, I do think it would be good if we had such a library.
There are more cases than the AIX dladdr issue that is duplicated, like
jio_snprintf() and friends. This has actually caused some headaches when
doing static builds, since duplication of these functions are not
allowed when creating a single linked instance. (The current duplication
in dynamic libraries is just ugly and bad programming, not a compilation
error.)

But it is a much larger question than just fixing an AIX issue.

/Magnus

Integrated: 8331886: Allow markdown src file overrides

[Erik Joelsson]

On Wed, 8 May 2024 01:02:41 GMT, Erik Joelsson  wrote:

> For c, c++  and java source files, we have a built in system for letting more 
> specific files override if there are multiple files with the same name found, 
> by letting the first found file with a given name override any later found 
> files with that name. This is typically used for OS specific variants or when 
> needing to override a source file with a file from a custom source set. We 
> would like to make it possible to do the same for markdown files used to 
> generate man pages.
> 
> This will not have an immediate use i OpenJDK, but is needed for a custom 
> override in proprietary code.
> 
> The change in Docs.gmk removes unnecessary extra loops so that 
> SetupProcessMarkdown is called only once per module. This is necessary for 
> the override mechanism to kick in for each module src set.
> 
> The logic in ProcessMarkdown.gmk is more or less copied from 
> SetupNativeCompilation.

This pull request has now been integrated.

Changeset: 588e314e
Author:Erik Joelsson 
URL:   
https://git.openjdk.org/jdk/commit/588e314e4b96f2a48d46ab8a088a7b8d26be318d
Stats: 75 lines in 2 files changed: 14 ins; 11 del; 50 mod

8331886: Allow markdown src file overrides

Reviewed-by: ihse

-

PR: https://git.openjdk.org/jdk/pull/19132

Integrated: 8331939: Add custom hook for TestImage

[Erik Joelsson]

On Wed, 8 May 2024 14:03:48 GMT, Erik Joelsson  wrote:

> We need a custom hook in TestImage.gmk to modify behavior when building with 
> custom source.

This pull request has now been integrated.

Changeset: 0d1216c7
Author:Erik Joelsson 
URL:   
https://git.openjdk.org/jdk/commit/0d1216c7a1dc215550ac769afc21dea91c638215
Stats: 3 lines in 1 file changed: 3 ins; 0 del; 0 mod

8331939: Add custom hook for TestImage

Reviewed-by: mikael

-

PR: https://git.openjdk.org/jdk/pull/19139

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

[Magnus Ihse Bursie]

On Tue, 7 May 2024 16:52:12 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 105 commits:
> 
>  - 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
>  - Fix coment
>  - Fix comment
>  - Fix typo
>  - Revert some now unneded build changes
>  - Follow build tools naming convention
>  - ... and 95 more: https://git.openjdk.org/jdk/compare/23a72a1f...67aea4ca

make/Images.gmk line 100:

> 98: 
> 99: ifeq ($(JLINK_PRODUCE_RUNTIME_LINK_JDK), true)
> 100:   JLINK_JDK_EXTRA_OPTS := $(JLINK_JDK_EXTRA_OPTS) 
> --generate-linkable-runtime

I just noticed this slight improvement:

Suggestion:

  JLINK_JDK_EXTRA_OPTS += --generate-linkable-runtime

-

PR Review Comment: https://git.openjdk.org/jdk/pull/14787#discussion_r1594774220

Integrated: 8331541: [i386] linking with libjvm.so fails after JDK-8283326

[Vladimir Petko]

On Thu, 2 May 2024 08:23:38 GMT, Vladimir Petko  wrote:

> Hi, 
> 
> The `.type _SafeFetch32_impl,@function` symbol declaration contains a 
> spurious underscore causing an undefined symbol in libjvm.so.
> 
> I am not sure about change in default flags. I have removed the flag that 
> allows linking with undefined symbols
> because having the flag on might cause bugs like this one or 
> https://bugs.openjdk.org/browse/JDK-8329983 as the build succeeds even if 
> some symbols are not defined.
> Openjdk builds successfully without it on amd64, i386, armhf, arm64, s390, 
> ppc64el and riscv64 with this change[1]. riscv64 build was done locally 
> inside vm:
> 
> Finished building target 'images' in configuration 
> 'linux-riscv64-server-release'
> ubuntu@ubuntu:~/jdk$ 
> 
> Unfortunately I do not know why it was introduced, so I might be missing 
> something.
> I am happy to drop it from this one or move it to a separate issue/pr.
>  
> [1] 
> https://launchpad.net/~vpa1977/+archive/ubuntu/october-21/+sourcepub/16076564/+listing-archive-extra

This pull request has now been integrated.

Changeset: 2d622152
Author:Vladimir Petko 
Committer: Magnus Ihse Bursie 
URL:   
https://git.openjdk.org/jdk/commit/2d622152b07bba0aba8fd5b1e24293e28d6e69f5
Stats: 2 lines in 2 files changed: 0 ins; 1 del; 1 mod

8331541: [i386] linking with libjvm.so fails after JDK-8283326

Reviewed-by: djelinski, ihse

-

PR: https://git.openjdk.org/jdk/pull/19048

RFR: 8331952: --enable-compatible-cds-alignment should be enabled by default

[xiaotaonan]

--enable-compatible-cds-alignment should be enabled by default

-

Commit messages:
 - --enable-compatible-cds-alignment should be enabled by default

Changes: https://git.openjdk.org/jdk/pull/19150/files
  Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=19150&range=00
  Issue: https://bugs.openjdk.org/browse/JDK-8331952
  Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod
  Patch: https://git.openjdk.org/jdk/pull/19150.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/19150/head:pull/19150

PR: https://git.openjdk.org/jdk/pull/19150

Withdrawn: 8311846: Resolve duplicate 'Thread' related symbols with JDK static linking

[duke]

On Wed, 17 Jan 2024 00:14:58 GMT, Jiangli Zhou  wrote:

> Please review this PR with a simple solution for resolving duplicate `Thread` 
> symbol issue. In https://github.com/openjdk/jdk/pull/14808 comments, there 
> was an alternative suggestion to redefine the symbol at build time, such as  
> using`-DThread=HotSpotThread`. That would not address issues when symbol were 
> references as string literals. https://github.com/openjdk/jdk/pull/14808 also 
> discussed using namespace for hotspot code, which can have multiple 
> benefits/motivations. We could explore further using namespace with more 
> consensus on that approach.
> 
> Contributed by Chuck Rasbold and @jianglizhou.

This pull request has been closed without being integrated.

-

PR: https://git.openjdk.org/jdk/pull/17456

Re: RFR: 8331942: On Linux aarch64, CDS archives should be using 64K alignment by default

[Thomas Stuefe]

On Wed, 8 May 2024 15:14:16 GMT, Thomas Stuefe  wrote:

> On Linux aarch64, a JVM may encounter three different page sizes: 4K, 64K, 
> and (when run on Mac M1 hardware) 16K.
> 
> Since forgetting to specify `--enable-compatible-cds-alignment` is a common 
> error that is only noticed when running the produced JVM on hardware with 
> different page size, we propose to enable that option by default on Linux 
> aarch64. The cost is a moderate increase in CDS archive size (about 300K).
> 
> I tested this patch on:
> - x64 Linux
> - x64 Linux, crossbuilding to aarch64
> - building natively on aarch64 Linux

Does anyone else have an opinion on this matter? The question is:

"Should we enable cds-compatible alignment in the configure options by default 
generally, not only for Linux aarch64" ?

This would, in addition to the proposed change affecting Linux on aarch64, also 
affect MacOS x64. JVMs built there would be able to run, by default, in 
Rosetta. At the price of sligthly increased archive size of a dozen KB or so.

If nobody objects, I will do that since I like the simplicity of it.

-

PR Comment: https://git.openjdk.org/jdk/pull/19142#issuecomment-2101941697