Re: RFR: 8283660: Convert com/sun/jndi/ldap/AbstractLdapNamingEnumeration.java finalizer to Cleaner [v16]
On Fri, 22 Jul 2022 20:51:59 GMT, Brent Christian wrote: >> Please review this change to replace the finalizer in >> `AbstractLdapNamingEnumeration` with a Cleaner. >> >> The pieces of state required for cleanup (`LdapCtx homeCtx`, `LdapResult >> res`, and `LdapClient enumClnt`) are moved to a static inner class . From >> there, the change is fairly mechanical. >> >> Details of note: >> 1. Some operations need to change the state values (the update() method is >> probably the most interesting). >> 2. Subclasses need to access `homeCtx`; I added a `homeCtx()` method to read >> `homeCtx` from the superclass's `state`. >> >> The test case is based on a copy of >> `com/sun/jndi/ldap/blits/AddTests/AddNewEntry.java`. A more minimal test >> case might be possible, but this was done for expediency. >> >> The test only confirms that the new Cleaner use does not keep the object >> reachable. It only tests `LdapSearchEnumeration` (not >> `LdapNamingEnumeration` or `LdapBindingEnumeration`, though all are >> subclasses of `AbstractLdapNamingEnumeration`). >> >> Thanks. > > Brent Christian has updated the pull request incrementally with one > additional commit since the last revision: > > remove some more tabs Not yet - PR Comment: https://git.openjdk.org/jdk/pull/8311#issuecomment-1542989403
RFR: 8300794: Use @snippet in java.util:i18n
Please review this javadoc only change which uses `@snippet` and `@linkplain` in i18n related util packages. - Commit messages: - copyright years - property resource bundle changes - list resource bundle changes - resource bundle changes - currency changes - locale changes Changes: https://git.openjdk.org/jdk/pull/13920/files Webrev: https://webrevs.openjdk.org/?repo=jdk=13920=00 Issue: https://bugs.openjdk.org/browse/JDK-8300794 Stats: 127 lines in 5 files changed: 7 ins; 24 del; 96 mod Patch: https://git.openjdk.org/jdk/pull/13920.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/13920/head:pull/13920 PR: https://git.openjdk.org/jdk/pull/13920
Integrated: 8281103: Give example for Locale that is English and follows the ISO standards
On Tue, 9 May 2023 20:39:19 GMT, Justin Lu wrote: > Please review this PR which adds an example snippet to > `java.time.temporal.WeekFields.of(Locale locale)` > > The snippet demonstrates how to create a Locale that has English Locale > qualities with an ISO-8601 first day of the week. > > This snippet was added as there was a desire for an English/ISO-8601 Locale > constant to be added, but the existing methods in the JDK are better suited > for accomplishing this. This pull request has now been integrated. Changeset: 4795c395 Author:Justin Lu URL: https://git.openjdk.org/jdk/commit/4795c395e9799719dfcdd947fe8905f25f3a11a2 Stats: 9 lines in 1 file changed: 8 ins; 0 del; 1 mod 8281103: Give example for Locale that is English and follows the ISO standards Reviewed-by: rriggs, lancea, naoto - PR: https://git.openjdk.org/jdk/pull/13893
Re: RFR: 8303040: linux PPC64le: Implementation of Foreign Function & Memory API (Preview) [v22]
On Wed, 10 May 2023 14:26:54 GMT, Jorn Vernee wrote: >> FWIW, since Shenandoah changed their load barriers we have been cleaning >> away the usages of the Access API for loads and stores to primitive values. >> There's no such support in the C++ Runtime code. > > Ok, since this is loading a `long` (which represents an address that points > into the code cache) I think we're fine without using the access API then? Correct. The code had been written for the previous version of Shenandoah (1.0). No current GC uses barriers for non-oop types and the C++ Runtime doesn't support it any more as Stefan pointed out. It is still possible to use the access API on other platforms, but it does nothing more than a plain load/store for non-oop types. - PR Review Comment: https://git.openjdk.org/jdk/pull/12708#discussion_r1190447075
Re: RFR: 8307547: Support for multiple collations for a locale [v2]
On Wed, 10 May 2023 20:28:11 GMT, Naoto Sato wrote: >> The fix to https://bugs.openjdk.org/browse/JDK-8306927 switched the default >> collation for Swedish to the modern one. In order to provide a means for >> users who need the old collation, this PR intends to make `Collator` >> recognize the `co` Unicode locale extension so that multiple implementations >> for a locale can be provided. I would also like reviews for the >> corresponding CSR. > > Naoto Sato has updated the pull request incrementally with one additional > commit since the last revision: > > http -> https Thanks, Justin > does it make sense to add some other locale's collation variants? Maybe, but I am not seeing any requests for adding more collation variants. This enhancement is mainly for those who need the old Swedish collation as a workaround - PR Comment: https://git.openjdk.org/jdk/pull/13917#issuecomment-1542854951
Re: RFR: 8307547: Support for multiple collations for a locale [v2]
On Wed, 10 May 2023 20:28:11 GMT, Naoto Sato wrote: >> The fix to https://bugs.openjdk.org/browse/JDK-8306927 switched the default >> collation for Swedish to the modern one. In order to provide a means for >> users who need the old collation, this PR intends to make `Collator` >> recognize the `co` Unicode locale extension so that multiple implementations >> for a locale can be provided. I would also like reviews for the >> corresponding CSR. > > Naoto Sato has updated the pull request incrementally with one additional > commit since the last revision: > > http -> https LGTM, does it make sense to add some other locale's collation variants? - PR Comment: https://git.openjdk.org/jdk/pull/13917#issuecomment-1542828520
Re: RFR: 8307194: Add make target for optionally building a complete set of all JDK and hotspot libjvm static libraries [v12]
On Wed, 10 May 2023 17:13:13 GMT, Jiangli Zhou wrote: >> This PR is branched from the makefile changes for >> https://bugs.openjdk.org/browse/JDK-8303796 and contains the following for >> handling the JDK/hotspot static libraries: >> >> - Build hotspot libjvm.a and JDK static libraries for >> static-libs-image/static-libs-bundles targets; This change does not affect >> the graal-builder-image target >> >> - For libjvm.a specifically, exclude operator_new.o >> >> - Filter out "external" .o files (those are the .o files included from a >> different JDK library and needed when creating the .so shared library only) >> from JDK .a libraries; That's to avoid linker failures caused by duplicate >> symbols >> - For libjli.a: Not include inflate.o inftrees.o inffast.o zadler32.o >> zcrc32.o zutil.o (compiled from zlib sources) if zlib is built as JDK bundled >> - For libawt_xawt.a and libawt_head.a: Not include systemScale.o, since >> it's provided in libawt.a >> >> - Handle long arguments case for static build in >> make/common/NativeCompilation.gmk >> >> - Address @erikj79's comment in >> https://github.com/openjdk/jdk/pull/13709#discussion_r1180750185 for >> LIBJLI_STATIC_EXCLUDE_OBJS > > Jiangli Zhou 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 13 additional > commits since the last revision: > > - Merge branch 'master' into JDK-8307194 > - Merge branch 'master' into JDK-8307194 > - Address comments from @erikj79: >- Fix to use $(STATIC_LIBS_GRAAL_IMAGE_DIR) in GraalBuilderImage.gmk. >- Split the long line at 1281 in Main.gmk. > - Fix whitspace errors. > - Reflect on @erikj79 suggestions/input on > static-libs-image/graal-builder-image and supporting libjvm.a for different > JVM_VARIANTS: > >- Remove the new java-static-libs and java-static-libs-bundles targets. > Delete the new make/StaticJvmLibsImage.gmk. > >- Restore HOTSPOT_VARIANT_STATIC_LIBS_TARGETS and related definitions from > previous revision for different $(JVM_VARIANTS). >- Change the existing static-libs-image target to build libjvm.a in > addition to JDK static libraries. The libjvm.a is placed under > images/static-libs/lib/. When building multiple JVM variants, > each variant contains its own libjvm.a under images/static-libs/lib/ > directory. > >- Add a new static-libs-graal-image target, which is used by > graal-builder-image. Hotspot libjvm.a is not created when building > static-libs-graal-image and graal-builder-image targets. > - Fix whitespace error in make/StaticJvmLibsImage.gmk. > - - Separate building libjvm.a from static-libs-image target, based on input > from @jerboaa and @olpaw: > - Add a new java-static-libs-image target in Main.gmk for creating the > JDK .a static libraries and libjvm.a super set. The static libraries are > placed in images/static-libs/lib. The existing static-libs-image target is > not affected and will not include hotspot libjvm.a. > - Add java-static-libs-bundles target in Bundles.gmk. The created > .tar.gz bundle contains JDK .a static libraries and hotspot libjvm.a. > - Add StaticJvmLibsImage.gmk for placing libjvm.a into > images/static-libs/lib. >- Further cleanup after incorporating erikj79's suggestion to only build > libjvm.a for $(JVM_VARIANT_MAIN) for now: > - Change HOTSPOT_VARIANT_STATIC_LIBS_TARGETS to > HOTSPOT_VARIANT_MAIN_STATIC_LIBS_TARGETS in Main.gmk. > - Change hotspot-$v-static-libs to > hotspot-$(JVM_VARIANT_MAIN)-static-libs in Main.gmk. > - Update to create and use only hotspot-$(JVM_VARIANT_MAIN)-static-libs, > based on @erikj79 input. > - Update make/StaticLibsIm... > > > This change caused all our builds but Linux to fail. Did you verify on > > > other platforms than Linux at all? I see you have GHA turned off. > > > > > > Sorry about the issue. There were failed workflows after I merged with the > > latest JDK master this morning. I canceled the workflow and tried a rerun, > > which didn't work. I retried with a new commit with merging again. And > > noticed the GHA was off after merging with the latest master, but didn't > > know what caused it. The last night tests in the workflow (before merging > > with the master) were successful. So I proceeded with integration. > > Please let me know if we should back out or fix forward ... > > Dan is already backing out in #13918, so we have plenty of time to figure out > the issues. Ok, thanks! Sorry about the build failure again. - PR Comment: https://git.openjdk.org/jdk/pull/13768#issuecomment-1542817870
Re: RFR: 8307194: Add make target for optionally building a complete set of all JDK and hotspot libjvm static libraries [v12]
On Wed, 10 May 2023 20:38:50 GMT, Jiangli Zhou wrote: > > This change caused all our builds but Linux to fail. Did you verify on > > other platforms than Linux at all? I see you have GHA turned off. > > Sorry about the issue. There were failed workflows after I merged with the > latest JDK master this morning. I canceled the workflow and tried a rerun, > which didn't work. I retried with a new commit with merging again. And > noticed the GHA was off after merging with the latest master, but didn't know > what caused it. The last night tests in the workflow (before merging with the > master) were successful. So I proceeded with integration. > > Please let me know if we should back out or fix forward ... Dan is already backing out in https://github.com/openjdk/jdk/pull/13918, so we have plenty of time to figure out the issues. - PR Comment: https://git.openjdk.org/jdk/pull/13768#issuecomment-1542805621
Re: RFR: 8307194: Add make target for optionally building a complete set of all JDK and hotspot libjvm static libraries [v12]
On Wed, 10 May 2023 20:24:30 GMT, Erik Joelsson wrote: > This change caused all our builds but Linux to fail. Did you verify on other > platforms than Linux at all? I see you have GHA turned off. Sorry about the issue. There were failed workflows after I merged with the latest JDK master this morning. I canceled the workflow and tried a rerun, which didn't work. I retried with a new commit with merging again. And noticed the GHA was off after merging with the latest master, but didn't know what caused it. The last night tests in the workflow (before merging with the master) were successful. So I proceeded with integration. Please let me know if we should back out or fix forward ... - PR Comment: https://git.openjdk.org/jdk/pull/13768#issuecomment-1542779168
Re: RFR: 8307547: Support for multiple collations for a locale [v2]
> The fix to https://bugs.openjdk.org/browse/JDK-8306927 switched the default > collation for Swedish to the modern one. In order to provide a means for > users who need the old collation, this PR intends to make `Collator` > recognize the `co` Unicode locale extension so that multiple implementations > for a locale can be provided. I would also like reviews for the corresponding > CSR. Naoto Sato has updated the pull request incrementally with one additional commit since the last revision: http -> https - Changes: - all: https://git.openjdk.org/jdk/pull/13917/files - new: https://git.openjdk.org/jdk/pull/13917/files/276178f8..9f0eb048 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=13917=01 - incr: https://webrevs.openjdk.org/?repo=jdk=13917=00-01 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.org/jdk/pull/13917.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/13917/head:pull/13917 PR: https://git.openjdk.org/jdk/pull/13917
Re: RFR: 8307194: Add make target for optionally building a complete set of all JDK and hotspot libjvm static libraries [v12]
On Wed, 10 May 2023 17:13:13 GMT, Jiangli Zhou wrote: >> This PR is branched from the makefile changes for >> https://bugs.openjdk.org/browse/JDK-8303796 and contains the following for >> handling the JDK/hotspot static libraries: >> >> - Build hotspot libjvm.a and JDK static libraries for >> static-libs-image/static-libs-bundles targets; This change does not affect >> the graal-builder-image target >> >> - For libjvm.a specifically, exclude operator_new.o >> >> - Filter out "external" .o files (those are the .o files included from a >> different JDK library and needed when creating the .so shared library only) >> from JDK .a libraries; That's to avoid linker failures caused by duplicate >> symbols >> - For libjli.a: Not include inflate.o inftrees.o inffast.o zadler32.o >> zcrc32.o zutil.o (compiled from zlib sources) if zlib is built as JDK bundled >> - For libawt_xawt.a and libawt_head.a: Not include systemScale.o, since >> it's provided in libawt.a >> >> - Handle long arguments case for static build in >> make/common/NativeCompilation.gmk >> >> - Address @erikj79's comment in >> https://github.com/openjdk/jdk/pull/13709#discussion_r1180750185 for >> LIBJLI_STATIC_EXCLUDE_OBJS > > Jiangli Zhou 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 13 additional > commits since the last revision: > > - Merge branch 'master' into JDK-8307194 > - Merge branch 'master' into JDK-8307194 > - Address comments from @erikj79: >- Fix to use $(STATIC_LIBS_GRAAL_IMAGE_DIR) in GraalBuilderImage.gmk. >- Split the long line at 1281 in Main.gmk. > - Fix whitspace errors. > - Reflect on @erikj79 suggestions/input on > static-libs-image/graal-builder-image and supporting libjvm.a for different > JVM_VARIANTS: > >- Remove the new java-static-libs and java-static-libs-bundles targets. > Delete the new make/StaticJvmLibsImage.gmk. > >- Restore HOTSPOT_VARIANT_STATIC_LIBS_TARGETS and related definitions from > previous revision for different $(JVM_VARIANTS). >- Change the existing static-libs-image target to build libjvm.a in > addition to JDK static libraries. The libjvm.a is placed under > images/static-libs/lib/. When building multiple JVM variants, > each variant contains its own libjvm.a under images/static-libs/lib/ > directory. > >- Add a new static-libs-graal-image target, which is used by > graal-builder-image. Hotspot libjvm.a is not created when building > static-libs-graal-image and graal-builder-image targets. > - Fix whitespace error in make/StaticJvmLibsImage.gmk. > - - Separate building libjvm.a from static-libs-image target, based on input > from @jerboaa and @olpaw: > - Add a new java-static-libs-image target in Main.gmk for creating the > JDK .a static libraries and libjvm.a super set. The static libraries are > placed in images/static-libs/lib. The existing static-libs-image target is > not affected and will not include hotspot libjvm.a. > - Add java-static-libs-bundles target in Bundles.gmk. The created > .tar.gz bundle contains JDK .a static libraries and hotspot libjvm.a. > - Add StaticJvmLibsImage.gmk for placing libjvm.a into > images/static-libs/lib. >- Further cleanup after incorporating erikj79's suggestion to only build > libjvm.a for $(JVM_VARIANT_MAIN) for now: > - Change HOTSPOT_VARIANT_STATIC_LIBS_TARGETS to > HOTSPOT_VARIANT_MAIN_STATIC_LIBS_TARGETS in Main.gmk. > - Change hotspot-$v-static-libs to > hotspot-$(JVM_VARIANT_MAIN)-static-libs in Main.gmk. > - Update to create and use only hotspot-$(JVM_VARIANT_MAIN)-static-libs, > based on @erikj79 input. > - Update make/StaticLibsIm... This change caused all our builds but Linux to fail. Did you verify on other platforms than Linux at all? I see you have GHA turned off. - PR Comment: https://git.openjdk.org/jdk/pull/13768#issuecomment-1542764704
RFR: 8307547: Support for multiple collations for a locale
The fix to https://bugs.openjdk.org/browse/JDK-8306927 switched the default collation for Swedish to the modern one. In order to provide a means for users who need the old collation, this PR intends to make `Collator` recognize the `co` Unicode locale extension so that multiple implementations for a locale can be provided. I would also like reviews for the corresponding CSR. - Commit messages: - Test refinement - Clean-up - javadoc - 8307547: Support for multiple collation tables Changes: https://git.openjdk.org/jdk/pull/13917/files Webrev: https://webrevs.openjdk.org/?repo=jdk=13917=00 Issue: https://bugs.openjdk.org/browse/JDK-8307547 Stats: 71 lines in 4 files changed: 57 ins; 4 del; 10 mod Patch: https://git.openjdk.org/jdk/pull/13917.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/13917/head:pull/13917 PR: https://git.openjdk.org/jdk/pull/13917
Integrated: 8307194: Add make target for optionally building a complete set of all JDK and hotspot libjvm static libraries
On Wed, 3 May 2023 02:09:22 GMT, Jiangli Zhou wrote: > This PR is branched from the makefile changes for > https://bugs.openjdk.org/browse/JDK-8303796 and contains the following for > handling the JDK/hotspot static libraries: > > - Build hotspot libjvm.a and JDK static libraries for > static-libs-image/static-libs-bundles targets; This change does not affect > the graal-builder-image target > > - For libjvm.a specifically, exclude operator_new.o > > - Filter out "external" .o files (those are the .o files included from a > different JDK library and needed when creating the .so shared library only) > from JDK .a libraries; That's to avoid linker failures caused by duplicate > symbols > - For libjli.a: Not include inflate.o inftrees.o inffast.o zadler32.o > zcrc32.o zutil.o (compiled from zlib sources) if zlib is built as JDK bundled > - For libawt_xawt.a and libawt_head.a: Not include systemScale.o, since it's > provided in libawt.a > > - Handle long arguments case for static build in > make/common/NativeCompilation.gmk > > - Address @erikj79's comment in > https://github.com/openjdk/jdk/pull/13709#discussion_r1180750185 for > LIBJLI_STATIC_EXCLUDE_OBJS This pull request has now been integrated. Changeset: 1964954d Author:Jiangli Zhou URL: https://git.openjdk.org/jdk/commit/1964954da9ac1d020e0b5ba35893f475d86ec909 Stats: 178 lines in 8 files changed: 127 ins; 34 del; 17 mod 8307194: Add make target for optionally building a complete set of all JDK and hotspot libjvm static libraries Reviewed-by: erikj, sgehwolf - PR: https://git.openjdk.org/jdk/pull/13768
Re: RFR: 8307194: Add make target for optionally building a complete set of all JDK and hotspot libjvm static libraries [v12]
> This PR is branched from the makefile changes for > https://bugs.openjdk.org/browse/JDK-8303796 and contains the following for > handling the JDK/hotspot static libraries: > > - Build hotspot libjvm.a and JDK static libraries for > static-libs-image/static-libs-bundles targets; This change does not affect > the graal-builder-image target > > - For libjvm.a specifically, exclude operator_new.o > > - Filter out "external" .o files (those are the .o files included from a > different JDK library and needed when creating the .so shared library only) > from JDK .a libraries; That's to avoid linker failures caused by duplicate > symbols > - For libjli.a: Not include inflate.o inftrees.o inffast.o zadler32.o > zcrc32.o zutil.o (compiled from zlib sources) if zlib is built as JDK bundled > - For libawt_xawt.a and libawt_head.a: Not include systemScale.o, since it's > provided in libawt.a > > - Handle long arguments case for static build in > make/common/NativeCompilation.gmk > > - Address @erikj79's comment in > https://github.com/openjdk/jdk/pull/13709#discussion_r1180750185 for > LIBJLI_STATIC_EXCLUDE_OBJS Jiangli Zhou 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 13 additional commits since the last revision: - Merge branch 'master' into JDK-8307194 - Merge branch 'master' into JDK-8307194 - Address comments from @erikj79: - Fix to use $(STATIC_LIBS_GRAAL_IMAGE_DIR) in GraalBuilderImage.gmk. - Split the long line at 1281 in Main.gmk. - Fix whitspace errors. - Reflect on @erikj79 suggestions/input on static-libs-image/graal-builder-image and supporting libjvm.a for different JVM_VARIANTS: - Remove the new java-static-libs and java-static-libs-bundles targets. Delete the new make/StaticJvmLibsImage.gmk. - Restore HOTSPOT_VARIANT_STATIC_LIBS_TARGETS and related definitions from previous revision for different $(JVM_VARIANTS). - Change the existing static-libs-image target to build libjvm.a in addition to JDK static libraries. The libjvm.a is placed under images/static-libs/lib/. When building multiple JVM variants, each variant contains its own libjvm.a under images/static-libs/lib/ directory. - Add a new static-libs-graal-image target, which is used by graal-builder-image. Hotspot libjvm.a is not created when building static-libs-graal-image and graal-builder-image targets. - Fix whitespace error in make/StaticJvmLibsImage.gmk. - - Separate building libjvm.a from static-libs-image target, based on input from @jerboaa and @olpaw: - Add a new java-static-libs-image target in Main.gmk for creating the JDK .a static libraries and libjvm.a super set. The static libraries are placed in images/static-libs/lib. The existing static-libs-image target is not affected and will not include hotspot libjvm.a. - Add java-static-libs-bundles target in Bundles.gmk. The created .tar.gz bundle contains JDK .a static libraries and hotspot libjvm.a. - Add StaticJvmLibsImage.gmk for placing libjvm.a into images/static-libs/lib. - Further cleanup after incorporating erikj79's suggestion to only build libjvm.a for $(JVM_VARIANT_MAIN) for now: - Change HOTSPOT_VARIANT_STATIC_LIBS_TARGETS to HOTSPOT_VARIANT_MAIN_STATIC_LIBS_TARGETS in Main.gmk. - Change hotspot-$v-static-libs to hotspot-$(JVM_VARIANT_MAIN)-static-libs in Main.gmk. - Update to create and use only hotspot-$(JVM_VARIANT_MAIN)-static-libs, based on @erikj79 input. - Update make/StaticLibsImage.gmk thanks Co-authored-by: Erik Joelsson <37597443+erik...@users.noreply.github.com> - Update make/StaticLibsImage.gmk Thanks Co-authored-by: Erik Joelsson <37597443+erik...@users.noreply.github.com> - ... and 3 more: https://git.openjdk.org/jdk/compare/54e2bacc...ba2e9ce6 - Changes: - all: https://git.openjdk.org/jdk/pull/13768/files - new: https://git.openjdk.org/jdk/pull/13768/files/36526d4f..ba2e9ce6 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=13768=11 - incr: https://webrevs.openjdk.org/?repo=jdk=13768=10-11 Stats: 125 lines in 7 files changed: 26 ins; 96 del; 3 mod Patch: https://git.openjdk.org/jdk/pull/13768.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/13768/head:pull/13768 PR: https://git.openjdk.org/jdk/pull/13768
Integrated: JDK-8151531: Add notes to BaseStream.spliterator/iterator docs regarding them being escape hatches
On Tue, 9 May 2023 15:50:13 GMT, Viktor Klang wrote: > Still relevant to address? /cc @PaulSandoz This pull request has now been integrated. Changeset: 8a95020a Author:Viktor Klang Committer: Paul Sandoz URL: https://git.openjdk.org/jdk/commit/8a95020ab6c61f25954a56e1965529bb2f6f58af Stats: 11 lines in 1 file changed: 10 ins; 0 del; 1 mod 8151531: Add notes to BaseStream.spliterator/iterator docs regarding them being escape hatches Reviewed-by: psandoz - PR: https://git.openjdk.org/jdk/pull/13890
Integrated: 8305748: Clarify reentrant behavior of close() in FileInputStream, FileOutputStream, and RandomAccessFile
On Thu, 6 Apr 2023 22:36:33 GMT, Archie Cobbs wrote: > IO stream classes like `FileOutputStream` can have assocated NIO channels. > > When `close()` is invoked on one of these classes, it in turn invokes > `close()` on the associated channel (if any). But when the associated > channel's `close()` method is invoked, it in turn invokes `close()` on the > associated stream (if any). > > As a result, these IO stream `close()` methods invoke themselves reentrantly > when there is an associated channel. > > This is not a problem for these classes because they are written to handle > this (i.e., they are idempotent), but it can be surprising (or worse, just > silently bug-inducing) for subclasses that override `close()`. > > There are two possible ways to address this: > 1. Modify the code to detect and avoid the (unnecessary) reentrant invocations > 2. Add a `@implNote` to the Javadoc so subclass implementers are made aware > > This patch takes the second, more conservative approach. This pull request has now been integrated. Changeset: 0198afca Author:Archie Cobbs Committer: Brian Burkhalter URL: https://git.openjdk.org/jdk/commit/0198afca3ac1a7c421b0669ae2180eee3e4f1482 Stats: 17 lines in 4 files changed: 16 ins; 0 del; 1 mod 8305748: Clarify reentrant behavior of close() in FileInputStream, FileOutputStream, and RandomAccessFile Reviewed-by: alanb, bpb - PR: https://git.openjdk.org/jdk/pull/13379
Re: RFR: 8307194: Add make target for optionally building a complete set of all JDK and hotspot libjvm static libraries [v11]
> This PR is branched from the makefile changes for > https://bugs.openjdk.org/browse/JDK-8303796 and contains the following for > handling the JDK/hotspot static libraries: > > - Build hotspot libjvm.a and JDK static libraries for > static-libs-image/static-libs-bundles targets; This change does not affect > the graal-builder-image target > > - For libjvm.a specifically, exclude operator_new.o > > - Filter out "external" .o files (those are the .o files included from a > different JDK library and needed when creating the .so shared library only) > from JDK .a libraries; That's to avoid linker failures caused by duplicate > symbols > - For libjli.a: Not include inflate.o inftrees.o inffast.o zadler32.o > zcrc32.o zutil.o (compiled from zlib sources) if zlib is built as JDK bundled > - For libawt_xawt.a and libawt_head.a: Not include systemScale.o, since it's > provided in libawt.a > > - Handle long arguments case for static build in > make/common/NativeCompilation.gmk > > - Address @erikj79's comment in > https://github.com/openjdk/jdk/pull/13709#discussion_r1180750185 for > LIBJLI_STATIC_EXCLUDE_OBJS Jiangli Zhou 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 12 additional commits since the last revision: - Merge branch 'master' into JDK-8307194 - Address comments from @erikj79: - Fix to use $(STATIC_LIBS_GRAAL_IMAGE_DIR) in GraalBuilderImage.gmk. - Split the long line at 1281 in Main.gmk. - Fix whitspace errors. - Reflect on @erikj79 suggestions/input on static-libs-image/graal-builder-image and supporting libjvm.a for different JVM_VARIANTS: - Remove the new java-static-libs and java-static-libs-bundles targets. Delete the new make/StaticJvmLibsImage.gmk. - Restore HOTSPOT_VARIANT_STATIC_LIBS_TARGETS and related definitions from previous revision for different $(JVM_VARIANTS). - Change the existing static-libs-image target to build libjvm.a in addition to JDK static libraries. The libjvm.a is placed under images/static-libs/lib/. When building multiple JVM variants, each variant contains its own libjvm.a under images/static-libs/lib/ directory. - Add a new static-libs-graal-image target, which is used by graal-builder-image. Hotspot libjvm.a is not created when building static-libs-graal-image and graal-builder-image targets. - Fix whitespace error in make/StaticJvmLibsImage.gmk. - - Separate building libjvm.a from static-libs-image target, based on input from @jerboaa and @olpaw: - Add a new java-static-libs-image target in Main.gmk for creating the JDK .a static libraries and libjvm.a super set. The static libraries are placed in images/static-libs/lib. The existing static-libs-image target is not affected and will not include hotspot libjvm.a. - Add java-static-libs-bundles target in Bundles.gmk. The created .tar.gz bundle contains JDK .a static libraries and hotspot libjvm.a. - Add StaticJvmLibsImage.gmk for placing libjvm.a into images/static-libs/lib. - Further cleanup after incorporating erikj79's suggestion to only build libjvm.a for $(JVM_VARIANT_MAIN) for now: - Change HOTSPOT_VARIANT_STATIC_LIBS_TARGETS to HOTSPOT_VARIANT_MAIN_STATIC_LIBS_TARGETS in Main.gmk. - Change hotspot-$v-static-libs to hotspot-$(JVM_VARIANT_MAIN)-static-libs in Main.gmk. - Update to create and use only hotspot-$(JVM_VARIANT_MAIN)-static-libs, based on @erikj79 input. - Update make/StaticLibsImage.gmk thanks Co-authored-by: Erik Joelsson <37597443+erik...@users.noreply.github.com> - Update make/StaticLibsImage.gmk Thanks Co-authored-by: Erik Joelsson <37597443+erik...@users.noreply.github.com> - Update based on @erikj79 review comments and suggestions: - Change to copy libjvm.a for $(JVM_VARIANT_MAIN) only in make/StaticLibsImage.gmk. - Use $(OBJ_SUFFIX) instead of .o in make/modules/java.base/lib/CoreLibraries.gmk. - ... and 2 more: https://git.openjdk.org/jdk/compare/84938550...36526d4f - Changes: - all: https://git.openjdk.org/jdk/pull/13768/files - new: https://git.openjdk.org/jdk/pull/13768/files/45dd2a00..36526d4f Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=13768=10 - incr: https://webrevs.openjdk.org/?repo=jdk=13768=09-10 Stats: 44134 lines in 1102 files changed: 31678 ins; 5168 del; 7288 mod Patch: https://git.openjdk.org/jdk/pull/13768.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/13768/head:pull/13768 PR: https://git.openjdk.org/jdk/pull/13768
Re: RFR: JDK-8151531: Add notes to BaseStream.spliterator/iterator docs regarding them being escape hatches [v3]
On Wed, 10 May 2023 07:48:11 GMT, Viktor Klang wrote: >> Still relevant to address? /cc @PaulSandoz > > Viktor Klang has updated the pull request incrementally with one additional > commit since the last revision: > > Updating copyright year Marked as reviewed by psandoz (Reviewer). - PR Review: https://git.openjdk.org/jdk/pull/13890#pullrequestreview-1420974155
Re: RFR: 8307194: Add make target for optionally building a complete set of all JDK and hotspot libjvm static libraries
On Wed, 3 May 2023 13:34:12 GMT, Erik Joelsson wrote: >> This PR is branched from the makefile changes for >> https://bugs.openjdk.org/browse/JDK-8303796 and contains the following for >> handling the JDK/hotspot static libraries: >> >> - Introduce new make target(s) for creating image/bundle containing hotspot >> libjvm.a and JDK static libraries >> >> - For libjvm.a specifically, exclude operator_new.o >> >> - Filter out "external" .o files (those are the .o files included from a >> different JDK library and needed when creating the .so shared library only) >> from JDK .a libraries; That's to avoid linker failures caused by duplicate >> symbols >> - For libjli.a: Not include inflate.o inftrees.o inffast.o zadler32.o >> zcrc32.o zutil.o (compiled from zlib sources) if zlib is built as JDK bundled >> - For libawt_xawt.a and libawt_head.a: Not include systemScale.o, since >> it's provided in libawt.a >> >> - Handle long arguments case for static build in >> make/common/NativeCompilation.gmk >> >> - Address @erikj79's comment in >> https://github.com/openjdk/jdk/pull/13709#discussion_r1180750185 for >> LIBJLI_STATIC_EXCLUDE_OBJS > > The current target user of the .a libraries is GraalVM, so we should check > with them that the changes to the contents of the `.a` files isn't impacting > them in a bad way. @dougxc what do you think? Thanks a lot for the multiple iterations of the discussions and reviews in this PR thread. All the input, especially https://github.com/openjdk/jdk/pull/13768#pullrequestreview-1415436469 from @erikj79 helped shaped this change. - PR Comment: https://git.openjdk.org/jdk/pull/13768#issuecomment-1542387929
Re: RFR: JDK-8077371: Binary files in JAXP test should be removed [v7]
> Test is updated to create the binary files during test execution. Mahendra Chhipa has updated the pull request incrementally with one additional commit since the last revision: Implemented the review comments. - Changes: - all: https://git.openjdk.org/jdk/pull/13537/files - new: https://git.openjdk.org/jdk/pull/13537/files/b690e77b..a940f237 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=13537=06 - incr: https://webrevs.openjdk.org/?repo=jdk=13537=05-06 Stats: 113 lines in 2 files changed: 36 ins; 10 del; 67 mod Patch: https://git.openjdk.org/jdk/pull/13537.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/13537/head:pull/13537 PR: https://git.openjdk.org/jdk/pull/13537
Re: RFR: JDK-8077371: Binary files in JAXP test should be removed [v6]
On Fri, 5 May 2023 21:50:06 GMT, Lance Andersen wrote: >> Mahendra Chhipa has updated the pull request incrementally with one >> additional commit since the last revision: >> >> Move the pseudo code generation part from setup() to seperate methods. > > test/jdk/javax/xml/jaxp/datatype/8033980/SerializationTest.java line 232: > >> 230: * JDKGregorianCalendarAndDurationSerData.java files. >> 231: * @param baos >> 232: */ > > I think there needs to be a general comment describing how these methods are > used to create the JDKGregorianCalendarAndDurationSerData.java > files. There should also be a description/comment for the methods defined in > GregorianCalendarAndDurationSerData.java > > We need to try and put ourselves in the place of a future maintainer who > needs to understand how to create a version of one of these files. > > You could probably also create a method which generates a > JDKGregorianCalendarAndDurationSerData.java file to save the > developer from multiple cut an pastes. > > At a minimum, there really should be a step by step guide How these methods can be used, added comment regarding this under setup method. Added description for the methods defined in GregorianCalendarAndDurationSerData.java file. Now developer should be able to create the GregorianCalendarAndDurationSerData.java file after reading these comments. - PR Review Comment: https://git.openjdk.org/jdk/pull/13537#discussion_r1190014838
Re: RFR: 8303040: linux PPC64le: Implementation of Foreign Function & Memory API (Preview) [v22]
On Wed, 10 May 2023 13:53:53 GMT, Stefan Karlsson wrote: >> I just figured it out. It was introduced by >> https://bugs.openjdk.org/browse/JDK-8203172 (on aarch64) which mentions >> Shenandoah and future GCs. However, the Shenandoah comment says >> "non-reference load, no additional barrier is needed" and it doesn't use >> barriers in such a case. So, for the time being, I'll keep the normal load >> (because `access_load_at` is not ready for non-oop types). But I should add >> the `NONZERO` check. > > FWIW, since Shenandoah changed their load barriers we have been cleaning away > the usages of the Access API for loads and stores to primitive values. > There's no such support in the C++ Runtime code. Ok, since this is loading a `long` (which represents an address that points into the code cache) I think we're fine without using the access API then? - PR Review Comment: https://git.openjdk.org/jdk/pull/12708#discussion_r1189996018
Re: RFR: 8303040: linux PPC64le: Implementation of Foreign Function & Memory API (Preview) [v24]
On Wed, 10 May 2023 11:16:44 GMT, Martin Doerr wrote: >> src/hotspot/cpu/ppc/upcallLinker_ppc.cpp line 202: >> >>> 200: >>> 201: MacroAssembler* _masm = new MacroAssembler(); >>> 202: address start = __ function_entry(); // called by C >> >> If `!defined(ABI_ELFv2)` a function descriptor will be emitted here. It will >> be initialized with `friend_toc` and `friend_env`. But that's not correct >> for external callers, is it? If so, wouldn't an `Unimplemented()` be better >> than obscure crashes? > > No, this code is correct and tested (I have a partially working Big Endian > patch). `toc` and `env` are loaded by the external caller (C code), but not > used by the stub. So, we don't need to initialize them to any specific values. I think I understand. The loaded `toc` and `env` of the stub are never used as Java execution does not use them and native or runtime calls will load corresponding `toc` and `env` of the callee. - PR Review Comment: https://git.openjdk.org/jdk/pull/12708#discussion_r1189980161
Re: RFR: 8303040: linux PPC64le: Implementation of Foreign Function & Memory API (Preview) [v30]
> Implementation of "Foreign Function & Memory API" for linux on Power (Little
> Endian) according to "Power Architecture 64-Bit ELF V2 ABI Specification".
>
> This PR does not include code for VaList support because it's supposed to get
> removed by [JDK-8299736](https://bugs.openjdk.org/browse/JDK-8299736). I've
> kept the related tests disabled for this platform and throw an exception
> instead. Note that the ABI doesn't precisely specify variable argument lists.
> Instead, it refers to `` (2.2.4 Variable Argument Lists).
>
> Big Endian support is implemented to some extend, but not complete. E.g.
> structs with size not divisible by 8 are not passed correctly (see `useABIv2`
> in CallArranger.java). Big Endian is excluded by selecting
> `ARCH.equals("ppc64le")` (CABI.java) only.
>
> There's another limitation: This PR only accepts structures with size
> divisible by 4. (An `IllegalArgumentException` gets thrown otherwise.) I
> think arbitrary sizes are not usable on other platforms, either, because
> `SharedUtils.primitiveCarrierForSize` only accepts powers of 2. Update:
> Resolved after merging of
> [JDK-8303017](https://bugs.openjdk.org/browse/JDK-8303017)
>
> The ABI has some tricky corner cases related to HFA (Homogeneous Float
> Aggregate). The same argument may need to get passed in both, a FP reg and a
> GP reg or stack slot (see "no partial DW rule"). This cases are not covered
> by the existing tests.
>
> I had to make changes to shared code and code for other platforms:
> 1. Pass type information when creating `VMStorage` objects from `VMReg`. This
> is needed for the following reasons:
> - PPC64 ABI requires integer types to get extended to 64 bit (also see
> CCallingConventionRequiresIntsAsLongs in existing hotspot code). We need to
> know the type or at least the bit width for that.
> - Floating point load / store instructions need the correct width to select
> between the correct IEEE 754 formats. The register representation in single
> FP registers is always IEEE 754 double precision on PPC64.
> - Big Endian also needs usage of the precise size. Storing 8 Bytes and
> loading 4 Bytes yields different values than on Little Endian!
> 2. It happens that a `NativeMemorySegmentImpl` is used as a raw pointer (with
> byteSize() == 0) while running TestUpcallScope. Hence, existing size checks
> don't work (see MemorySegment.java). As a workaround, I'm just skipping the
> check in this particular case. Please check if this makes sense or if there's
> a better fix (possibly as separate RFE). Update: This issue is resolved by
> 2nd commit.
Martin Doerr has updated the pull request incrementally with one additional
commit since the last revision:
Add NONZERO check for downcall_stub_address_offset_in_bytes().
-
Changes:
- all: https://git.openjdk.org/jdk/pull/12708/files
- new: https://git.openjdk.org/jdk/pull/12708/files/93060258..edcdefba
Webrevs:
- full: https://webrevs.openjdk.org/?repo=jdk=12708=29
- incr: https://webrevs.openjdk.org/?repo=jdk=12708=28-29
Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod
Patch: https://git.openjdk.org/jdk/pull/12708.diff
Fetch: git fetch https://git.openjdk.org/jdk.git pull/12708/head:pull/12708
PR: https://git.openjdk.org/jdk/pull/12708
Re: RFR: 8305341: Alignment should be enforced by alignas instead of compiler specific attributes [v4]
On Wed, 12 Apr 2023 07:12:10 GMT, Julian Waters wrote: >> C11 has been stable for a long time on all platforms, so native code can use >> the standard alignas operator for alignment requirements > > Julian Waters has updated the pull request incrementally with four additional > commits since the last revision: > > - Restore visCPP > - Restore gcc attribute > - Revert gcc > - Revert Well, that's unfortunate - PR Comment: https://git.openjdk.org/jdk/pull/13258#issuecomment-1542279520
Re: RFR: 8303040: linux PPC64le: Implementation of Foreign Function & Memory API (Preview) [v22]
On Wed, 10 May 2023 13:43:41 GMT, Martin Doerr wrote: >> You are reasoning about implementation details. By using the provided >> abstraction you and other maintainers (who might be unfamiliar with them) >> would not have to do that. Also the assumptions you make introduce a hidden >> dependency. > > I just figured it out. It was introduced by > https://bugs.openjdk.org/browse/JDK-8203172 (on aarch64) which mentions > Shenandoah and future GCs. However, the Shenandoah comment says > "non-reference load, no additional barrier is needed" and it doesn't use > barriers in such a case. So, for the time being, I'll keep the normal load > (because `access_load_at` is not ready for non-oop types). But I should add > the `NONZERO` check. FWIW, since Shenandoah changed their load barriers we have been cleaning away the usages of the Access API for loads and stores to primitive values. There's no such support in the C++ Runtime code. - PR Review Comment: https://git.openjdk.org/jdk/pull/12708#discussion_r1189948988
Re: RFR: 8303040: linux PPC64le: Implementation of Foreign Function & Memory API (Preview) [v22]
On Wed, 10 May 2023 13:33:02 GMT, Richard Reingruber wrote: >> GC barriers are used when loading or storing an oop. No GC we currently have >> (not even the generational ones) use barriers for loading a plain address >> from an oop. The PPC64 implementation of the BarrierSetAssembler currently >> has `Unimplemented()` for non-oop types and all GCs are implemented. >> Maybe it was intended for some future GC or other feature which has not yet >> reached the official repo. > > You are reasoning about implementation details. By using the provided > abstraction you and other maintainers (who might be unfamiliar with them) > would not have to do that. Also the assumptions you make introduce a hidden > dependency. I just figured it out. It was introduced by https://bugs.openjdk.org/browse/JDK-8203172 (on aarch64) which mentions Shenandoah and future GCs. However, the Shenandoah comment says "non-reference load, no additional barrier is needed" and it doesn't use barriers in such a case. So, for the time being, I'll keep the normal load (because `access_load_at` is not ready for non-oop types). But I should add the `NONZERO` check. - PR Review Comment: https://git.openjdk.org/jdk/pull/12708#discussion_r1189934352
Re: RFR: 8303040: linux PPC64le: Implementation of Foreign Function & Memory API (Preview) [v22]
On Wed, 10 May 2023 13:24:55 GMT, Martin Doerr wrote: >> As I see it, the access API is an abstraction to be used instead of raw >> loads. It hides details. See for instance >> `TemplateTable::getfield_or_static` on x86 where it is also used. PPC lags >> behind in making use of the access API. >> With a fancy new GC the oop in nep_reg could be stale, requiring some >> processing which would be taken care of by using the access API. > > GC barriers are used when loading or storing an oop. No GC we currently have > (not even the generational ones) use barriers for loading a plain address > from an oop. The PPC64 implementation of the BarrierSetAssembler currently > has `Unimplemented()` for non-oop types and all GCs are implemented. > Maybe it was intended for some future GC or other feature which has not yet > reached the official repo. You are reasoning about implementation details. By using the provided abstraction you and other maintainers (who might be unfamiliar with them) would not have to do that. Also the assumptions you make introduce a hidden dependency. - PR Review Comment: https://git.openjdk.org/jdk/pull/12708#discussion_r1189919874
Re: RFR: 8303040: linux PPC64le: Implementation of Foreign Function & Memory API (Preview) [v22]
On Wed, 10 May 2023 13:22:48 GMT, Richard Reingruber wrote: >>> It does the same but with a more complicated API. >> >> AFAIK It depends on the GC that's being used. `access_load_at` will make >> sure the right GC barriers are inserted (mostly for concurrent GCs). > > As I see it, the access API is an abstraction to be used instead of raw > loads. It hides details. See for instance `TemplateTable::getfield_or_static` > on x86 where it is also used. PPC lags behind in making use of the access API. > With a fancy new GC the oop in nep_reg could be stale, requiring some > processing which would be taken care of by using the access API. GC barriers are used when loading or storing an oop. No GC we currently have (not even the generational ones) use barriers for loading a plain address from an oop. The PPC64 implementation of the BarrierSetAssembler currently has `Unimplemented()` for non-oop types and all GCs are implemented. Maybe it was intended for some future GC or other feature which has not yet reached the official repo. - PR Review Comment: https://git.openjdk.org/jdk/pull/12708#discussion_r1189908142
Re: RFR: 8303040: linux PPC64le: Implementation of Foreign Function & Memory API (Preview) [v22]
On Wed, 10 May 2023 13:07:35 GMT, Jorn Vernee wrote: >> Interesting. I have no idea why. It does the same but with a more >> complicated API. >> I just noticed that other platforms use `NONZERO`. I think I should at least >> add that. > >> It does the same but with a more complicated API. > > AFAIK It depends on the GC that's being used. `access_load_at` will make sure > the right GC barriers are inserted (mostly for concurrent GCs). As I see it, the access API is an abstraction to be used instead of raw loads. It hides details. See for instance `TemplateTable::getfield_or_static` on x86 where it is also used. PPC lags behind in making use of the access API. With a fancy new GC the oop in nep_reg could be stale, requiring some processing which would be taken care of by using the access API. - PR Review Comment: https://git.openjdk.org/jdk/pull/12708#discussion_r1189905194
Re: RFR: 8303040: linux PPC64le: Implementation of Foreign Function & Memory API (Preview) [v24]
On Wed, 10 May 2023 11:23:04 GMT, Martin Doerr wrote:
>> src/hotspot/cpu/ppc/upcallLinker_ppc.cpp line 236:
>>
>>> 234: __ block_comment("{ receiver ");
>>> 235: __ load_const_optimized(R3_ARG1, (intptr_t)receiver, R0);
>>> 236: __ resolve_jobject(R3_ARG1, tmp, R31,
>>> MacroAssembler::PRESERVATION_FRAME_LR_GP_FP_REGS); // kills R31
>>
>> As a simplification the receiver could be resolved in
>> `UpcallLinker::on_entry` and returned in `JavaThread::_vm_result`.
>
> This sounds like a nice enhancement proposal for all platforms. The register
> spilling code in `resolve_jobject` can get lengthy dependent on the selected
> GC. Doing it in the C code (which we call anyway above) would make the upcall
> stubs smaller.
> @JornVernee: What do you think about this idea?
Seems like a nice idea. The resolution here pre-dates the time where we called
into the VM.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/12708#discussion_r1189892999
Re: RFR: 8303040: linux PPC64le: Implementation of Foreign Function & Memory API (Preview) [v22]
On Wed, 10 May 2023 11:13:14 GMT, Martin Doerr wrote: > It does the same but with a more complicated API. AFAIK It depends on the GC that's being used. `access_load_at` will make sure the right GC barriers are inserted (mostly for concurrent GCs). - PR Review Comment: https://git.openjdk.org/jdk/pull/12708#discussion_r1189885698
Re: RFR: 8307194: Add make target for optionally building a complete set of all JDK and hotspot libjvm static libraries [v10]
On Tue, 9 May 2023 23:06:23 GMT, Jiangli Zhou wrote: >> This PR is branched from the makefile changes for >> https://bugs.openjdk.org/browse/JDK-8303796 and contains the following for >> handling the JDK/hotspot static libraries: >> >> - Introduce new make target(s) for creating image/bundle containing hotspot >> libjvm.a and JDK static libraries >> >> - For libjvm.a specifically, exclude operator_new.o >> >> - Filter out "external" .o files (those are the .o files included from a >> different JDK library and needed when creating the .so shared library only) >> from JDK .a libraries; That's to avoid linker failures caused by duplicate >> symbols >> - For libjli.a: Not include inflate.o inftrees.o inffast.o zadler32.o >> zcrc32.o zutil.o (compiled from zlib sources) if zlib is built as JDK bundled >> - For libawt_xawt.a and libawt_head.a: Not include systemScale.o, since >> it's provided in libawt.a >> >> - Handle long arguments case for static build in >> make/common/NativeCompilation.gmk >> >> - Address @erikj79's comment in >> https://github.com/openjdk/jdk/pull/13709#discussion_r1180750185 for >> LIBJLI_STATIC_EXCLUDE_OBJS > > Jiangli Zhou has updated the pull request incrementally with one additional > commit since the last revision: > > Address comments from @erikj79: > - Fix to use $(STATIC_LIBS_GRAAL_IMAGE_DIR) in GraalBuilderImage.gmk. > - Split the long line at 1281 in Main.gmk. Marked as reviewed by erikj (Reviewer). - PR Review: https://git.openjdk.org/jdk/pull/13768#pullrequestreview-1420562567
Re: RFR: 8303040: linux PPC64le: Implementation of Foreign Function & Memory API (Preview) [v28]
On Tue, 9 May 2023 15:48:52 GMT, Richard Reingruber wrote: >> Martin Doerr has updated the pull request incrementally with one additional >> commit since the last revision: >> >> libTestHFA: Add explicit type conversion to avoid build warning. > > src/hotspot/cpu/ppc/vmstorage_ppc.hpp line 81: > >> 79: case T_BYTE : >> 80: case T_SHORT : >> 81: case T_INT: segment_mask = REG32_MASK; break; > > I wonder why the segment_mask depends on `bt` on ppc? The usage of the `segment_mask` can be defined for each platform. I'm using it to encode the information if a value on the Java side uses a 32 or 64 bit slot. In case of 32 bit values, the C side requires all 64 register bits to get defined values (ints get sign extended, floats get converted to double-precision format). - PR Review Comment: https://git.openjdk.org/jdk/pull/12708#discussion_r1189768204
Re: RFR: 8303040: linux PPC64le: Implementation of Foreign Function & Memory API (Preview) [v24]
On Fri, 28 Apr 2023 13:18:27 GMT, Richard Reingruber wrote:
>> Martin Doerr has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Revert unintended formatting changes. Fix comment.
>
> src/hotspot/cpu/ppc/upcallLinker_ppc.cpp line 236:
>
>> 234: __ block_comment("{ receiver ");
>> 235: __ load_const_optimized(R3_ARG1, (intptr_t)receiver, R0);
>> 236: __ resolve_jobject(R3_ARG1, tmp, R31,
>> MacroAssembler::PRESERVATION_FRAME_LR_GP_FP_REGS); // kills R31
>
> As a simplification the receiver could be resolved in
> `UpcallLinker::on_entry` and returned in `JavaThread::_vm_result`.
This sounds like a nice enhancement proposal for all platforms. The register
spilling code in `resolve_jobject` can get lengthy dependent on the selected
GC. Doing it in the C code (which we call anyway above) would make the upcall
stubs smaller.
@JornVernee: What do you think about this idea?
-
PR Review Comment: https://git.openjdk.org/jdk/pull/12708#discussion_r1189763910
Re: RFR: 8303040: linux PPC64le: Implementation of Foreign Function & Memory API (Preview) [v24]
On Thu, 27 Apr 2023 16:19:46 GMT, Richard Reingruber wrote: >> Martin Doerr has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Revert unintended formatting changes. Fix comment. > > src/hotspot/cpu/ppc/upcallLinker_ppc.cpp line 202: > >> 200: >> 201: MacroAssembler* _masm = new MacroAssembler(); >> 202: address start = __ function_entry(); // called by C > > If `!defined(ABI_ELFv2)` a function descriptor will be emitted here. It will > be initialized with `friend_toc` and `friend_env`. But that's not correct for > external callers, is it? If so, wouldn't an `Unimplemented()` be better than > obscure crashes? No, this code is correct and tested (I have a partially working Big Endian patch). `toc` and `env` are loaded by the external caller (C code), but not used by the stub. So, we don't need to initialize them to any specific values. - PR Review Comment: https://git.openjdk.org/jdk/pull/12708#discussion_r1189755698
Re: RFR: 8303040: linux PPC64le: Implementation of Foreign Function & Memory API (Preview) [v22]
On Wed, 26 Apr 2023 14:41:51 GMT, Richard Reingruber wrote: >> Martin Doerr has updated the pull request with a new target base due to a >> merge or a rebase. The pull request now contains 24 commits: >> >> - Adaptation for JDK-8305668 >> - Merge remote-tracking branch 'origin' into PPC64_Panama >> - Move ABIv2CallArranger out of linux subdirectory. ABIv1/2 does match the >> AIX/linux separation. >> - Adaptation for JDK-8303022. >> - Adaptation for JDK-8303684. >> - Merge branch 'openjdk:master' into PPC64_Panama >> - Merge branch 'master' into PPC64_Panama >> - Fix Copyright format. >> - Fix storing 32 bit integers into Java frames. Enable TestArrayStructs. >> - Allow TestHFA to run on musl. Add Upcalls. >> - ... and 14 more: https://git.openjdk.org/jdk/compare/3bba8995...725732a0 > > src/hotspot/cpu/ppc/methodHandles_ppc.cpp line 316: > >> 314: // Load the invoker, as NEP -> .invoker >> 315: __ verify_oop(nep_reg); >> 316: __ ld(temp_target, >> jdk_internal_foreign_abi_NativeEntryPoint::downcall_stub_address_offset_in_bytes(), >> nep_reg); > > Other platforms use `access_load_at`. Interesting. I have no idea why. It does the same but with a more complicated API. I just noticed that other platforms use `NONZERO`. I think I should at least add that. - PR Review Comment: https://git.openjdk.org/jdk/pull/12708#discussion_r1189752212
Re: RFR: 8303040: linux PPC64le: Implementation of Foreign Function & Memory API (Preview) [v22]
On Wed, 26 Apr 2023 14:32:59 GMT, Richard Reingruber wrote: >> Martin Doerr has updated the pull request with a new target base due to a >> merge or a rebase. The pull request now contains 24 commits: >> >> - Adaptation for JDK-8305668 >> - Merge remote-tracking branch 'origin' into PPC64_Panama >> - Move ABIv2CallArranger out of linux subdirectory. ABIv1/2 does match the >> AIX/linux separation. >> - Adaptation for JDK-8303022. >> - Adaptation for JDK-8303684. >> - Merge branch 'openjdk:master' into PPC64_Panama >> - Merge branch 'master' into PPC64_Panama >> - Fix Copyright format. >> - Fix storing 32 bit integers into Java frames. Enable TestArrayStructs. >> - Allow TestHFA to run on musl. Add Upcalls. >> - ... and 14 more: https://git.openjdk.org/jdk/compare/3bba8995...725732a0 > > src/hotspot/cpu/ppc/frame_ppc.cpp line 219: > >> 217: UpcallStub* blob = _cb->as_upcall_stub(); >> 218: JavaFrameAnchor* jfa = blob->jfa_for_frame(*this); >> 219: return jfa->last_Java_sp() == NULL; > > Suggestion: > > return jfa->last_Java_sp() == nullptr; > > I'd suggest to do the same for all occurrences in the patch. Good catch! I've replaced them. - PR Review Comment: https://git.openjdk.org/jdk/pull/12708#discussion_r1189742225
Re: RFR: 8303040: linux PPC64le: Implementation of Foreign Function & Memory API (Preview) [v21]
On Mon, 27 Mar 2023 16:54:31 GMT, Richard Reingruber wrote:
>> Martin Doerr has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Move ABIv2CallArranger out of linux subdirectory. ABIv1/2 does match the
>> AIX/linux separation.
>
> src/hotspot/cpu/ppc/downcallLinker_ppc.cpp line 185:
>
>> 183:
>> 184: allocated_frame_size = align_up(allocated_frame_size,
>> StackAlignmentInBytes);
>> 185: _frame_size_slots = allocated_frame_size >> LogBytesPerInt;
>
> `VMRegImpl::stack_slot_size` could be used when converting from size in bytes
> to size in slots.
Yes, I think this would be better readable. But the following code should also
be adapted, then:
int framesize() const {
return (_frame_size_slots >> (LogBytesPerWord - LogBytesPerInt));
}
Maybe it makes sense to do some cleanup for all platforms.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/12708#discussion_r1189741783
Re: RFR: 8303040: linux PPC64le: Implementation of Foreign Function & Memory API (Preview) [v29]
> Implementation of "Foreign Function & Memory API" for linux on Power (Little
> Endian) according to "Power Architecture 64-Bit ELF V2 ABI Specification".
>
> This PR does not include code for VaList support because it's supposed to get
> removed by [JDK-8299736](https://bugs.openjdk.org/browse/JDK-8299736). I've
> kept the related tests disabled for this platform and throw an exception
> instead. Note that the ABI doesn't precisely specify variable argument lists.
> Instead, it refers to `` (2.2.4 Variable Argument Lists).
>
> Big Endian support is implemented to some extend, but not complete. E.g.
> structs with size not divisible by 8 are not passed correctly (see `useABIv2`
> in CallArranger.java). Big Endian is excluded by selecting
> `ARCH.equals("ppc64le")` (CABI.java) only.
>
> There's another limitation: This PR only accepts structures with size
> divisible by 4. (An `IllegalArgumentException` gets thrown otherwise.) I
> think arbitrary sizes are not usable on other platforms, either, because
> `SharedUtils.primitiveCarrierForSize` only accepts powers of 2. Update:
> Resolved after merging of
> [JDK-8303017](https://bugs.openjdk.org/browse/JDK-8303017)
>
> The ABI has some tricky corner cases related to HFA (Homogeneous Float
> Aggregate). The same argument may need to get passed in both, a FP reg and a
> GP reg or stack slot (see "no partial DW rule"). This cases are not covered
> by the existing tests.
>
> I had to make changes to shared code and code for other platforms:
> 1. Pass type information when creating `VMStorage` objects from `VMReg`. This
> is needed for the following reasons:
> - PPC64 ABI requires integer types to get extended to 64 bit (also see
> CCallingConventionRequiresIntsAsLongs in existing hotspot code). We need to
> know the type or at least the bit width for that.
> - Floating point load / store instructions need the correct width to select
> between the correct IEEE 754 formats. The register representation in single
> FP registers is always IEEE 754 double precision on PPC64.
> - Big Endian also needs usage of the precise size. Storing 8 Bytes and
> loading 4 Bytes yields different values than on Little Endian!
> 2. It happens that a `NativeMemorySegmentImpl` is used as a raw pointer (with
> byteSize() == 0) while running TestUpcallScope. Hence, existing size checks
> don't work (see MemorySegment.java). As a workaround, I'm just skipping the
> check in this particular case. Please check if this makes sense or if there's
> a better fix (possibly as separate RFE). Update: This issue is resolved by
> 2nd commit.
Martin Doerr has updated the pull request incrementally with one additional
commit since the last revision:
Replace NULL by nullptr.
-
Changes:
- all: https://git.openjdk.org/jdk/pull/12708/files
- new: https://git.openjdk.org/jdk/pull/12708/files/74586ab8..93060258
Webrevs:
- full: https://webrevs.openjdk.org/?repo=jdk=12708=28
- incr: https://webrevs.openjdk.org/?repo=jdk=12708=27-28
Stats: 3 lines in 2 files changed: 0 ins; 0 del; 3 mod
Patch: https://git.openjdk.org/jdk/pull/12708.diff
Fetch: git fetch https://git.openjdk.org/jdk.git pull/12708/head:pull/12708
PR: https://git.openjdk.org/jdk/pull/12708
Re: RFR: 8306842: Classfile API performance improvements [v4]
On Wed, 10 May 2023 09:55:13 GMT, Adam Sotona wrote: >> That seems misplaced. Please file an RFE to have this cleaned up. >> >> Each microbenchmark that has to add opens needs to take responsibility for >> that themselves and not change the environment for everything else. And all >> micros the benchmarks.jar should be runnable standalone, not rely on quirks >> in the make runner. > > All benchmarks fixed and --add-exports removed from make file. > Thank you for pointing this out. Thanks for fixing this! - PR Review Comment: https://git.openjdk.org/jdk/pull/13671#discussion_r1189692833
Re: RFR: 8306842: Classfile API performance improvements [v4]
> Following improvements implemented: > - Switch over `String` replaced with switch single char > - Binary search for frames in `StackMapGenerator` > - `StackMapGenerator.rawHandlers` with pre-calculated offsets > - `ClassEntry` is caching `ClassDesc` symbol > - Caching of type symbols in `NameAndTypeEntry` and `MethodTypeEntry` > - Caching `MethodTypeDesc` in `MethodInfo` implementations > - `StackMapGenerator` and `Utils` delegating to cached `MethodTypeDesc` > instead of custom parsing > > No API change. > > Benchmarks show stack map generation improved by 21% and code generation from > symbols improved by 30%. > > > Benchmark Mode Cnt Score Error Units > GenerateStackMaps.benchmark thrpt 10 407931.202 ± 13101.023 ops/s > RebuildMethodBodies.shared thrpt4 10258.597 ± 383.699 ops/s > RebuildMethodBodies.unshared thrpt47224.543 ± 256.800 ops/s > > > > Benchmark Mode Cnt Score Error Units > GenerateStackMaps.benchmark thrpt 10 495101.110 ± 2389.628 ops/s > RebuildMethodBodies.sharedthrpt 4 13380.272 ± 810.113 ops/s > RebuildMethodBodies.unshared thrpt 49399.863 ± 557.060 ops/s Adam Sotona has updated the pull request incrementally with one additional commit since the last revision: fixed jmh benchmark parameters - Changes: - all: https://git.openjdk.org/jdk/pull/13671/files - new: https://git.openjdk.org/jdk/pull/13671/files/5af9f9c4..3cbb11c2 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=13671=03 - incr: https://webrevs.openjdk.org/?repo=jdk=13671=02-03 Stats: 46 lines in 5 files changed: 32 ins; 9 del; 5 mod Patch: https://git.openjdk.org/jdk/pull/13671.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/13671/head:pull/13671 PR: https://git.openjdk.org/jdk/pull/13671
Re: RFR: 8306842: Classfile API performance improvements [v4]
On Tue, 9 May 2023 16:15:27 GMT, Claes Redestad wrote: >> They are added in the `make/RunTests.gmk`: >> https://github.com/openjdk/jdk/pull/13550/files#diff-041bf69ea79b333b9ce99c1f879e398d698538530a35c361500b72631f059233R599-R608 > > That seems misplaced. Please file an RFE to have this cleaned up. > > Each microbenchmark that has to add opens needs to take responsibility for > that themselves and not change the environment for everything else. And all > micros the benchmarks.jar should be runnable standalone, not rely on quirks > in the make runner. All benchmarks fixed and --add-exports removed from make file. Thank you for pointing this out. - PR Review Comment: https://git.openjdk.org/jdk/pull/13671#discussion_r1189664602
Integrated: 8302845: Replace finalizer usage in JNDI DNS provider with Cleaner
On Fri, 5 May 2023 17:25:38 GMT, Aleksei Efimov wrote: > JNDI `DnsClient` has a finalize method to close its internal datagram channel > selector. > The change proposed here replaces it with a cleaner to close the selector > once the `DnsClient` > instance becomes phantom reachable. > > The change was tested with `jdk-tier1` to `jdk-tier3` test sets which showed > no failures. This pull request has now been integrated. Changeset: da2c9302 Author:Aleksei Efimov URL: https://git.openjdk.org/jdk/commit/da2c9302628886bbdb4cf588f8275c4a44ff5ac9 Stats: 31 lines in 3 files changed: 14 ins; 13 del; 4 mod 8302845: Replace finalizer usage in JNDI DNS provider with Cleaner Reviewed-by: alanb, dfuchs, djelinski - PR: https://git.openjdk.org/jdk/pull/13837
Re: RFR: 8307194: Add make target for optionally building a complete set of all JDK and hotspot libjvm static libraries [v10]
On Tue, 9 May 2023 23:06:23 GMT, Jiangli Zhou wrote: >> This PR is branched from the makefile changes for >> https://bugs.openjdk.org/browse/JDK-8303796 and contains the following for >> handling the JDK/hotspot static libraries: >> >> - Introduce new make target(s) for creating image/bundle containing hotspot >> libjvm.a and JDK static libraries >> >> - For libjvm.a specifically, exclude operator_new.o >> >> - Filter out "external" .o files (those are the .o files included from a >> different JDK library and needed when creating the .so shared library only) >> from JDK .a libraries; That's to avoid linker failures caused by duplicate >> symbols >> - For libjli.a: Not include inflate.o inftrees.o inffast.o zadler32.o >> zcrc32.o zutil.o (compiled from zlib sources) if zlib is built as JDK bundled >> - For libawt_xawt.a and libawt_head.a: Not include systemScale.o, since >> it's provided in libawt.a >> >> - Handle long arguments case for static build in >> make/common/NativeCompilation.gmk >> >> - Address @erikj79's comment in >> https://github.com/openjdk/jdk/pull/13709#discussion_r1180750185 for >> LIBJLI_STATIC_EXCLUDE_OBJS > > Jiangli Zhou has updated the pull request incrementally with one additional > commit since the last revision: > > Address comments from @erikj79: > - Fix to use $(STATIC_LIBS_GRAAL_IMAGE_DIR) in GraalBuilderImage.gmk. > - Split the long line at 1281 in Main.gmk. Looks fine to me. - Marked as reviewed by sgehwolf (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/13768#pullrequestreview-1420136948
Re: RFR: 8307610: Linker::nativeLinker should not be restricted (mainline) [v3]
> Port of: https://git.openjdk.org/panama-foreign/pull/831 Maurizio Cimadamore has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains four commits: - Merge branch 'master' into linker_restricted - Address review comment - Cleanup code so that restricted method check is shared - Initial push - Changes: https://git.openjdk.org/jdk/pull/13863/files Webrev: https://webrevs.openjdk.org/?repo=jdk=13863=02 Stats: 128 lines in 11 files changed: 65 ins; 17 del; 46 mod Patch: https://git.openjdk.org/jdk/pull/13863.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/13863/head:pull/13863 PR: https://git.openjdk.org/jdk/pull/13863
Re: RFR: JDK-8151531: Add notes to BaseStream.spliterator/iterator docs regarding them being escape hatches [v3]
> Still relevant to address? /cc @PaulSandoz Viktor Klang has updated the pull request incrementally with one additional commit since the last revision: Updating copyright year - Changes: - all: https://git.openjdk.org/jdk/pull/13890/files - new: https://git.openjdk.org/jdk/pull/13890/files/4739a781..3f1cb743 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=13890=02 - incr: https://webrevs.openjdk.org/?repo=jdk=13890=01-02 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.org/jdk/pull/13890.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/13890/head:pull/13890 PR: https://git.openjdk.org/jdk/pull/13890
Re: RFR: JDK-8151531: Add notes to BaseStream.spliterator/iterator docs regarding them being escape hatches [v2]
On Tue, 9 May 2023 16:28:52 GMT, Paul Sandoz wrote: >> Viktor Klang has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Using apiNote and rewording from method to operation > > Marked as reviewed by psandoz (Reviewer). @PaulSandoz Forgot to update copyright year so added that as well - PR Comment: https://git.openjdk.org/jdk/pull/13890#issuecomment-1541507232
