Re: RFR: JDK-8317799 : AIX PPC64: FFI symbol lookup doesn't find symbols
On Fri, 10 Nov 2023 07:56:46 GMT, suchismith1993 wrote: > > That said, I think we should limit the exported symbols to only those found > > in the standard C library header files: > > https://en.cppreference.com/w/c/header > > Currently the symbols are restricted to some parts of libc and the entire > math library libm. Are you suggesting we restrict the list to common math > functions such as https://en.cppreference.com/w/c/numeric/math ? I'm suggesting to include things that are defined by the C standard, plus some platform specific extensions. i.e. limit this to what is considered part of the public/supported API of the standard library on AIX. - PR Comment: https://git.openjdk.org/jdk/pull/16414#issuecomment-1807614260
Re: RFR: JDK-8317799 : AIX PPC64: FFI symbol lookup doesn't find symbols
On Fri, 10 Nov 2023 16:30:35 GMT, suchismith1993 wrote: > There is not generic way of generating this. It needs a manual intervention > and symbols are to be added on a need basis in future. Looks like a list to > be maintained. I had tried adding comments to explain the list, but that > causes build failures. Would it be possible to paste the summary of the "instructions" to generate this? My initial reaction when seeing this PR is to wonder why it can't be generated at build time but from the discussion, it seems like it's a subset of the symbols but it is really just the functions used by libjvm or is it all the native libraries? - PR Comment: https://git.openjdk.org/jdk/pull/16414#issuecomment-1807608795
Re: RFR: JDK-8317799 : AIX PPC64: FFI symbol lookup doesn't find symbols
On Mon, 13 Nov 2023 02:32:48 GMT, David Holmes wrote: > I cannot decide if this is an AIX issue for not using dynamic libraries; a > FFI issue for assuming dynamic libraries; or a test issue. Won't this be a > general problem for any function you try to access via FFI that is statically > linked? Not sure what FFI is expected to do differently here. It is not possible to load a static library at runtime and look up functions inside of it, as far as I know. - PR Comment: https://git.openjdk.org/jdk/pull/16414#issuecomment-1807600121
Re: RFR: JDK-8319123 : Implementation of JEP-461: Stream Gatherers (Preview) [v4]
> This Pull-Request implements [JEP-461](https://openjdk.org/jeps/461) Viktor Klang has updated the pull request incrementally with one additional commit since the last revision: Addressing further review feedback - Changes: - all: https://git.openjdk.org/jdk/pull/16420/files - new: https://git.openjdk.org/jdk/pull/16420/files/74936cdd..a4745ccf Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=16420&range=03 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=16420&range=02-03 Stats: 123 lines in 4 files changed: 49 ins; 3 del; 71 mod Patch: https://git.openjdk.org/jdk/pull/16420.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/16420/head:pull/16420 PR: https://git.openjdk.org/jdk/pull/16420
Re: RFR: 8318776: Require supports_cx8 to always be true [v3]
> As discussed in JBS all platforms (some tweaks to Zero are in progress) > actually do support `cx8` i.e. 64-bit compare-and-exchange, so we can strip > out the locked-based alternatives to using it and just add a guarantee that > it is true at runtime. And all platforms except some ARM variants set > `SUPPORTS_NATIVE_CX8`, so we can greatly simplify things. Summary of changes: > - `_supports_cx8` field is only needed when `SUPPORTS_NATIVE_CX8` is not > defined > - Assertions for `supports_cx8()` are removed > - Compiler predicates requiring `supports_cx8()` are removed > - Access backend is greatly simplified without the need for lock-based > alternative > - `java.util.concurrent.AtomicLongFieldUpdater` is simplified without the > need for a lock-based alternative > > I did consider moving all the ARM `kuser_helper` related code to be only > defined when `SUPPORTS_NATIVE_CX8` is not defined, but there was a > theoretical risk this could change the behaviour if ARMv7 binaries were run > on other ARM CPU's. I added a note to that effect in > vm_version_linux_arm32.cpp so the ARM port maintainers could clean this up > further if desired. > > Testing: > - All Oracle tiers 1-5 builds (which includes an ARMv7 build) > - GHA builds/tests > - Oracle tiers 1-3 sanity testing > > Zero changes coming in via > [JDK-8319777](https://bugs.openjdk.org/browse/JDK-8319777) will be merged > when they arrive. > > Thanks. David Holmes has updated the pull request incrementally with two additional commits since the last revision: - Remove cx8 comment as no longer relevant (the spinlock is used regardless of cx8) - Remove suports_cx8() checks from gtest - Changes: - all: https://git.openjdk.org/jdk/pull/16625/files - new: https://git.openjdk.org/jdk/pull/16625/files/b6dea4b6..65871144 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=16625&range=02 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=16625&range=01-02 Stats: 14 lines in 2 files changed: 0 ins; 14 del; 0 mod Patch: https://git.openjdk.org/jdk/pull/16625.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/16625/head:pull/16625 PR: https://git.openjdk.org/jdk/pull/16625
Re: RFR: 8318776: Require supports_cx8 to always be true [v2]
> As discussed in JBS all platforms (some tweaks to Zero are in progress) > actually do support `cx8` i.e. 64-bit compare-and-exchange, so we can strip > out the locked-based alternatives to using it and just add a guarantee that > it is true at runtime. And all platforms except some ARM variants set > `SUPPORTS_NATIVE_CX8`, so we can greatly simplify things. Summary of changes: > - `_supports_cx8` field is only needed when `SUPPORTS_NATIVE_CX8` is not > defined > - Assertions for `supports_cx8()` are removed > - Compiler predicates requiring `supports_cx8()` are removed > - Access backend is greatly simplified without the need for lock-based > alternative > - `java.util.concurrent.AtomicLongFieldUpdater` is simplified without the > need for a lock-based alternative > > I did consider moving all the ARM `kuser_helper` related code to be only > defined when `SUPPORTS_NATIVE_CX8` is not defined, but there was a > theoretical risk this could change the behaviour if ARMv7 binaries were run > on other ARM CPU's. I added a note to that effect in > vm_version_linux_arm32.cpp so the ARM port maintainers could clean this up > further if desired. > > Testing: > - All Oracle tiers 1-5 builds (which includes an ARMv7 build) > - GHA builds/tests > - Oracle tiers 1-3 sanity testing > > Zero changes coming in via > [JDK-8319777](https://bugs.openjdk.org/browse/JDK-8319777) will be merged > when they arrive. > > Thanks. David Holmes has updated the pull request incrementally with one additional commit since the last revision: Remove test for VMSupportsCX8 - Changes: - all: https://git.openjdk.org/jdk/pull/16625/files - new: https://git.openjdk.org/jdk/pull/16625/files/3f2ec66f..b6dea4b6 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=16625&range=01 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=16625&range=00-01 Stats: 53 lines in 1 file changed: 0 ins; 53 del; 0 mod Patch: https://git.openjdk.org/jdk/pull/16625.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/16625/head:pull/16625 PR: https://git.openjdk.org/jdk/pull/16625
RFR: 8318776: Require supports_cx8 to always be true
As discussed in JBS all platforms (some tweaks to Zero are in progress) actually do support `cx8` i.e. 64-bit compare-and-exchange, so we can strip out the locked-based alternatives to using it and just add a guarantee that it is true at runtime. And all platforms except some ARM variants set `SUPPORTS_NATIVE_CX8`, so we can greatly simplify things. Summary of changes: - `_supports_cx8` field is only needed when `SUPPORTS_NATIVE_CX8` is not defined - Assertions for `supports_cx8()` are removed - Access backend is greatly simplified without the need for lock-based alternative - `java.util.concurrent.AtomicLongFieldUpdater` is simplified without the need for a lock-based alternative I did consider moving all the ARM `kuser_helper` related code to be only defined when `SUPPORTS_NATIVE_CX8` is not defined, but there was a theoretical risk this could change the behaviour if ARMv7 binaries were run on other ARM CPU's. I added a note to that effect in vm_version_linux_arm32.cpp so the ARM port maintainers could clean this up further if desired. Testing: - All Oracle tiers 1-5 builds (which includes an ARMv7 build) - GHA builds/tests - Oracle tiers 1-3 sanity testing Zero changes coming in via [JDK-8319777](https://bugs.openjdk.org/browse/JDK-8319777) will be merged when they arrive. Thanks. - Commit messages: - 8318776: Require supports_cx8 to always be true Changes: https://git.openjdk.org/jdk/pull/16625/files Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=16625&range=00 Issue: https://bugs.openjdk.org/browse/JDK-8318776 Stats: 386 lines in 35 files changed: 14 ins; 359 del; 13 mod Patch: https://git.openjdk.org/jdk/pull/16625.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/16625/head:pull/16625 PR: https://git.openjdk.org/jdk/pull/16625
Re: RFR: JDK-8317799 : AIX PPC64: FFI symbol lookup doesn't find symbols
On Mon, 30 Oct 2023 10:54:48 GMT, suchismith1993 wrote: > 1. Adding required compiler flags. > 2. Adding required symbols. > > JBS-ISSUE : [JDK-8317799](https://bugs.openjdk.org/browse/JDK-8317799) I cannot decide if this is an AIX issue for not using dynamic libraries; a FFI issue for assuming dynamic libraries; or a test issue. Won't this be a general problem for any function you try to access via FFI that is statically linked? - PR Comment: https://git.openjdk.org/jdk/pull/16414#issuecomment-1807386320
RFR: JDK-8319628: DateFormat does not mention IllegalArgumentException for invalid style args
Please review this PR and [CSR](https://bugs.openjdk.org/browse/JDK-8319868) which makes IllegalArgumentExceptions apparent for the _j.text.DateFormat_ static factory methods that have the _style_ parameter. - Commit messages: - init Changes: https://git.openjdk.org/jdk/pull/16624/files Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=16624&range=00 Issue: https://bugs.openjdk.org/browse/JDK-8319628 Stats: 6 lines in 1 file changed: 2 ins; 0 del; 4 mod Patch: https://git.openjdk.org/jdk/pull/16624.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/16624/head:pull/16624 PR: https://git.openjdk.org/jdk/pull/16624
Integrated: JDK-8318189: ChoiceFormat::format throws undocumented AIOOBE
On Thu, 9 Nov 2023 21:58:12 GMT, Justin Lu wrote: > Please review this PR which makes an `ArrayIndexOutOfBoundsException` > apparent in ChoiceFormat::format. > > An _incomplete_ ChoiceFormat can be created, which is not revealed until > format is invoked. This pull request has now been integrated. Changeset: caf71810 Author:Justin Lu URL: https://git.openjdk.org/jdk/commit/caf71810f85ea55083ce7d1c76307a0c42d9be0e Stats: 13 lines in 1 file changed: 11 ins; 0 del; 2 mod 8318189: ChoiceFormat::format throws undocumented AIOOBE Reviewed-by: naoto - PR: https://git.openjdk.org/jdk/pull/16587
Re: RFR: 8311302: Allow for jlinking a custom runtime without packaged modules being present [v6]
On Wed, 18 Oct 2023 17:37:30 GMT, Severin Gehwolf 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.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.management.jmod >> jdk.jlink.jmod jdk.naming.dns.j... > > Severin Gehwolf has updated the pull request incrementally with four > additional commits since the last revision: > > - Add a message when a run-image based link is being performed > - Add hint to the modified file case > - AddJmodResourcesPlugin => AddRunImageResourcesPlugin rename > - --add-jmod-resources => --add-run-image-resources rename > >Also rename jmod_resources to runimage_resources I looked at the latest proposal and CSR. Overall I think the proposal is good and you've got the right default to fail if the run-time image has been modified. So I think we are down to a few lesser topics now. I'm wondering if the CLI option to override, meaning a first run-time image containing the jdk.jlink module generates a second run-time image, and the first run-time image has been modified, whether this is really needed. I'm wondering about this because this CLI option is hard to explain, will developers really understand it? If there is an CLI option then we'll need to find a better name, I don't think "--unlock-run-time" works as a name (the usage of options talk about "runtime image" for example, maybe "-ignore-signing-information" can provide inspiration as an override too). Can --add-run-image-resources be dropped? Exposing this in the interface feels like it's an attractive nuisance and now clear (to me anyway) what developers would do with this. A few very minor things that I jotted down while looking at the current proposal: - Adding a resource to serve as a marker that indicates it was created without the packaged modules is fine. I think the name should be looked as "runimage" is a bit consistent for this area. I'm also wondering if it would be better to hide in jdk/internal somewhere to avoid any tooling assuming it's a supported interface. - The error message exclaims "Please double check!", I think that error message will need to be tweaked once we get down to details like this. - PR Comment: https://git.openjdk.org/jdk/pull/14787#issuecomment-1807100511
Re: RFR: 8316557: Make fields final in 'sun.util' package
On Thu, 14 Sep 2023 08:58:56 GMT, Andrey Turbanov wrote: > A few classes in `sun.util` package have non-final fields which could easily > be marked `final`. Can I get a review? - PR Comment: https://git.openjdk.org/jdk/pull/15736#issuecomment-1807066050
