Re: RFR: 8289768: Clean up unused code [v2]
> This patch removes many unused variables and one unused label reported by the > compilers when relevant warnings are enabled. > > The unused code was found by compiling after removing `unused` from the list > of disabled warnings for > [gcc](https://github.com/openjdk/jdk/blob/master/make/autoconf/flags-cflags.m4#L190) > and > [clang](https://github.com/openjdk/jdk/blob/master/make/autoconf/flags-cflags.m4#L203), > and enabling > [C4189](https://docs.microsoft.com/en-us/cpp/error-messages/compiler-warnings/compiler-warning-level-4-c4189?view=msvc-170) > MSVC warning. > > I only removed variables that were uninitialized or initialized without side > effects. I verified that the removed variables were not used in any > `#ifdef`'d code. I checked that the changed code still compiles on Windows, > Linux and Mac, both in release and debug versions. Daniel Jeliński 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 four additional commits since the last revision: - Merge remote-tracking branch 'origin' into unused-variables - Remove unused variables (windows) - Remove unused variables (macos) - Remove unused variables (linux) - Changes: - all: https://git.openjdk.org/jdk/pull/9383/files - new: https://git.openjdk.org/jdk/pull/9383/files/915680c0..b4cd5374 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=9383&range=01 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=9383&range=00-01 Stats: 491 lines in 22 files changed: 458 ins; 5 del; 28 mod Patch: https://git.openjdk.org/jdk/pull/9383.diff Fetch: git fetch https://git.openjdk.org/jdk pull/9383/head:pull/9383 PR: https://git.openjdk.org/jdk/pull/9383
Re: [jdk19] RFR: 8287809: Revisit implementation of memory session [v5]
> This is a JDK 19 clone of: https://github.com/openjdk/jdk/pull/9017 Maurizio Cimadamore has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains ten commits: - Merge branch 'master' into memory_session_cleanup - Fix ambiguity between session vs. base session - Revert implicit vs. heap session changes - Unify heap vs. implicit scopes - Merge branch 'master' into memory_session_cleanup - Fix issue in Direct-X-Buffer template - Simplify and drop the state class - Add missing files - Initial push - Changes: https://git.openjdk.org/jdk19/pull/22/files Webrev: https://webrevs.openjdk.org/?repo=jdk19&pr=22&range=04 Stats: 484 lines in 24 files changed: 56 ins; 107 del; 321 mod Patch: https://git.openjdk.org/jdk19/pull/22.diff Fetch: git fetch https://git.openjdk.org/jdk19 pull/22/head:pull/22 PR: https://git.openjdk.org/jdk19/pull/22
Re: RFR: JDK-8266431: Dual-Pivot Quicksort improvements (Radix sort) [v14]
On Thu, 30 Jun 2022 16:41:36 GMT, iaroslavski wrote: >> Sorting: >> >> - adopt radix sort for sequential and parallel sorts on >> int/long/float/double arrays (almost random and length > 6K) >> - fix tryMergeRuns() to better handle case when the last run is a single >> element >> - minor javadoc and comment changes >> >> Testing: >> - add new data inputs in tests for sorting >> - add min/max/infinity values to float/double testing >> - add tests for radix sort > > iaroslavski has updated the pull request incrementally with one additional > commit since the last revision: > > JDK-8266431: Dual-Pivot Quicksort improvements (Radix sort) > > * Fix @since version I mean adding benchmarks into `micro.org.openjdk.bench` - PR: https://git.openjdk.org/jdk/pull/3938
Re: RFR: 8289768: Clean up unused code [v2]
On Wed, 6 Jul 2022 05:32:29 GMT, Daniel Jeliński wrote: >> This patch removes many unused variables and one unused label reported by >> the compilers when relevant warnings are enabled. >> >> The unused code was found by compiling after removing `unused` from the list >> of disabled warnings for >> [gcc](https://github.com/openjdk/jdk/blob/master/make/autoconf/flags-cflags.m4#L190) >> and >> [clang](https://github.com/openjdk/jdk/blob/master/make/autoconf/flags-cflags.m4#L203), >> and enabling >> [C4189](https://docs.microsoft.com/en-us/cpp/error-messages/compiler-warnings/compiler-warning-level-4-c4189?view=msvc-170) >> MSVC warning. >> >> I only removed variables that were uninitialized or initialized without side >> effects. I verified that the removed variables were not used in any >> `#ifdef`'d code. I checked that the changed code still compiles on Windows, >> Linux and Mac, both in release and debug versions. > > Daniel Jeliński 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 four additional > commits since the last revision: > > - Merge remote-tracking branch 'origin' into unused-variables > - Remove unused variables (windows) > - Remove unused variables (macos) > - Remove unused variables (linux) test failure is unrelated, caused by [JDK-8289619](https://bugs.openjdk.org/browse/JDK-8289619) - PR: https://git.openjdk.org/jdk/pull/9383
RFR: 8289834: Add SBCS and DBCS Only EBCDIC charsets
OpenJDK supports "Japanese EBCDIC - Katakana" and "Korean EBCDIC" SBCS and DBCS Only charsets. |Charset|Mix|SBCS|DBCS| | -- | -- | -- | -- | | Japanese EBCDIC - Katakana | Cp930 | Cp290 | Cp300 | | Korean | Cp933 | Cp833 | Cp834 | But OpenJDK does not supports some of "Japanese EBCDIC - English" / "Simplified Chinese EBCDIC" / "Traditional Chinese EBCDIC" SBCS and DBCS Only charsets. I'd like to request Cp1027/Cp835/Cp836/Cp837 for consistency |Charset|Mix|SBCS|DBCS| | - | - | - | - | | Japanese EBCDIC - English | Cp939 | **Cp1027** | Cp300 | | Simplified Chinese EBCDIC | Cp935 | **Cp836** | **Cp837** | | Traditional Chinese EBCDIC | Cp937 | (*1) | **Cp835** | *1: Cp037 compatible - Commit messages: - 8289834: Missing SBCS and DBCS Only EBCDIC charsets Changes: https://git.openjdk.org/jdk/pull/9399/files Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=9399&range=00 Issue: https://bugs.openjdk.org/browse/JDK-8289834 Stats: 369 lines in 6 files changed: 367 ins; 0 del; 2 mod Patch: https://git.openjdk.org/jdk/pull/9399.diff Fetch: git fetch https://git.openjdk.org/jdk pull/9399/head:pull/9399 PR: https://git.openjdk.org/jdk/pull/9399
Re: RFR: 8279283 - BufferedInputStream should override transferTo [v5]
On Mon, 27 Dec 2021 13:43:12 GMT, Markus KARG wrote: >> Implementation of JDK-8279283 > > Markus KARG has updated the pull request incrementally with one additional > commit since the last revision: > > fixed missing BufferedInputStream Please keep this PR open; I will continune work on it soon. - PR: https://git.openjdk.org/jdk/pull/6935
Re: [jdk19] RFR: 8287809: Revisit implementation of memory session [v4]
On Fri, 17 Jun 2022 18:39:03 GMT, Maurizio Cimadamore wrote: >> This is a JDK 19 clone of: https://github.com/openjdk/jdk/pull/9017 > > Maurizio Cimadamore has updated the pull request incrementally with one > additional commit since the last revision: > > Revert implicit vs. heap session changes src/java.base/share/classes/java/lang/invoke/X-VarHandleSegmentView.java.template line 131: > 129: AbstractMemorySegmentImpl bb = checkAddress(obb, base, > handle.length, true); > 130: #if[floatingPoint] > 131: $rawType$ rawValue = > SCOPED_MEMORY_ACCESS.get$RawType$Unaligned(bb.session(), For instance, it's not clear to me why `baseSession()` is not called here. src/java.base/share/classes/jdk/internal/foreign/abi/aarch64/macos/MacOsAArch64VaList.java line 172: > 170: > 171: public Builder(MemorySession session) { > 172: ((MemorySessionImpl)session).checkValidState(); Or here, if the memory session is a non-closeable view. - PR: https://git.openjdk.org/jdk19/pull/22
Integrated: JDK-8289775: Update java.lang.invoke.MethodHandle[s] to use snippets
On Tue, 5 Jul 2022 22:22:46 GMT, Joe Darcy wrote: > Update existing examples in java.lang.invoke.{MethodHandle, MethodHandles} > to use snippets rather than the older markup idiom. This pull request has now been integrated. Changeset: a40c17b7 Author:Joe Darcy URL: https://git.openjdk.org/jdk/commit/a40c17b730257919f18066dbce4fc92ed3c4f10e Stats: 123 lines in 2 files changed: 0 ins; 1 del; 122 mod 8289775: Update java.lang.invoke.MethodHandle[s] to use snippets Reviewed-by: jrose - PR: https://git.openjdk.org/jdk/pull/9385
Re: [jdk19] RFR: 8289601: SegmentAllocator::allocateUtf8String(String str) should be clarified for strings containing \0
On Mon, 4 Jul 2022 13:01:34 GMT, Jorn Vernee wrote: > This PR updates the spec and implementation to throw an > `IllegalArgumentException` when an attempt is made to convert a Java string > containing null characters to a C string. > > Testing: local run of the `jdk_foreign` test suite. I've tweaked the Javadoc. I realized it should also be tweaked for `MemorySegment::setUtf8String`. - PR: https://git.openjdk.org/jdk19/pull/107
Re: [jdk19] RFR: 8287809: Revisit implementation of memory session [v4]
On Fri, 17 Jun 2022 18:39:03 GMT, Maurizio Cimadamore wrote: >> This is a JDK 19 clone of: https://github.com/openjdk/jdk/pull/9017 > > Maurizio Cimadamore has updated the pull request incrementally with one > additional commit since the last revision: > > Revert implicit vs. heap session changes Am I understanding correctly that, before calling `checkValidState` we also need to always call `baseSession()`? `checkValidState()` just calls `checkValidStateRaw` in a try/catch, and for non-closeable views this always throws. There seem to be many cases where `baseSession()` is not called... - PR: https://git.openjdk.org/jdk19/pull/22
Re: [jdk19] RFR: 8287809: Revisit implementation of memory session [v4]
On Wed, 6 Jul 2022 17:05:51 GMT, Jorn Vernee wrote: >> Maurizio Cimadamore has updated the pull request incrementally with one >> additional commit since the last revision: >> >> Revert implicit vs. heap session changes > > src/java.base/share/classes/java/lang/invoke/X-VarHandleSegmentView.java.template > line 131: > >> 129: AbstractMemorySegmentImpl bb = checkAddress(obb, base, >> handle.length, true); >> 130: #if[floatingPoint] >> 131: $rawType$ rawValue = >> SCOPED_MEMORY_ACCESS.get$RawType$Unaligned(bb.session(), > > For instance, it's not clear to me why `baseSession()` is not called here. It seems that, if `get$RawType$Unaligned` calls `checkValidStateRaw()` this will end up throwing an exception - PR: https://git.openjdk.org/jdk19/pull/22
Re: [jdk19] RFR: 8289030: [macos] app image signature invalid when creating DMG or PKG
On Wed, 29 Jun 2022 03:03:15 GMT, Alexander Matveev wrote: > Fixed 3 issues which made signature invalid: > - We should not remove .jpackage.xml from signed app image when creating DMG > or PKG otherwise it invalidates signature. > - .package should be created when app image is generated, so this file can be > signed. > - Copying predefine app image for DMG and PKG should not follow symbolic > links, otherwise several files from runtime (COPYRIGHT and LICENSE) will be > copied instead of symbolic links being created, since it invalidates > signature as well. > > Added additional test to validate signature when DMG or PKG is generated from > predefined app image. 8289030: [macos] app image signature invalid when creating DMG or PKG [v2] - Fixed as suggested by Alexey. If app image already signed we will not add `.package` file. - PR: https://git.openjdk.org/jdk19/pull/89
Re: RFR: 8289768: Clean up unused code [v2]
On Wed, 6 Jul 2022 05:32:29 GMT, Daniel Jeliński wrote: >> This patch removes many unused variables and one unused label reported by >> the compilers when relevant warnings are enabled. >> >> The unused code was found by compiling after removing `unused` from the list >> of disabled warnings for >> [gcc](https://github.com/openjdk/jdk/blob/master/make/autoconf/flags-cflags.m4#L190) >> and >> [clang](https://github.com/openjdk/jdk/blob/master/make/autoconf/flags-cflags.m4#L203), >> and enabling >> [C4189](https://docs.microsoft.com/en-us/cpp/error-messages/compiler-warnings/compiler-warning-level-4-c4189?view=msvc-170) >> MSVC warning. >> >> I only removed variables that were uninitialized or initialized without side >> effects. I verified that the removed variables were not used in any >> `#ifdef`'d code. I checked that the changed code still compiles on Windows, >> Linux and Mac, both in release and debug versions. > > Daniel Jeliński 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 four additional > commits since the last revision: > > - Merge remote-tracking branch 'origin' into unused-variables > - Remove unused variables (windows) > - Remove unused variables (macos) > - Remove unused variables (linux) zlib looks fine given it is openjdk and not an upstream needed change - Marked as reviewed by lancea (Reviewer). PR: https://git.openjdk.org/jdk/pull/9383
Re: [jdk19] RFR: 8289779: Map::replaceAll javadoc has redundant @throws clauses
On Wed, 6 Jul 2022 00:32:00 GMT, Stuart Marks wrote: > Simple javadoc fix of an editorial nature. Thanks for doing this, Stuart. Not only does this PR remove duplication from the `Map` documentation, but it also ensures that when JDK-6509045 is integrated, that duplication won't spread to `ConcurrentMap`. A bonus of this fix is that the irrelevant mention of "keys" is gone from the description of `NullPointerException`. Also, consider closing 8255426 as a duplicate. - Marked as reviewed by prappo (Reviewer). PR: https://git.openjdk.org/jdk19/pull/111
Integrated: 8289260: BigDecimal movePointLeft() and movePointRight() do not follow their API spec
On Tue, 28 Jun 2022 10:09:36 GMT, Raffaello Giulietti wrote: > `BigDecimal.morePoint[Left|Right]()` should return the target `this` when the > argument is 0 _and_ the scale is non-negative. This pull request has now been integrated. Changeset: 35387d5c Author:Raffaello Giulietti Committer: Joe Darcy URL: https://git.openjdk.org/jdk/commit/35387d5cb6aa9e59d62b8e1b137b53ec88521310 Stats: 108 lines in 2 files changed: 106 ins; 0 del; 2 mod 8289260: BigDecimal movePointLeft() and movePointRight() do not follow their API spec Reviewed-by: darcy - PR: https://git.openjdk.org/jdk/pull/9307
Re: RFR: 8288723: Avoid redundant ConcurrentHashMap.get call in java.time [v2]
On Tue, 5 Jul 2022 21:27:16 GMT, Attila Szegedi wrote: >> Can there be some JMH tests to confirm the performance? >> The value domain of the keys is pretty limited (7 * 7) max; and I'm not sure >> that the combination of creating a new record and the hashcode and equals >> methods would be faster than string concat of a constant and a single digit >> integer. > > @RogerRiggs in my view, it's not about performance per se… I'll sometimes > help along these kinds of small fix PRs (that are more likely than not driven > by IntelliJ code refactoring suggestions) if I think they lead to more > idiomatic code in the JDK with no drawbacks. I think there's value in > periodically revisiting existing code and improve it to use new language and > library construct that have become available since – people study JDK source > code, and I think it's important they see as good examples in there as > possible. I do like seeing an old example of hand-rolled compute-if-absent > being replaced by an actual `computeIfAbsent` call. Similarly, I think using > records as composite keys is in general a better practice over simulating a > composite key by creating a string representation of it. > > Yeah, it'll load additional code at runtime for that record's class, and yes, > I'm aware records' `hashCode`/`equals`/`toString` are implemented by > delegating to factory methods that heavily use method handle combinators. If > records are meant as lightweight value-semantics composites, it should be > okay to use them like this and if there are performance concerns with their > use, then those concerns should be addressed by improving their performance. > OTOH, MH combinators installed into constant call sites are well optimized by > HotSpot these days… anyhow, a discussion for another day. > > In this specific instance, the value domain of the keys is limited, but the > number of method calls to `WeekFields.of` isn't. It could be benchmarked but > if there's concerns, it's also okay to de-scope the record-as-key from this > PR. I don't have strong feelings either way. The comment was about WeekFields.of(), I misplaced the comment. @szegedi All good points about modernizing code... One of the reasons to ask about specific performance data is to validate the general performance impact of using lambdas. In the case of WeekFields.of(), the lambda is passed on every call to `computeIfAbsent` even if the key is present and the lambda won't be used. `WeekFields` is way-way down the long tail of frequency of use and I expect that 99% of calls are for the same one or two combinations. - PR: https://git.openjdk.org/jdk/pull/9208
Re: [jdk19] RFR: 8289779: Map::replaceAll javadoc has redundant @throws clauses [v2]
On Wed, 6 Jul 2022 23:03:42 GMT, Stuart Marks wrote: >> Simple javadoc fix of an editorial nature. > > Stuart Marks has updated the pull request incrementally with one additional > commit since the last revision: > > Reflow and adjust whitespace per Alan's & Iris' comments. Reflowed and adjusted whitespace. Also, Pavel wrote: > Also, consider closing 8255426 as a duplicate. Ha! Thanks, I had forgotten about that one. Easier to file a new bug than to dig out an old one. Oh well, closed as duplicate. - PR: https://git.openjdk.org/jdk19/pull/111
Integrated: Merge jdk19
On Wed, 6 Jul 2022 13:01:02 GMT, Jesper Wilhelmsson wrote: > Forwardport JDK 19 -> JDK 20 This pull request has now been integrated. Changeset: 2a6ec88c Author:Jesper Wilhelmsson URL: https://git.openjdk.org/jdk/commit/2a6ec88cd09adec43df3da1b22653271517b14a8 Stats: 888 lines in 24 files changed: 705 ins; 64 del; 119 mod Merge - PR: https://git.openjdk.org/jdk/pull/9397
Re: RFR: 8288838: jpackage: file association additional arguments [v4]
On Mon, 4 Jul 2022 23:38:08 GMT, Alex Kasko wrote: >> jpackage implementation of file association on Windows currently passes a >> selected filename as an only argument to associated executable. >> >> It is proposed to introduce additional option in file association property >> file to allow optionally support additional arguments using `%*` batch >> wildcard. >> >> Note, current implementation, while fully functional, is only a **DRAFT** >> one, it is not ready for integration in this form. I would appreciate any >> guidance on the following points: >> >> - option naming inside a properties file, currently `pass-all-args` is used >> - option naming in a bundler parameter implementation, it is not clear if >> it should introduce a new group of "file association windows specific >> options" next to the existing "file association mac specific options" group >> - test organization to cover the new option: currently it is included >> inside `FileAssociationTest` and piggybacks on the existing (and unrelated) >> `includeDescription` parameter; it is not clear whether it should be done in >> a separate test and whether to include runs for every parameter combination >> - test run implementation: currently arguments are checked when a file with >> associated extension is invoked from command line; it is not clear whether >> it would be more appropriate instead to create a desktop shortcut with the >> same command as a target and to invoke it with `java.awt.Desktop` >> >> Also please note, that full install/uninstall run is currently enabled in >> `FileAssociationTest`, it is intended to be used only in a draft code during >> the development and to be removed (to use the same "install or unpack" logic >> as other tests) in a final version. >> >> Testing: >> >> - [x] test to cover new logic is included >> - [x] ran jtreg:jdk/tools/jpackage with no new failures > > Alex Kasko has updated the pull request with a new target base due to a merge > or a rebase. The pull request now contains three commits: > > - enable passing args, add test coverage > - Added input params validation > - Proposed test changes to make FA testing of jpackage more flexible Marked as reviewed by asemenyuk (Reviewer). I confirm the patch works and there are no regressions. SQE uses only the part of jpackage jtreg tests that create test packages, so there is no point in customizing tests with reading args from the file system. I think I have enough information to help SQE update test spec and cover Unicode args. - PR: https://git.openjdk.org/jdk/pull/9224
Re: RFR: 8288984: Simplification in java.lang.Runtime::exit [v5]
On Tue, 5 Jul 2022 18:43:26 GMT, Ryan Ernst wrote: >> This is a followup to simplify Shutdown.exit after the removal of >> finalizers (https://bugs.openjdk.org/browse/JDK-8198250). Once agreement >> on the approach has been reached in this PR, a CSR will be filed to >> propose the spec change to Runtime.exit. > > Ryan Ernst 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 five additional commits since > the last revision: > > - Merge branch 'master' into shutdown_cleanup > - iter text > - iterate on wording > - better clarify multiple threads behavior > - 8288984: Simplification in Shutdown.exit > >This is a followup to simplify Shutdown.exit after the removal of >finalizers (https://bugs.openjdk.org/browse/JDK-8198250). Once agreement >on the approach has been reached in this PR, a CSR will be filed to >propose the spec change to Runtime.exit. Marked as reviewed by chegar (Reviewer). - PR: https://git.openjdk.org/jdk/pull/9351
Re: RFR: JDK-8289741 : Remove unused imports from DTDBuilder.java [v2]
> This is tracked in JBS as > > [JDK-8289741](https://bugs.openjdk.org/browse/JDK-8289741) > >> **Remove unused imports from DTDBuilder.java** > > Some imports are no more used. > > - java.io.FileNotFoundException; > - java.io.BufferedInputStream; > - java.io.OutputStream; > - java.util.BitSet; > - java.util.StringTokenizer; > - java.util.Properties; > - java.util.zip.DeflaterOutputStream; > - java.util.zip.Deflater; > - java.net.URL; > > https://github.com/scientificware/jdk/issues/4 ScientificWare 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 eight additional commits since the last revision: - Merge branch 'openjdk:master' into scientificware-patch-001-DTDBuilder - Merge branch 'openjdk:master' into scientificware-patch-001-DTDBuilder - Merge branch 'openjdk:master' into scientificware-patch-001-DTDBuilder - Merge branch 'openjdk:master' into scientificware-patch-001-DTDBuilder - Merge branch 'openjdk:master' into scientificware-patch-001-DTDBuilder - Merge branch 'openjdk:master' into scientificware-patch-001-DTDBuilder - Merge branch 'openjdk:master' into scientificware-patch-001-DTDBuilder - Remove unused imports. Some imports are no more used. - java.io.FileNotFoundException; - java.io.BufferedInputStream; - java.io.OutputStream; - java.util.BitSet; - java.util.StringTokenizer; - java.util.Properties; - java.util.zip.DeflaterOutputStream; - java.util.zip.Deflater; - java.net.URL; - Changes: - all: https://git.openjdk.org/jdk/pull/9360/files - new: https://git.openjdk.org/jdk/pull/9360/files/2bf79718..6d097e77 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=9360&range=01 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=9360&range=00-01 Stats: 3364 lines in 138 files changed: 2399 ins; 468 del; 497 mod Patch: https://git.openjdk.org/jdk/pull/9360.diff Fetch: git fetch https://git.openjdk.org/jdk pull/9360/head:pull/9360 PR: https://git.openjdk.org/jdk/pull/9360
Re: RFR: 8289834: Add SBCS and DBCS Only EBCDIC charsets
On Wed, 6 Jul 2022 14:05:39 GMT, Ichiroh Takiguchi wrote: > OpenJDK supports "Japanese EBCDIC - Katakana" and "Korean EBCDIC" SBCS and > DBCS Only charsets. > |Charset|Mix|SBCS|DBCS| > | -- | -- | -- | -- | > | Japanese EBCDIC - Katakana | Cp930 | Cp290 | Cp300 | > | Korean | Cp933 | Cp833 | Cp834 | > > But OpenJDK does not supports some of "Japanese EBCDIC - English" / > "Simplified Chinese EBCDIC" / "Traditional Chinese EBCDIC" SBCS and DBCS Only > charsets. > > I'd like to request Cp1027/Cp835/Cp836/Cp837 for consistency > |Charset|Mix|SBCS|DBCS| > | - | - | - | - | > | Japanese EBCDIC - English | Cp939 | **Cp1027** | Cp300 | > | Simplified Chinese EBCDIC | Cp935 | **Cp836** | **Cp837** | > | Traditional Chinese EBCDIC | Cp937 | (*1) | **Cp835** | > > *1: Cp037 compatible Discussions are available on : [JDK-8289834](https://bugs.openjdk.org/browse/JDK-8289834): Add SBCS and DBCS Only EBCDIC charsets - PR: https://git.openjdk.org/jdk/pull/9399
Re: RFR: 8288706: Unused parameter 'boolean newln' in method java.lang.VersionProps#print(boolean, boolean)
On Tue, 5 Jul 2022 18:38:57 GMT, Lance Andersen wrote: > Hi all, > > This PR cleans up `VersionProps::print` removing the unused parameter `newln`. > > Mach5 tiers1-3 are currently running. > > Best, > Lance Marked as reviewed by rriggs (Reviewer). - PR: https://git.openjdk.org/jdk/pull/9382
Re: RFR: 8289711: Add container configuration data to mbeans
On Tue, 5 Jul 2022 04:21:55 GMT, xpbob wrote: > Container configuration information is useful for troubleshooting > problems,Exposing information in MBeans is ideal for monitoring, jConsole, > and other scenarios. > Results the following > ![图片](https://user-images.githubusercontent.com/7837910/177248834-50beefe9-4db6-470c-8f15-df5a93892804.png) src/java.base/share/classes/module-info.java line 233: > 231: exports jdk.internal.platform to > 232: jdk.management, > 233: java.management, Maybe, we'd better put `jdk.management,` before `jdk.management,`. src/java.management/share/classes/java/lang/management/ContainerMXBean.java line 1: > 1: package java.lang.management; Shall we add a copyright header for this new file? - PR: https://git.openjdk.org/jdk/pull/9372
Re: RFR: 8288984: Simplification in java.lang.Runtime::exit [v2]
On Mon, 4 Jul 2022 12:19:27 GMT, David Holmes wrote: >> Ryan Ernst has updated the pull request incrementally with one additional >> commit since the last revision: >> >> better clarify multiple threads behavior > >> Let's say we've run shutdown so run all the hooks but not halted. Then >> someone calls exit(0). That seems to suggest the call will block >> indefinitely, which is neither desirable nor what was actually implemented. > > If the hook threads do not halt then the exiting thread (which holds the > lock) blocks forever in the join(). Any other call to exit blocks trying to > acquire the lock. That has always been the way this works - if hook threads > don't terminate then the VM doesn't (unless someone calls halt() directly). > That is one of the things the window you suggested be closed, allowed - a > call to exit(non-zero) could force a call to halt(). @dholmes-ora @AlanBateman @kimbarrett The following [CSR](https://bugs.openjdk.org/browse/JDK-8289824) has been filed to cover the specification related changes in this PR - please review as appropriate. - PR: https://git.openjdk.org/jdk/pull/9351
Re: [jdk19] RFR: 8289148: j.l.foreign.VaList::nextVarg call could throw IndexOutOfBoundsException or even crash the VM [v3]
On Tue, 5 Jul 2022 17:27:34 GMT, Jorn Vernee wrote: >> This patch changes all VaList implementations to throw >> `NoSuchElementException` when out of bounds reads occur on a VaList that is >> created using the Java builder API. The docs are updated accordingly. >> >> For VaLists that are created from native addresses, we don't know their >> bounds, so we can not throw exceptions in that case. >> >> Testing: `jdk_foreign` test suite on all platforms that implement VaList. > > Jorn Vernee has updated the pull request incrementally with one additional > commit since the last revision: > > hmtl -> html Looks good! - Marked as reviewed by mcimadamore (Reviewer). PR: https://git.openjdk.org/jdk19/pull/91
Re: RFR: 8288984: Simplification in java.lang.Runtime::exit [v2]
On Mon, 4 Jul 2022 12:09:46 GMT, David Holmes wrote: >>> Is "deadlock" accurate? >> >> Yes. >> >> In the context of the specification, "shutdown hook" means _application_ >> shutdown hook - as far as the specification is concerned, application >> shutdown hooks are the only kind of hooks. Right? >> >> For example, the following will deadlock (when run with the changes in this >> PR): >> >> >> public class TestHook { >> public static void main(String... arg) { >> Thread hook = new Thread("my-hook") { >> @Override >> public void run() { >> System.exit(1); >> } >> }; >> Runtime.getRuntime().addShutdownHook(hook); >> System.exit(0); >> } >> } > > It is a general deadlock, not a monitor based deadlock: the thread that > called exit and holds the lock has to join() the hook thread. The hook thread > blocks on the lock held by the exiting thread. Neither thread can progress. You folks are correct. I hadn't noticed the ApplicationShutdownHooks thing. Sorry for the noise. - PR: https://git.openjdk.org/jdk/pull/9351
Re: RFR: 8288984: Simplification in Shutdown.exit [v2]
On Tue, 5 Jul 2022 04:51:50 GMT, Ryan Ernst wrote: >> YAO (Yet Another Opinion)! But I think we're actually converging. >> >> David's wording: >>>Invocations of this method are serialized such that only one invocation will >>>actually proceed with the shutdown sequence and terminate the VM with the >>>given status code. >> >> ... gives the impression that an invocation of this method WILL terminate >> the VM with the given status code - which is not actually true, given the >> potential for signals. This is already alluded to in >> `Runtime::addShutdownHook`. I agree that this could be resolved separately, >> but we should _allow_ for signals (even if we don't explicitly mention them). >> >> The original wording: >>> If this method is invoked after all shutdown hooks have already been run >>> ... >> >> ... seems quite clever (and allows for signals). Maybe we could keep things >> super simple: >>> If this method is invoked after shutdown hooks have already been started, >>> it will block indefinitely. If this method is invoked from a shutdown hook >>> the system will deadlock. > > I like the new suggested wording, but I would slightly tweak it. As Alan > mentioned in another comment the first paragraph makes clear the method never > returns normally. How does the following sound? > >> If this method is invoked after shutdown hooks have already been started, >> the supplied status code will be ignored. If this method is invoked from a >> shutdown hook the system will deadlock. > gives the impression that an invocation of this method WILL terminate the VM > with the given status code - which is not actually true, given the potential > for signals. The subtle distinction being calling Runtime.exit versus Shutdown.exit. The actual serialization takes places in Shutdown.exit and serializes with signals too. But discussion of signals has no place here - I'm not even sure we document those ways of terminating the VM anywhere? - PR: https://git.openjdk.org/jdk/pull/9351
Re: RFR: 8289711: Add container configuration data to mbeans [v2]
On Wed, 6 Jul 2022 03:52:30 GMT, xpbob wrote: >> Container configuration information is useful for troubleshooting >> problems,Exposing information in MBeans is ideal for monitoring, jConsole, >> and other scenarios. >> Results the following >> ![图片](https://user-images.githubusercontent.com/7837910/177248834-50beefe9-4db6-470c-8f15-df5a93892804.png) > > xpbob has updated the pull request incrementally with one additional commit > since the last revision: > > update header I do think this could be useful. Having said that, some of this has been discussed in https://bugs.openjdk.org/browse/JDK-8199944 (and been considered as won't fix at the time). I tend to agree that it should be a JDK (and even an OS) specific bean. The relevant information is only available on Linux. +1 on making this JDK specific. First step is probably to get consensus on a CSR. - PR: https://git.openjdk.org/jdk/pull/9372
Re: RFR: 8288984: Simplification in java.lang.Runtime::exit [v2]
On Tue, 5 Jul 2022 21:56:27 GMT, Kim Barrett wrote: > I think that isn't true. If a hook doesn't terminate then VM.shutdown doesn't > get called, so the window never gets cracked opened to call halt directly. @kimbarrett Any thread can call halt() directly while the attempted shutdown is in progress. - PR: https://git.openjdk.org/jdk/pull/9351
Re: [jdk19] RFR: 8289779: Map::replaceAll javadoc has redundant @throws clauses
On Wed, 6 Jul 2022 00:32:00 GMT, Stuart Marks wrote: > Simple javadoc fix of an editorial nature. Marked as reviewed by iris (Reviewer). src/java.base/share/classes/java/util/Map.java line 740: > 738: * @param function the function to apply to each entry > 739: * @throws UnsupportedOperationException if the {@code set} operation > 740: * is not supported by this map's entry set iterator. Maybe fix indentation here and at line 742 for the CCE for consistency? - PR: https://git.openjdk.org/jdk19/pull/111
Re: RFR: 8288838: jpackage: file association additional arguments [v3]
On Wed, 29 Jun 2022 12:29:33 GMT, Alex Kasko wrote: >> jpackage implementation of file association on Windows currently passes a >> selected filename as an only argument to associated executable. >> >> It is proposed to introduce additional option in file association property >> file to allow optionally support additional arguments using `%*` batch >> wildcard. >> >> Note, current implementation, while fully functional, is only a **DRAFT** >> one, it is not ready for integration in this form. I would appreciate any >> guidance on the following points: >> >> - option naming inside a properties file, currently `pass-all-args` is used >> - option naming in a bundler parameter implementation, it is not clear if >> it should introduce a new group of "file association windows specific >> options" next to the existing "file association mac specific options" group >> - test organization to cover the new option: currently it is included >> inside `FileAssociationTest` and piggybacks on the existing (and unrelated) >> `includeDescription` parameter; it is not clear whether it should be done in >> a separate test and whether to include runs for every parameter combination >> - test run implementation: currently arguments are checked when a file with >> associated extension is invoked from command line; it is not clear whether >> it would be more appropriate instead to create a desktop shortcut with the >> same command as a target and to invoke it with `java.awt.Desktop` >> >> Also please note, that full install/uninstall run is currently enabled in >> `FileAssociationTest`, it is intended to be used only in a draft code during >> the development and to be removed (to use the same "install or unpack" logic >> as other tests) in a final version. >> >> Testing: >> >> - [x] test to cover new logic is included >> - [x] ran jtreg:jdk/tools/jpackage with no new failures > > Alex Kasko has refreshed the contents of this pull request, and previous > commits have been removed. Incremental views are not available. test/jdk/tools/jpackage/share/FileAssociationsTest.java line 108: > 106: .applyTo(packageTest); > 107: > 108: packageTest.run(RunnablePackageTest.Action.CREATE, > RunnablePackageTest.Action.INSTALL, FYI: the default test steps can be overridden with the value of `jpackage.test.action` system property. Its value would be `create,install,verify_install,uninstall` for this case. UPD: Nevermind this comment. It applied to the old version of the PR. - PR: https://git.openjdk.org/jdk/pull/9224
Re: RFR: 8288838: jpackage: file association additional arguments [v4]
On Mon, 4 Jul 2022 23:38:08 GMT, Alex Kasko wrote: >> jpackage implementation of file association on Windows currently passes a >> selected filename as an only argument to associated executable. >> >> It is proposed to introduce additional option in file association property >> file to allow optionally support additional arguments using `%*` batch >> wildcard. >> >> Note, current implementation, while fully functional, is only a **DRAFT** >> one, it is not ready for integration in this form. I would appreciate any >> guidance on the following points: >> >> - option naming inside a properties file, currently `pass-all-args` is used >> - option naming in a bundler parameter implementation, it is not clear if >> it should introduce a new group of "file association windows specific >> options" next to the existing "file association mac specific options" group >> - test organization to cover the new option: currently it is included >> inside `FileAssociationTest` and piggybacks on the existing (and unrelated) >> `includeDescription` parameter; it is not clear whether it should be done in >> a separate test and whether to include runs for every parameter combination >> - test run implementation: currently arguments are checked when a file with >> associated extension is invoked from command line; it is not clear whether >> it would be more appropriate instead to create a desktop shortcut with the >> same command as a target and to invoke it with `java.awt.Desktop` >> >> Also please note, that full install/uninstall run is currently enabled in >> `FileAssociationTest`, it is intended to be used only in a draft code during >> the development and to be removed (to use the same "install or unpack" logic >> as other tests) in a final version. >> >> Testing: >> >> - [x] test to cover new logic is included >> - [x] ran jtreg:jdk/tools/jpackage with no new failures > > Alex Kasko 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: > > - enable passing args, add test coverage > - Added input params validation > - Proposed test changes to make FA testing of jpackage more flexible Thanks for the reviews! - PR: https://git.openjdk.org/jdk/pull/9224
Re: RFR: 8284780: Need methods to create pre-sized HashSet and LinkedHashSet [v17]
On Wed, 1 Jun 2022 16:45:35 GMT, liach wrote: >> XenoAmess has updated the pull request incrementally with one additional >> commit since the last revision: >> >> do it as naotoj said > > 'the new' fix should be applied to newHashMap etc. too. @liach > 'the new' fix should be applied to newHashMap etc. too. I've filed [JDK-8289872](https://bugs.openjdk.org/browse/JDK-8289872) for this. It's a small thing but best to fix it before we forget about it. - PR: https://git.openjdk.org/jdk/pull/8302
Re: [jdk19] RFR: 8287809: Revisit implementation of memory session [v6]
> This is a JDK 19 clone of: https://github.com/openjdk/jdk/pull/9017 Maurizio Cimadamore has updated the pull request incrementally with one additional commit since the last revision: Turn non-closeable view back into MemorySession impl - Changes: - all: https://git.openjdk.org/jdk19/pull/22/files - new: https://git.openjdk.org/jdk19/pull/22/files/09bb7cf3..809a0a2e Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk19&pr=22&range=05 - incr: https://webrevs.openjdk.org/?repo=jdk19&pr=22&range=04-05 Stats: 79 lines in 14 files changed: 7 ins; 11 del; 61 mod Patch: https://git.openjdk.org/jdk19/pull/22.diff Fetch: git fetch https://git.openjdk.org/jdk19 pull/22/head:pull/22 PR: https://git.openjdk.org/jdk19/pull/22
RFR: 8249834: Marked Bug8146568 and Bug8148174 as manual tests.
Tests Bug8146568 and Bug8148174 were disabled for high memory consumption, over 17G. This is a task to re-enable these two tests by marking them as manual tests. - Commit messages: - Marked Bug8146568 and Bug8148174 as manual tests. Changes: https://git.openjdk.org/jdk/pull/9404/files Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=9404&range=00 Issue: https://bugs.openjdk.org/browse/JDK-8249834 Stats: 6 lines in 3 files changed: 2 ins; 0 del; 4 mod Patch: https://git.openjdk.org/jdk/pull/9404.diff Fetch: git fetch https://git.openjdk.org/jdk pull/9404/head:pull/9404 PR: https://git.openjdk.org/jdk/pull/9404
Re: RFR: 8288838: jpackage: file association additional arguments [v3]
On Thu, 30 Jun 2022 18:50:23 GMT, Alexey Semenyuk wrote: >> Alex Kasko has updated the pull request incrementally with one additional >> commit since the last revision: >> >> drop pass-all-args property > > test/jdk/tools/jpackage/share/FileAssociationsTest.java line 108: > >> 106: .applyTo(packageTest); >> 107: >> 108: packageTest.run(RunnablePackageTest.Action.CREATE, >> RunnablePackageTest.Action.INSTALL, > > FYI: the default test steps can be overridden with the value of > `jpackage.test.action` system property. > Its value would be `create,install,verify_install,uninstall` for this case. > > UPD: Nevermind this comment. It applied to the old version of the PR. Thanks for the info! I've seen `jpackage.test.action` property in `run_tests.sh`, but never used it. - PR: https://git.openjdk.org/jdk/pull/9224
Re: [jdk19] RFR: 8287809: Revisit implementation of memory session [v5]
On Wed, 6 Jul 2022 18:01:28 GMT, Maurizio Cimadamore wrote: >> This is a JDK 19 clone of: https://github.com/openjdk/jdk/pull/9017 > > Maurizio Cimadamore has updated the pull request with a new target base due > to a merge or a rebase. The pull request now contains ten commits: > > - Merge branch 'master' into memory_session_cleanup > - Fix ambiguity between session vs. base session > - Revert implicit vs. heap session changes > - Unify heap vs. implicit scopes > - Merge branch 'master' into memory_session_cleanup > - Fix issue in Direct-X-Buffer template > - Simplify and drop the state class > - Add missing files > - Initial push I've fixed the issues in the review comments, and further reduced the differences between the contents of this patch and mainline (while retaining the improvements). - PR: https://git.openjdk.org/jdk19/pull/22
Re: [jdk19] RFR: 8289872: wrong wording in @param doc for HashMap.newHashMap et. al.
On Thu, 7 Jul 2022 06:06:42 GMT, Stuart Marks wrote: > Minor doc wording changes, to be consistent with HashSet.newHashSet et. al. LGTM - Marked as reviewed by chegar (Reviewer). PR: https://git.openjdk.org/jdk19/pull/118
Re: RFR: 8289768: Clean up unused code [v2]
On Wed, 6 Jul 2022 05:32:29 GMT, Daniel Jeliński wrote: >> This patch removes many unused variables and one unused label reported by >> the compilers when relevant warnings are enabled. >> >> The unused code was found by compiling after removing `unused` from the list >> of disabled warnings for >> [gcc](https://github.com/openjdk/jdk/blob/master/make/autoconf/flags-cflags.m4#L190) >> and >> [clang](https://github.com/openjdk/jdk/blob/master/make/autoconf/flags-cflags.m4#L203), >> and enabling >> [C4189](https://docs.microsoft.com/en-us/cpp/error-messages/compiler-warnings/compiler-warning-level-4-c4189?view=msvc-170) >> MSVC warning. >> >> I only removed variables that were uninitialized or initialized without side >> effects. I verified that the removed variables were not used in any >> `#ifdef`'d code. I checked that the changed code still compiles on Windows, >> Linux and Mac, both in release and debug versions. > > Daniel Jeliński 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 four additional > commits since the last revision: > > - Merge remote-tracking branch 'origin' into unused-variables > - Remove unused variables (windows) > - Remove unused variables (macos) > - Remove unused variables (linux) Marked as reviewed by naoto (Reviewer). - PR: https://git.openjdk.org/jdk/pull/9383
Re: RFR: JDK-8289775: Update java.lang.invoke.MethodHandle[s] to use snippets
On Wed, 6 Jul 2022 00:08:04 GMT, liach wrote: >> Update existing examples in java.lang.invoke.{MethodHandle, MethodHandles} >> to use snippets rather than the older markup idiom. > > src/java.base/share/classes/java/lang/invoke/MethodHandle.java line 272: > >> 270: * Here are some examples of usage: >> 271: * {@snippet lang="java" : >> 272: Object x, y; String s; int i; > > These lines can now be properly indented, and they should be. That isn't an unreasonable suggestion. However, I wanted to present this change in an easy-to-review fashion so limited the diffs to the bracketing tags. - PR: https://git.openjdk.org/jdk/pull/9385
Re: [jdk19] RFR: 8289601: SegmentAllocator::allocateUtf8String(String str) should be clarified for strings containing \0 [v2]
On Wed, 6 Jul 2022 11:09:33 GMT, Jorn Vernee wrote: >> This PR updates the spec and implementation to throw an >> `IllegalArgumentException` when an attempt is made to convert a Java string >> containing null characters to a C string. >> >> Testing: local run of the `jdk_foreign` test suite. > > Jorn Vernee has updated the pull request incrementally with two additional > commits since the last revision: > > - Add javadoc > - Revert "Throw IAE when converting string with null byte." > >This reverts commit 70f83902a38e863b96fedb312982f7ac683ed068. Looks good - Marked as reviewed by mcimadamore (Reviewer). PR: https://git.openjdk.org/jdk19/pull/107
Re: [jdk19] RFR: 8289779: Map::replaceAll javadoc has redundant @throws clauses [v2]
On Wed, 6 Jul 2022 23:03:42 GMT, Stuart Marks wrote: >> Simple javadoc fix of an editorial nature. > > Stuart Marks has updated the pull request incrementally with one additional > commit since the last revision: > > Reflow and adjust whitespace per Alan's & Iris' comments. Marked as reviewed by iris (Reviewer). - PR: https://git.openjdk.org/jdk19/pull/111
Re: RFR: 8289834: Add SBCS and DBCS Only EBCDIC charsets
On Wed, 6 Jul 2022 16:18:08 GMT, Ichiroh Takiguchi wrote: > Discussions are available on : > [JDK-8289834](https://bugs.openjdk.org/browse/JDK-8289834): Add SBCS and DBCS > Only EBCDIC charsets Yes, I think this need discussion on whether the JDK really needs to keep including and adding more EBCDIC charsets. I understand they can be useful for someone using JDBC to connect to a database on z/OS but this scenario would work equally well if the EBCDIC charsets were deployed on the class path or module path. Do you know if the icu4j project is still alive? I've often wondered why there wasn't more use of the provider mechanism. - PR: https://git.openjdk.org/jdk/pull/9399
Re: RFR: JDK-8289775: Update java.lang.invoke.MethodHandle[s] to use snippets
On Tue, 5 Jul 2022 22:22:46 GMT, Joe Darcy wrote: > Update existing examples in java.lang.invoke.{MethodHandle, MethodHandles} > to use snippets rather than the older markup idiom. The code examples in these files were exdented to make it easier to extract example code and test it, and to maintain equivalence during active development of the APIs. The examples in runnable form are in test/jdk/java/lang/invoke/JavaDocExamplesTest.java in the same repo. It would be nice if some day we could tie together the two copies into one using a link. I don't recall whether snippets can do this or not, given that the two copies are far apart in the repo structure. I agree that just the bracket changes are valuable by themselves. I also have no objection to fixing the exdentation. - Marked as reviewed by jrose (Reviewer). PR: https://git.openjdk.org/jdk/pull/9385
Re: [jdk19] RFR: 8287809: Revisit implementation of memory session [v4]
On Wed, 6 Jul 2022 17:07:37 GMT, Jorn Vernee wrote: >> Maurizio Cimadamore has updated the pull request incrementally with one >> additional commit since the last revision: >> >> Revert implicit vs. heap session changes > > src/java.base/share/classes/jdk/internal/foreign/abi/aarch64/macos/MacOsAArch64VaList.java > line 172: > >> 170: >> 171: public Builder(MemorySession session) { >> 172: ((MemorySessionImpl)session).checkValidState(); > > Or here, if the memory session is a non-closeable view. I believe there was a wrong renaming with the IDE here, I will fix this - PR: https://git.openjdk.org/jdk19/pull/22
Re: RFR: JDK-8289106: Add model of class file versions to core reflection [v2]
On Wed, 29 Jun 2022 21:32:29 GMT, Joe Darcy wrote: >> JDK-8289106: Add model of class file versions to core reflection > > Joe Darcy 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 10 additional commits since > the last revision: > > - Add method to map from major class file version. > - Merge branch 'master' into JDK-8289106 > - Update phrasing. > - Augment and correct docs. > - Correct major versions; RELEASE_0 and RELEASE_1 are both 45. > - Make major method public. > - Add AccessFlag.locations(ClassFileFormatVersion) > - Add ClassFileFormatVersion <-> Runtime.Version conversions. > - Merge branch 'master' into JDK-8289106 > - JDK-8289106: Add model of class file versions to core reflection I like the proposed concept and Classfile Processing API is definitely one of the potential consumers. - PR: https://git.openjdk.org/jdk/pull/9299
Re: [jdk19] RFR: 8289779: Map::replaceAll javadoc has redundant @throws clauses [v2]
On Wed, 6 Jul 2022 23:03:42 GMT, Stuart Marks wrote: >> Simple javadoc fix of an editorial nature. > > Stuart Marks has updated the pull request incrementally with one additional > commit since the last revision: > > Reflow and adjust whitespace per Alan's & Iris' comments. Marked as reviewed by prappo (Reviewer). - PR: https://git.openjdk.org/jdk19/pull/111
Re: RFR: JDK-8266431: Dual-Pivot Quicksort improvements (Radix sort) [v14]
On Thu, 30 Jun 2022 16:41:36 GMT, iaroslavski wrote: >> Sorting: >> >> - adopt radix sort for sequential and parallel sorts on >> int/long/float/double arrays (almost random and length > 6K) >> - fix tryMergeRuns() to better handle case when the last run is a single >> element >> - minor javadoc and comment changes >> >> Testing: >> - add new data inputs in tests for sorting >> - add min/max/infinity values to float/double testing >> - add tests for radix sort > > iaroslavski has updated the pull request incrementally with one additional > commit since the last revision: > > JDK-8266431: Dual-Pivot Quicksort improvements (Radix sort) > > * Fix @since version This PR has been open and in development for a long time. One question is whether Arrays.sort has reached its "complexity budget". Right now, and before the addition of radix sort, there are cases where it will do insertion, counting, or heap sorts. Laurent has provided a good set of results to support the case for using radix for random data but it does beg the question on whether Array.sort really needs to try to be optimal for all cases, esp. as there are 4 implementations of each for int, long, float and double elements. Another question is whether the space/performance of tradeoff of using radix sort is too much of a change for Arrays.sort. If I read it correctly then it may allocate up to 1/128 of the heap but I can't immediately see the implications for parallel sort and whether the % of the heap used may be several allocations to up this size (it looks like it is only done on the non-left parts but I'm not 100% sure yet). I see the OOME handling so it can continue when it's not possible to allocate but it may be that allocating big arrays during sort will cause something else in the system to OOME. So before going any further, I think it would be useful to get input on whether this large change has support or whether there are suggestions for other avenues of exploration before committing to the proposal here. - PR: https://git.openjdk.org/jdk/pull/3938
Re: [jdk19] RFR: 8289030: [macos] app image signature invalid when creating DMG or PKG [v2]
> Fixed 3 issues which made signature invalid: > - We should not remove .jpackage.xml from signed app image when creating DMG > or PKG otherwise it invalidates signature. > - .package should be created when app image is generated, so this file can be > signed. > - Copying predefine app image for DMG and PKG should not follow symbolic > links, otherwise several files from runtime (COPYRIGHT and LICENSE) will be > copied instead of symbolic links being created, since it invalidates > signature as well. > > Added additional test to validate signature when DMG or PKG is generated from > predefined app image. Alexander Matveev 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: - Merge remote-tracking branch 'upstream/master' into JDK-8289030 - 8289030: [macos] app image signature invalid when creating DMG or PKG [v2] - 8289030: [macos] app image signature invalid when creating DMG or PKG - Changes: - all: https://git.openjdk.org/jdk19/pull/89/files - new: https://git.openjdk.org/jdk19/pull/89/files/5e47a1e3..4d5d4bce Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk19&pr=89&range=01 - incr: https://webrevs.openjdk.org/?repo=jdk19&pr=89&range=00-01 Stats: 1986 lines in 64 files changed: 1671 ins; 95 del; 220 mod Patch: https://git.openjdk.org/jdk19/pull/89.diff Fetch: git fetch https://git.openjdk.org/jdk19 pull/89/head:pull/89 PR: https://git.openjdk.org/jdk19/pull/89
Re: RFR: 8279283 - BufferedInputStream should override transferTo [v5]
On Mon, 27 Dec 2021 13:43:12 GMT, Markus KARG wrote: >> Implementation of JDK-8279283 > > Markus KARG has updated the pull request incrementally with one additional > commit since the last revision: > > fixed missing BufferedInputStream src/java.base/share/classes/java/io/BufferedInputStream.java line 495: > 493: int avail = count - pos; > 494: if (avail > 0) { > 495: out.write(buf, pos, avail); I think `buf` should not be accessed directly but via `getBufIfOpen()` because we need to throw IOE in case `this` is closed. - PR: https://git.openjdk.org/jdk/pull/6935
Re: RFR: JDK-8289775: Update java.lang.invoke.MethodHandle[s] to use snippets
On Wed, 6 Jul 2022 20:49:25 GMT, John R Rose wrote: > The code examples in these files were exdented to make it easier to extract > example code and test it, and to maintain equivalence during active > development of the APIs. > > The examples in runnable form are in > test/jdk/java/lang/invoke/JavaDocExamplesTest.java in the same repo. > > It would be nice if some day we could tie together the two copies into one > using a link. I don't recall whether snippets can do this or not, given that > the two copies are far apart in the repo structure. > > I agree that just the bracket changes are valuable by themselves. I also have > no objection to fixing the exdentation. Thanks John; I'll push the update to use the snippet tag as-is and leave further refinements to future work. - PR: https://git.openjdk.org/jdk/pull/9385
RFR: 8289797: [TESTBUG]tools/launcher/I18NArgTest.java fails on Japanese Windows enviromnent
I removed a section of via JDK_JAVA_OPTIONS because including main class is not allowed in the specification. This behavior is added in JDK-8170832, which add JAVA_OPTIONS environment variable. At this time, this test is mismatch with the specification. I tried to test and get Passed on Japanese Windows environment. Could you review this fix, please? - Commit messages: - 8289797: [TESTBUG]tools/launcher/I18NArgTest.java fails on Japanese Windows enviromnent Changes: https://git.openjdk.org/jdk/pull/9389/files Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=9389&range=00 Issue: https://bugs.openjdk.org/browse/JDK-8289797 Stats: 15 lines in 1 file changed: 0 ins; 14 del; 1 mod Patch: https://git.openjdk.org/jdk/pull/9389.diff Fetch: git fetch https://git.openjdk.org/jdk pull/9389/head:pull/9389 PR: https://git.openjdk.org/jdk/pull/9389
Re: RFR: 8288838: jpackage: file association additional arguments [v4]
On Mon, 4 Jul 2022 23:38:08 GMT, Alex Kasko wrote: >> jpackage implementation of file association on Windows currently passes a >> selected filename as an only argument to associated executable. >> >> It is proposed to introduce additional option in file association property >> file to allow optionally support additional arguments using `%*` batch >> wildcard. >> >> Note, current implementation, while fully functional, is only a **DRAFT** >> one, it is not ready for integration in this form. I would appreciate any >> guidance on the following points: >> >> - option naming inside a properties file, currently `pass-all-args` is used >> - option naming in a bundler parameter implementation, it is not clear if >> it should introduce a new group of "file association windows specific >> options" next to the existing "file association mac specific options" group >> - test organization to cover the new option: currently it is included >> inside `FileAssociationTest` and piggybacks on the existing (and unrelated) >> `includeDescription` parameter; it is not clear whether it should be done in >> a separate test and whether to include runs for every parameter combination >> - test run implementation: currently arguments are checked when a file with >> associated extension is invoked from command line; it is not clear whether >> it would be more appropriate instead to create a desktop shortcut with the >> same command as a target and to invoke it with `java.awt.Desktop` >> >> Also please note, that full install/uninstall run is currently enabled in >> `FileAssociationTest`, it is intended to be used only in a draft code during >> the development and to be removed (to use the same "install or unpack" logic >> as other tests) in a final version. >> >> Testing: >> >> - [x] test to cover new logic is included >> - [x] ran jtreg:jdk/tools/jpackage with no new failures > > Alex Kasko has updated the pull request with a new target base due to a merge > or a rebase. The pull request now contains three commits: > > - enable passing args, add test coverage > - Added input params validation > - Proposed test changes to make FA testing of jpackage more flexible Marked as reviewed by almatvee (Reviewer). - PR: https://git.openjdk.org/jdk/pull/9224
Re: [jdk19] RFR: 8289779: Map::replaceAll javadoc has redundant @throws clauses
On Wed, 6 Jul 2022 00:32:00 GMT, Stuart Marks wrote: > Simple javadoc fix of an editorial nature. src/java.base/share/classes/java/util/Map.java line 745: > 743: * ( href="{@docRoot}/java.base/java/util/Collection.html#optional-restrictions">optional) > 744: * @throws NullPointerException if the specified function is null, > or if a replacement value is null > 745: * and this map does not permit null values This looks okay although I'd probably reflow L744-745 to avoid to keep the line lengths somewhat consistent with the other exceptions. - PR: https://git.openjdk.org/jdk19/pull/111
RFR: 8289908: Skip bounds check for cases when String is constructed from entirely used byte[]
We can skip bounds check and null check for Charset in case we use the array entirely and the Charset is either default one or proven to be non-null. Benchmark results: before Benchmark Mode Cnt Score Error Units StringConstructor.newStringFromArray avgt 50 4,815 ± 0,154 ns/op StringConstructor.newStringFromArrayWithCharsetavgt 50 4,462 ± 0,068 ns/op StringConstructor.newStringFromArrayWithCharsetNameavgt 50 8,653 ± 0,040 ns/op StringConstructor.newStringFromRangedArray avgt 50 5,090 ± 0,066 ns/op StringConstructor.newStringFromRangedArrayWithCharset avgt 50 4,550 ± 0,041 ns/op StringConstructor.newStringFromRangedArrayWithCharsetName avgt 50 8,080 ± 0,055 ns/op after Benchmark Mode Cnt Score Error Units StringConstructor.newStringFromArray avgt 50 4,595 ± 0,053 ns/op StringConstructor.newStringFromArrayWithCharsetavgt 50 4,038 ± 0,062 ns/op StringConstructor.newStringFromArrayWithCharsetNameavgt 50 8,035 ± 0,031 ns/op StringConstructor.newStringFromRangedArray avgt 50 4,084 ± 0,007 ns/op StringConstructor.newStringFromRangedArrayWithCharset avgt 50 4,014 ± 0,008 ns/op StringConstructor.newStringFromRangedArrayWithCharsetName avgt 50 7,466 ± 0,071 ns/op - Commit messages: - 8289908: Skip bounds check for cases when String is constructed from entirely used byte[] Changes: https://git.openjdk.org/jdk/pull/9407/files Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=9407&range=00 Issue: https://bugs.openjdk.org/browse/JDK-8289908 Stats: 90 lines in 5 files changed: 77 ins; 1 del; 12 mod Patch: https://git.openjdk.org/jdk/pull/9407.diff Fetch: git fetch https://git.openjdk.org/jdk pull/9407/head:pull/9407 PR: https://git.openjdk.org/jdk/pull/9407
[jdk19] RFR: 8289872: wrong wording in @param doc for HashMap.newHashMap et. al.
Minor doc wording changes, to be consistent with HashSet.newHashSet et. al. - Commit messages: - 8289872: wrong wording in @param doc for HashMap.newHashMap et. al. Changes: https://git.openjdk.org/jdk19/pull/118/files Webrev: https://webrevs.openjdk.org/?repo=jdk19&pr=118&range=00 Issue: https://bugs.openjdk.org/browse/JDK-8289872 Stats: 3 lines in 3 files changed: 0 ins; 0 del; 3 mod Patch: https://git.openjdk.org/jdk19/pull/118.diff Fetch: git fetch https://git.openjdk.org/jdk19 pull/118/head:pull/118 PR: https://git.openjdk.org/jdk19/pull/118
Re: [jdk19] RFR: 8289779: Map::replaceAll javadoc has redundant @throws clauses [v2]
> Simple javadoc fix of an editorial nature. Stuart Marks has updated the pull request incrementally with one additional commit since the last revision: Reflow and adjust whitespace per Alan's & Iris' comments. - Changes: - all: https://git.openjdk.org/jdk19/pull/111/files - new: https://git.openjdk.org/jdk19/pull/111/files/8902f466..a3fbdb5b Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk19&pr=111&range=01 - incr: https://webrevs.openjdk.org/?repo=jdk19&pr=111&range=00-01 Stats: 3 lines in 1 file changed: 0 ins; 0 del; 3 mod Patch: https://git.openjdk.org/jdk19/pull/111.diff Fetch: git fetch https://git.openjdk.org/jdk19 pull/111/head:pull/111 PR: https://git.openjdk.org/jdk19/pull/111
Re: [jdk19] RFR: 8289223: Canonicalize header ids in foreign API javadocs
On Mon, 27 Jun 2022 16:44:17 GMT, Jorn Vernee wrote: > https://github.com/openjdk/jdk/pull/8817 added a button to copy a link to a > section of javadoc to the clipboard. For the copy button to appear, the > header needs to have an `id`. > > This cleanup PR canonicalizes all header ids in the java.lang.foreign package > to the preferred (non-legacy) style, and adds ids in places where they are > missing as well. > > In accordance with the Late-Enhancement Request Process [1], this is a > `noreg-doc` documentation only change, which does not require an enhancement > request. > > [1]: https://openjdk.org/jeps/3#Late-Enhancement-Request-Process Looks good - Marked as reviewed by mcimadamore (Reviewer). PR: https://git.openjdk.org/jdk19/pull/75
[jdk19] Integrated: 8289558: Need spec clarification of j.l.foreign.*Layout
On Fri, 1 Jul 2022 11:03:23 GMT, Maurizio Cimadamore wrote: > This patch fixes few javadoc issues in the memory layout API. > The main issues are that `SequenceLayout::flatten` and > `SequenceLayout::reshape` still mention failures caused by a lack of size. > But that condition is no longer possible in the new API. > > The javadoc of `ValueLayout::arrayElementVarHandle` is suboptimal and can be > clarified - UOE is only thrown if the value layout alignment is bigger than > its size. > > Finally, the `MemoryLayout::equals` method does not mention value layout > carriers. > > The JBS issue associated with this PR mentions also other issues, mostly > related to the overly broad visibility of some of the methods in the javadoc > (e.g. isPadding). Unfortunately, given the presence of an intermediate, > non-public, abstract class, this is what we get from javadoc. Fixing these > issues would require a deeper restructuring of the implementation, which > would be too riskt at this stage. This pull request has now been integrated. Changeset: 889150b4 Author:Maurizio Cimadamore URL: https://git.openjdk.org/jdk19/commit/889150b47a7a33d302c1883320d2cfbb915c52e7 Stats: 48 lines in 5 files changed: 14 ins; 11 del; 23 mod 8289558: Need spec clarification of j.l.foreign.*Layout Reviewed-by: psandoz, jvernee - PR: https://git.openjdk.org/jdk19/pull/98
Re: RFR: 8289908: Skip bounds check for cases when String is constructed from entirely used byte[]
On Thu, 7 Jul 2022 10:21:06 GMT, Сергей Цыпанов wrote: > We can skip bounds check and null check for Charset in case we use the array > entirely and the Charset is either default one or proven to be non-null. > > Benchmark results: > > before > > Benchmark Mode Cnt Score > Error Units > StringConstructor.newStringFromArray avgt 50 4,815 ± > 0,154 ns/op > StringConstructor.newStringFromArrayWithCharsetavgt 50 4,462 ± > 0,068 ns/op > StringConstructor.newStringFromArrayWithCharsetNameavgt 50 8,653 ± > 0,040 ns/op > StringConstructor.newStringFromRangedArray avgt 50 5,090 ± > 0,066 ns/op > StringConstructor.newStringFromRangedArrayWithCharset avgt 50 4,550 ± > 0,041 ns/op > StringConstructor.newStringFromRangedArrayWithCharsetName avgt 50 8,080 ± > 0,055 ns/op > > after > > Benchmark Mode Cnt Score > Error Units > StringConstructor.newStringFromArray avgt 50 4,595 ± > 0,053 ns/op > StringConstructor.newStringFromArrayWithCharsetavgt 50 4,038 ± > 0,062 ns/op > StringConstructor.newStringFromArrayWithCharsetNameavgt 50 8,035 ± > 0,031 ns/op > StringConstructor.newStringFromRangedArray avgt 50 4,084 ± > 0,007 ns/op > StringConstructor.newStringFromRangedArrayWithCharset avgt 50 4,014 ± > 0,008 ns/op > StringConstructor.newStringFromRangedArrayWithCharsetName avgt 50 7,466 ± > 0,071 ns/op src/java.base/share/classes/java/lang/String.java line 1429: > 1427: */ > 1428: public String(byte[] bytes, int offset, int length) { > 1429: this(bytes, offset, length, Charset.defaultCharset(), > checkBoundsOffCount(offset, length, bytes.length)); Can you avoid the extra constructor by applying `checkBoundOffCount` to the offset argument; it returns the offset. this(bytes, checkBoundsOffCount(offset, length, bytes.length), length, Charset.defaultCharset()); or call `Preconditions.checkFromIndexSize` directly. - PR: https://git.openjdk.org/jdk/pull/9407
[jdk19] Integrated: 8289486: Improve XSLT XPath operators count efficiency
On Fri, 1 Jul 2022 17:04:10 GMT, Joe Wang wrote: > To improve efficiency, this patch moves the limit check to within the Lexer > and reports any overlimit situation as soon as it happens. > > Note the change in XPathParser: diff (and also webrevs) showed the whole > error-report block was changed, the actual change was only placing the block > to within the isOverLimit statement. > > Test: java.xml tests passed. This pull request has now been integrated. Changeset: 3212dc9c Author:Joe Wang URL: https://git.openjdk.org/jdk19/commit/3212dc9c6f3538e1d0bd1809efd5f33ad8b47701 Stats: 59 lines in 2 files changed: 31 ins; 4 del; 24 mod 8289486: Improve XSLT XPath operators count efficiency Reviewed-by: naoto, lancea - PR: https://git.openjdk.org/jdk19/pull/101
Integrated: 8288838: jpackage: file association additional arguments
On Tue, 21 Jun 2022 09:30:30 GMT, Alex Kasko wrote: > jpackage implementation of file association on Windows currently passes a > selected filename as an only argument to associated executable. > > It is proposed to introduce additional option in file association property > file to allow optionally support additional arguments using `%*` batch > wildcard. > > Note, current implementation, while fully functional, is only a **DRAFT** > one, it is not ready for integration in this form. I would appreciate any > guidance on the following points: > > - option naming inside a properties file, currently `pass-all-args` is used > - option naming in a bundler parameter implementation, it is not clear if it > should introduce a new group of "file association windows specific options" > next to the existing "file association mac specific options" group > - test organization to cover the new option: currently it is included inside > `FileAssociationTest` and piggybacks on the existing (and unrelated) > `includeDescription` parameter; it is not clear whether it should be done in > a separate test and whether to include runs for every parameter combination > - test run implementation: currently arguments are checked when a file with > associated extension is invoked from command line; it is not clear whether it > would be more appropriate instead to create a desktop shortcut with the same > command as a target and to invoke it with `java.awt.Desktop` > > Also please note, that full install/uninstall run is currently enabled in > `FileAssociationTest`, it is intended to be used only in a draft code during > the development and to be removed (to use the same "install or unpack" logic > as other tests) in a final version. > > Testing: > > - [x] test to cover new logic is included > - [x] ran jtreg:jdk/tools/jpackage with no new failures This pull request has now been integrated. Changeset: a694e9e3 Author:Alex Kasko Committer: Alexey Semenyuk URL: https://git.openjdk.org/jdk/commit/a694e9e34d1e4388df200d11b168ca5265cea4ac Stats: 194 lines in 5 files changed: 167 ins; 9 del; 18 mod 8288838: jpackage: file association additional arguments Reviewed-by: asemenyuk, almatvee - PR: https://git.openjdk.org/jdk/pull/9224
Re: [jdk19] RFR: 8289486: Improve XSLT XPath operators count efficiency [v2]
On Thu, 7 Jul 2022 17:32:45 GMT, Joe Wang wrote: >> To improve efficiency, this patch moves the limit check to within the Lexer >> and reports any overlimit situation as soon as it happens. >> >> Note the change in XPathParser: diff (and also webrevs) showed the whole >> error-report block was changed, the actual change was only placing the block >> to within the isOverLimit statement. >> >> Test: java.xml tests passed. > > Joe Wang has updated the pull request incrementally with one additional > commit since the last revision: > > add comments Marked as reviewed by lancea (Reviewer). - PR: https://git.openjdk.org/jdk19/pull/101