JMH results for IndexedLinkedList
Data structure repo: https://github.com/coderodde/IndexedLinkedList Benchmark repo: https://github.com/coderodde/IndexedLinkedListBenchmark I have profiled my data structure and it seems it’s more performant than java.util.LinkedList or TreeList, if nothing else. So, is there any chance of including IndexedLinkedList to JDK? Best regards, rodde
Re: RFR: 8275662: remove test/lib/sun/hotspot
On Fri, 8 Jul 2022 19:46:17 GMT, Coleen Phillimore wrote:
> This change removes the last remnants of sun/hotspot/WhiteBox.java and other
> classes, and uses the versions in jdk/test/whitebox.
> I used sed to change sun.hotspot.{gc,code,cpuinfo} to jdk.test.whitebox and
> deleted the old files and some references to sun.hotspot.
> Tested with tier1-4.
Marked as reviewed by lmesnik (Reviewer).
-
PR: https://git.openjdk.org/jdk/pull/9434
Re: RFR: 8289643: File descriptor leak with ProcessBuilder.startPipeline [v2]
On Fri, 8 Jul 2022 16:24:16 GMT, Roger Riggs wrote:
>> The `ProcessBuilder.pipelineStart()` implementation does not close all of
>> the file descriptors it uses to create the pipeline of processes.
>>
>> The process calling `pipelineStart()` is creating the pipes between the
>> stages.
>> As each process is launched, the file descriptor is inherited by the child
>> process and
>> the child process `dup`s them to the respective stdin/stdout/stderr fd.
>> These copies of inherited file descriptors are handled correctly.
>>
>> Between the launching of each Process, the file descriptor for the read-side
>> of the pipe for the output of the previous process is kept open (in the
>> parent process invoking `pipelineStart`). The file descriptor is correctly
>> passed to the child and is dup'd to the stdin of the next process.
>>
>> However, the open file descriptor in the parent is not closed after it has
>> been used as the input for the next Process.
>> The fix is to close the fd after it has been used as the input of the next
>> process.
>>
>> A new test verifies that after `pipelineStart` is complete, the same file
>> descriptors are open for Unix Pipes as before the test.
>> The test only runs on Linux using the /proc//fd filesystem to identify
>> open file descriptors.
>>
>> The bug fix is in `ProcessBuilder.pipelineStart` and is applicable to all
>> platforms.
>
> Roger Riggs has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Cleanup of PipelineLeaksFD test improving error messages and source cleanup.
test/jdk/java/lang/ProcessBuilder/PipelineLeaksFD.java line 130:
> 128: })
> 129: .filter(p1 -> p1.link().toString().startsWith("pipe:"))
> 130: .collect(Collectors.toSet());
Is this intentionally leaking the returned `DirectoryStream` from `Files.walk`
to trigger the previous failure mode or should this be auto-closed (i.e.
https://errorprone.info/bugpattern/StreamResourceLeak )?
Suggestion:
try (Stream s = Files.walk(path)) {
return s.filter(Files::isSymbolicLink)
.map(p -> {
try {
return new PipeRecord(p, Files.readSymbolicLink(p));
} catch (IOException ioe) {
}
return new PipeRecord(p, null);
})
.filter(p1 -> p1.link().toString().startsWith("pipe:"))
.collect(Collectors.toSet());
}
-
PR: https://git.openjdk.org/jdk/pull/9414
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 kbarrett (Reviewer). - PR: https://git.openjdk.org/jdk/pull/9351
Re: RFR: 8275662: remove test/lib/sun/hotspot
On Fri, 8 Jul 2022 19:46:17 GMT, Coleen Phillimore wrote:
> This change removes the last remnants of sun/hotspot/WhiteBox.java and other
> classes, and uses the versions in jdk/test/whitebox.
> I used sed to change sun.hotspot.{gc,code,cpuinfo} to jdk.test.whitebox and
> deleted the old files and some references to sun.hotspot.
> Tested with tier1-4.
This looks good.
Thanks,
Serguei
-
Marked as reviewed by sspitsyn (Reviewer).
PR: https://git.openjdk.org/jdk/pull/9434
Re: RFR: 8288984: Simplification in java.lang.Runtime::exit [v2]
On Thu, 7 Jul 2022 08:03:52 GMT, David Holmes 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. The "that" I was referring to / disagreeing with is about the window for exit(non-zero), which I think won't ever open if any hook doesn't terminate. - PR: https://git.openjdk.org/jdk/pull/9351
Re: RFR: 8249834: java/util/ArrayList/Bug8146568.java and j/u/Vector/Bug8148174.java use @ignore w/o bug-id [v2]
On Fri, 8 Jul 2022 21:34:44 GMT, crazybones wrote: >> Bill Huang has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Added missing backslash in jdk/TEST.groups > > test/jdk/TEST.groups line 587: > >> 585: javax/net/ssl/compatibility/HrrTest.java \ >> 586: javax/net/ssl/compatibility/SniTest.java \ >> 587: jdk/nio/zipfs/TestLocOffsetFromZip64EF.java > > Shouldn't we add here a backslash at the end of the line? Good catch! I will add it back. Thank you for pointing out. - PR: https://git.openjdk.org/jdk/pull/9404
Re: RFR: 8249834: java/util/ArrayList/Bug8146568.java and j/u/Vector/Bug8148174.java use @ignore w/o bug-id [v2]
On Fri, 8 Jul 2022 21:50:40 GMT, Bill Huang wrote: >> 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. > > Bill Huang has updated the pull request incrementally with one additional > commit since the last revision: > > Added missing backslash in jdk/TEST.groups test/jdk/TEST.groups line 587: > 585: javax/net/ssl/compatibility/HrrTest.java \ > 586: javax/net/ssl/compatibility/SniTest.java \ > 587: jdk/nio/zipfs/TestLocOffsetFromZip64EF.java Shouldn't we add here a backslash at the end of the line? - PR: https://git.openjdk.org/jdk/pull/9404
Re: RFR: 8249834: java/util/ArrayList/Bug8146568.java and j/u/Vector/Bug8148174.java use @ignore w/o bug-id [v2]
> 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. Bill Huang has updated the pull request incrementally with one additional commit since the last revision: Added missing backslash in jdk/TEST.groups - Changes: - all: https://git.openjdk.org/jdk/pull/9404/files - new: https://git.openjdk.org/jdk/pull/9404/files/d37b084d..fc2d6bb0 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=9404&range=01 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=9404&range=00-01 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 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: 8275662: remove test/lib/sun/hotspot
On Fri, 8 Jul 2022 19:46:17 GMT, Coleen Phillimore wrote:
> This change removes the last remnants of sun/hotspot/WhiteBox.java and other
> classes, and uses the versions in jdk/test/whitebox.
> I used sed to change sun.hotspot.{gc,code,cpuinfo} to jdk.test.whitebox and
> deleted the old files and some references to sun.hotspot.
> Tested with tier1-4.
Thanks Misha!
-
PR: https://git.openjdk.org/jdk/pull/9434
Re: RFR: 8275662: remove test/lib/sun/hotspot
On Fri, 8 Jul 2022 19:46:17 GMT, Coleen Phillimore wrote:
> This change removes the last remnants of sun/hotspot/WhiteBox.java and other
> classes, and uses the versions in jdk/test/whitebox.
> I used sed to change sun.hotspot.{gc,code,cpuinfo} to jdk.test.whitebox and
> deleted the old files and some references to sun.hotspot.
> Tested with tier1-4.
Changes look good to me. Thank you.
-
Marked as reviewed by mseledtsov (Committer).
PR: https://git.openjdk.org/jdk/pull/9434
RFR: 8275662: remove test/lib/sun/hotspot
This change removes the last remnants of sun/hotspot/WhiteBox.java and other
classes, and uses the versions in jdk/test/whitebox.
I used sed to change sun.hotspot.{gc,code,cpuinfo} to jdk.test.whitebox and
deleted the old files and some references to sun.hotspot.
Tested with tier1-4.
-
Commit messages:
- 8275662: remove test/lib/sun/hotspot
Changes: https://git.openjdk.org/jdk/pull/9434/files
Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=9434&range=00
Issue: https://bugs.openjdk.org/browse/JDK-8275662
Stats: 1484 lines in 99 files changed: 0 ins; 1367 del; 117 mod
Patch: https://git.openjdk.org/jdk/pull/9434.diff
Fetch: git fetch https://git.openjdk.org/jdk pull/9434/head:pull/9434
PR: https://git.openjdk.org/jdk/pull/9434
[jdk19] Integrated: 8282071: Update java.xml module-info
On Fri, 8 Jul 2022 18:23:05 GMT, Joe Wang wrote: > Update module-info This pull request has now been integrated. Changeset: c86c51cc Author:Joe Wang URL: https://git.openjdk.org/jdk19/commit/c86c51cc72e3457756434b9150b0c5ef2f5d496d Stats: 6 lines in 1 file changed: 2 ins; 0 del; 4 mod 8282071: Update java.xml module-info Reviewed-by: lancea, iris, naoto - PR: https://git.openjdk.org/jdk19/pull/126
Re: RFR: 8265891: (ch) InputStream returned by Channels.newInputStream should override transferTo [v13]
On Sun, 1 Aug 2021 22:01:33 GMT, Markus KARG wrote: >> This PR-*draft* is **work in progress** and an invitation to discuss a >> possible solution for issue >> [JDK-8265891](https://bugs.openjdk.java.net/browse/JDK-8265891). It is *not >> yet* intended for a final review. >> >> As proposed in JDK-8265891, this PR provides an implementation for >> `Channels.newInputStream().transferTo()` which provide superior performance >> compared to the current implementation. The changes are: >> * Prevents transfers through the JVM heap as much as possibly by offloading >> to deeper levels via NIO, hence allowing the operating system to optimize >> the transfer. >> * Using more JRE heap in the fallback case when no NIO is possible (still >> only KiBs, hence mostl ynegligible even on SBCs) to better perform on modern >> hardware / fast I/O devides. >> >> Using JMH I have benchmarked both, the original implementation and this >> implementation, and (depending on the used hardware and use case) >> performance change was approx. doubled performance. So this PoC proofs that >> it makes sense to finalize this work and turn it into an actual OpenJDK >> contribution. >> >> I encourage everybody to discuss this draft: >> * Are there valid arguments for *not* doing this change? >> * Is there a *better* way to improve performance of >> `Channels.newInputStream().transferTo()`? >> * How to go on from here: What is missing to get this ready for an actual >> review? > > Markus KARG has updated the pull request incrementally with two additional > commits since the last revision: > > - Draft: Eliminated duplicate code using lambda expressions > - Draft: Use blocking mode also for target channel Please keep this PR open; I will continune work on it soon. - PR: https://git.openjdk.org/jdk/pull/4263
Re: [jdk19] RFR: 8282071: Update java.xml module-info
On Fri, 8 Jul 2022 18:23:05 GMT, Joe Wang wrote: > Update module-info Marked as reviewed by naoto (Reviewer). - PR: https://git.openjdk.org/jdk19/pull/126
Re: [jdk19] RFR: 8282071: Update java.xml module-info
On Fri, 8 Jul 2022 18:23:05 GMT, Joe Wang wrote: > Update module-info Marked as reviewed by iris (Reviewer). - PR: https://git.openjdk.org/jdk19/pull/126
Re: RFR: 8289797: tools/launcher/I18NArgTest.java fails on Japanese Windows enviromnent
On Wed, 6 Jul 2022 06:40:32 GMT, KIRIYAMA Takuya wrote: > 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? I agree with Jai here. It would be desirable to convert the incorrect test to something originally intended, ie., tests whether `JDK_JAVA_OPTIONS` env can correctly handle non-ASCII variables. - PR: https://git.openjdk.org/jdk/pull/9389
Re: [jdk19] RFR: 8282071: Update java.xml module-info
On Fri, 8 Jul 2022 18:23:05 GMT, Joe Wang wrote: > Update module-info Marked as reviewed by lancea (Reviewer). - PR: https://git.openjdk.org/jdk19/pull/126
[jdk19] RFR: 8282071: Update java.xml module-info
Update module-info - Commit messages: - 8282071: Update java.xml module-info Changes: https://git.openjdk.org/jdk19/pull/126/files Webrev: https://webrevs.openjdk.org/?repo=jdk19&pr=126&range=00 Issue: https://bugs.openjdk.org/browse/JDK-8282071 Stats: 6 lines in 1 file changed: 2 ins; 0 del; 4 mod Patch: https://git.openjdk.org/jdk19/pull/126.diff Fetch: git fetch https://git.openjdk.org/jdk19 pull/126/head:pull/126 PR: https://git.openjdk.org/jdk19/pull/126
Re: RFR: 8278469: Test java/nio/channels/FileChannel/LargeGatheringWrite.java times out
On Fri, 8 Jul 2022 16:55:28 GMT, Alan Bateman wrote: > My preference is to keep the unit tests in the original location and the only > tests in largeMemory are the tests that really need a lot of memory. Likewise. - PR: https://git.openjdk.org/jdk/pull/9416
Re: RFR: 8278469: Test java/nio/channels/FileChannel/LargeGatheringWrite.java times out
On Fri, 8 Jul 2022 15:15:03 GMT, Brian Burkhalter wrote: > We can see whether they both stop failing regularly after this change and if > so move `MapTest` out of the large memory list. I suspect it's a victim and shouldn't in the largeMemory directory but let's see if there are issues in the coming days. My preference is to keep the unit tests in the original location and the only tests in largeMemory are the tests that really need a lot of memory. - PR: https://git.openjdk.org/jdk/pull/9416
[jdk19] Integrated: 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. This pull request has now been integrated. Changeset: eeaf0bba Author:Stuart Marks URL: https://git.openjdk.org/jdk19/commit/eeaf0bbabc6632c181b191854678e72a333ec0a5 Stats: 3 lines in 3 files changed: 0 ins; 0 del; 3 mod 8289872: wrong wording in @param doc for HashMap.newHashMap et. al. Reviewed-by: chegar, naoto, iris - PR: https://git.openjdk.org/jdk19/pull/118
Re: RFR: 8289643: File descriptor leak with ProcessBuilder.startPipeline [v2]
> The `ProcessBuilder.pipelineStart()` implementation does not close all of the > file descriptors it uses to create the pipeline of processes. > > The process calling `pipelineStart()` is creating the pipes between the > stages. > As each process is launched, the file descriptor is inherited by the child > process and > the child process `dup`s them to the respective stdin/stdout/stderr fd. > These copies of inherited file descriptors are handled correctly. > > Between the launching of each Process, the file descriptor for the read-side > of the pipe for the output of the previous process is kept open (in the > parent process invoking `pipelineStart`). The file descriptor is correctly > passed to the child and is dup'd to the stdin of the next process. > > However, the open file descriptor in the parent is not closed after it has > been used as the input for the next Process. > The fix is to close the fd after it has been used as the input of the next > process. > > A new test verifies that after `pipelineStart` is complete, the same file > descriptors are open for Unix Pipes as before the test. > The test only runs on Linux using the /proc//fd filesystem to identify > open file descriptors. > > The bug fix is in `ProcessBuilder.pipelineStart` and is applicable to all > platforms. Roger Riggs has updated the pull request incrementally with one additional commit since the last revision: Cleanup of PipelineLeaksFD test improving error messages and source cleanup. - Changes: - all: https://git.openjdk.org/jdk/pull/9414/files - new: https://git.openjdk.org/jdk/pull/9414/files/3b154680..0d628d66 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=9414&range=01 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=9414&range=00-01 Stats: 70 lines in 2 files changed: 16 ins; 21 del; 33 mod Patch: https://git.openjdk.org/jdk/pull/9414.diff Fetch: git fetch https://git.openjdk.org/jdk pull/9414/head:pull/9414 PR: https://git.openjdk.org/jdk/pull/9414
Integrated: 8271707: migrate tests to use jdk.test.whitebox.WhiteBox
On Thu, 7 Jul 2022 20:43:09 GMT, Coleen Phillimore wrote: > This change uses sed to change sun.hotspot.WhiteBox to > jdk.test.whitebox.Whitebox, and sun/hotspot/Whitebox similarly. Due to > indirect inclusions of some of the test libraries, changing a few wasn't a > reliable option, and I need the new one for a different change I was looking > at. > The non-sed changes are for jdk/test/whitebox/WhiteBox to add some code for > GC that was only added to the sun version. > Also, the ClassFileInstaller has a label for sun.hotspot.Whitebox so that > didn't change with the edit. > Tested with tiers1-6. This pull request has now been integrated. Changeset: e7795851 Author:Coleen Phillimore URL: https://git.openjdk.org/jdk/commit/e7795851d2e02389e63950fef939084b18ec4bfb Stats: 2994 lines in 984 files changed: 6 ins; 0 del; 2988 mod 8271707: migrate tests to use jdk.test.whitebox.WhiteBox Reviewed-by: lmesnik, dholmes - PR: https://git.openjdk.org/jdk/pull/9417
[jdk19] Integrated: 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. This pull request has now been integrated. Changeset: 460d879a Author:Jorn Vernee URL: https://git.openjdk.org/jdk19/commit/460d879a75133fc071802bbc2c742b4232db604e Stats: 11 lines in 2 files changed: 11 ins; 0 del; 0 mod 8289601: SegmentAllocator::allocateUtf8String(String str) should be clarified for strings containing \0 Reviewed-by: psandoz, mcimadamore - PR: https://git.openjdk.org/jdk19/pull/107
Re: RFR: 8271707: migrate tests to use jdk.test.whitebox.WhiteBox
On Thu, 7 Jul 2022 20:43:09 GMT, Coleen Phillimore wrote: > This change uses sed to change sun.hotspot.WhiteBox to > jdk.test.whitebox.Whitebox, and sun/hotspot/Whitebox similarly. Due to > indirect inclusions of some of the test libraries, changing a few wasn't a > reliable option, and I need the new one for a different change I was looking > at. > The non-sed changes are for jdk/test/whitebox/WhiteBox to add some code for > GC that was only added to the sun version. > Also, the ClassFileInstaller has a label for sun.hotspot.Whitebox so that > didn't change with the edit. > Tested with tiers1-6. Thanks Leonid! - PR: https://git.openjdk.org/jdk/pull/9417
Re: RFR: 8289768: Clean up unused code [v2]
On Thu, 7 Jul 2022 19:06:52 GMT, Chris Plummer wrote: >> 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) > > src/jdk.hotspot.agent/linux/native/libsaproc/symtab.c line 308: > >> 306: ELF_SHDR* shbuf = NULL; >> 307: ELF_SHDR* cursct = NULL; >> 308: ELF_PHDR* phbuf = NULL; > > phbuf is also essentially unused. The only reference is to free it if > non-null, but it's always NULL, so that code can be removed too. Good catch! Also removed similar code from macOS version. - PR: https://git.openjdk.org/jdk/pull/9383
Re: RFR: 8271707: migrate tests to use jdk.test.whitebox.WhiteBox
On Thu, 7 Jul 2022 20:43:09 GMT, Coleen Phillimore wrote: > This change uses sed to change sun.hotspot.WhiteBox to > jdk.test.whitebox.Whitebox, and sun/hotspot/Whitebox similarly. Due to > indirect inclusions of some of the test libraries, changing a few wasn't a > reliable option, and I need the new one for a different change I was looking > at. > The non-sed changes are for jdk/test/whitebox/WhiteBox to add some code for > GC that was only added to the sun version. > Also, the ClassFileInstaller has a label for sun.hotspot.Whitebox so that > didn't change with the edit. > Tested with tiers1-6. I skimmed the diff and this seems fine. Are we not going to remove the old WhiteBox at the same time? Thanks. - Marked as reviewed by dholmes (Reviewer). PR: https://git.openjdk.org/jdk/pull/9417
Re: RFR: 8271707: migrate tests to use jdk.test.whitebox.WhiteBox
On Fri, 8 Jul 2022 06:12:18 GMT, David Holmes wrote: >> This change uses sed to change sun.hotspot.WhiteBox to >> jdk.test.whitebox.Whitebox, and sun/hotspot/Whitebox similarly. Due to >> indirect inclusions of some of the test libraries, changing a few wasn't a >> reliable option, and I need the new one for a different change I was looking >> at. >> The non-sed changes are for jdk/test/whitebox/WhiteBox to add some code for >> GC that was only added to the sun version. >> Also, the ClassFileInstaller has a label for sun.hotspot.Whitebox so that >> didn't change with the edit. >> Tested with tiers1-6. > > I skimmed the diff and this seems fine. > > Are we not going to remove the old WhiteBox at the same time? > > Thanks. @dholmes-ora We'll remove it with this patch JDK-8275662 and the other obsolete sun.hotspot test classes which will be easier to look at. Thanks for reviewing. - PR: https://git.openjdk.org/jdk/pull/9417
Re: RFR: 8289908: Skip bounds check for cases when String is constructed from entirely used byte[]
On Fri, 8 Jul 2022 07:37:36 GMT, Сергей Цыпанов wrote:
>> 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.
>
> But if I roll back the added constructor I'll go through existing public one
> `public String(byte[] bytes, int offset, int length, Charset charset)` doing
> bounds check twice, won't I?
The new constructor looks very odd, especially when it does not have an
explanation and doesn't describe the required preconditions for calling it. Is
there a better way than adding a non-functional argument?
The "unused" name is going to draw a warning from IntelliJ and some
enterprising developer is going to try to remove it, not understanding why its
there. And there is a bit of overhead pushing the extra arg.
The constructor should be private, it has a very narrow scope of use given the
lack of checking its args.
It would be nice if the Hotspot compiler would recognize the llmits on the
range and optimize away the checks;
it would have broader applicability then this point fix.
I would be interesting to ask the compiler folks if that's a future
optimization.
These source changes make it harder to understand what's happening where;
though that is sometimes work it for a noticeable performance improvement.
-
PR: https://git.openjdk.org/jdk/pull/9407
Re: [jdk19] Integrated: 8278469: Test java/nio/channels/FileChannel/LargeGatheringWrite.java times out
On Thu, 7 Jul 2022 22:25:10 GMT, Brian Burkhalter wrote: > Modify test directives so that `LargeGatheringWrite` and `MapTest` in > `java/nio/channels/FileChannel` are not run concurrently with other tests. > > Redirected from https://github.com/openjdk/jdk/pull/9416. Hello Brian, should `java/nio/channels/Channels/ReadXBytes.java` be treated similarly to prevent https://bugs.openjdk.org/browse/JDK-8289470? That test uses `-Xmx12G`, so probably would need to be running exclusively? - PR: https://git.openjdk.org/jdk19/pull/122
Re: RFR: 8278469: Test java/nio/channels/FileChannel/LargeGatheringWrite.java times out
On Fri, 8 Jul 2022 06:30:53 GMT, Alan Bateman wrote: > Are you sure MapTest needs a lot of memory? It's a unit test and doesn't have > anything in its jtreg test description to require a lot of resources. I > wonder if it's just the victim. We can see whether they both stop failing regularly after this change and if so move `MapTest` out of the large memory list. - PR: https://git.openjdk.org/jdk/pull/9416
Re: [jdk19] Integrated: 8278469: Test java/nio/channels/FileChannel/LargeGatheringWrite.java times out
On Fri, 8 Jul 2022 11:12:26 GMT, Jaikiran Pai wrote: > Hello Brian, should `java/nio/channels/Channels/ReadXBytes.java` be treated > similarly to prevent https://bugs.openjdk.org/browse/JDK-8289470? That test > uses `-Xmx12G`, so probably would need to be running exclusively? Probably. I had been considering because of this [prior comment](https://github.com/openjdk/jdk19/pull/122#issuecomment-1178864972). - PR: https://git.openjdk.org/jdk19/pull/122
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. Sorry for I already forgotten it. - PR: https://git.openjdk.org/jdk/pull/8302
Re: [jdk19] RFR: 8287809: Revisit implementation of memory session [v6]
On Wed, 6 Jul 2022 21:50:36 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: > > Turn non-closeable view back into MemorySession impl Marked as reviewed by jvernee (Reviewer). - PR: https://git.openjdk.org/jdk19/pull/22
Re: RFR: 8289643: File descriptor leak with ProcessBuilder.startPipeline
On Thu, 7 Jul 2022 19:08:35 GMT, Roger Riggs wrote:
> The `ProcessBuilder.pipelineStart()` implementation does not close all of the
> file descriptors it uses to create the pipeline of processes.
>
> The process calling `pipelineStart()` is creating the pipes between the
> stages.
> As each process is launched, the file descriptor is inherited by the child
> process and
> the child process `dup`s them to the respective stdin/stdout/stderr fd.
> These copies of inherited file descriptors are handled correctly.
>
> Between the launching of each Process, the file descriptor for the read-side
> of the pipe for the output of the previous process is kept open (in the
> parent process invoking `pipelineStart`). The file descriptor is correctly
> passed to the child and is dup'd to the stdin of the next process.
>
> However, the open file descriptor in the parent is not closed after it has
> been used as the input for the next Process.
> The fix is to close the fd after it has been used as the input of the next
> process.
>
> A new test verifies that after `pipelineStart` is complete, the same file
> descriptors are open for Unix Pipes as before the test.
> The test only runs on Linux using the /proc//fd filesystem to identify
> open file descriptors.
>
> The bug fix is in `ProcessBuilder.pipelineStart` and is applicable to all
> platforms.
Hello Roger, the change looks OK to me. The `ProcessBuilder` file will need a
copyright year update.
test/jdk/java/lang/ProcessBuilder/PipelineLeaksFD.java line 59:
> 57: * @requires (os.family == "linux" & !vm.musl)
> 58: * @summary file descriptor leak with ProcessBuilder.startPipeline
> 59: * @run testng PipelineLeaksFD
Should this use `othervm` to avoid any kind of interference while we are
counting the file descriptors in this test?
test/jdk/java/lang/ProcessBuilder/PipelineLeaksFD.java line 101:
> 99:
> 100: Set pipesAfter = myPipes();
> 101: if (!pipesBefore.equals(pipesAfter)) {
Since `myPipes()` can return null, should there be a null check here for
`pipesBefore`?
-
PR: https://git.openjdk.org/jdk/pull/9414
Re: RFR: 8289643: File descriptor leak with ProcessBuilder.startPipeline
On Fri, 8 Jul 2022 13:27:42 GMT, Alan Bateman wrote: >> The `ProcessBuilder.pipelineStart()` implementation does not close all of >> the file descriptors it uses to create the pipeline of processes. >> >> The process calling `pipelineStart()` is creating the pipes between the >> stages. >> As each process is launched, the file descriptor is inherited by the child >> process and >> the child process `dup`s them to the respective stdin/stdout/stderr fd. >> These copies of inherited file descriptors are handled correctly. >> >> Between the launching of each Process, the file descriptor for the read-side >> of the pipe for the output of the previous process is kept open (in the >> parent process invoking `pipelineStart`). The file descriptor is correctly >> passed to the child and is dup'd to the stdin of the next process. >> >> However, the open file descriptor in the parent is not closed after it has >> been used as the input for the next Process. >> The fix is to close the fd after it has been used as the input of the next >> process. >> >> A new test verifies that after `pipelineStart` is complete, the same file >> descriptors are open for Unix Pipes as before the test. >> The test only runs on Linux using the /proc//fd filesystem to identify >> open file descriptors. >> >> The bug fix is in `ProcessBuilder.pipelineStart` and is applicable to all >> platforms. > > test/jdk/java/lang/ProcessBuilder/PipelineLeaksFD.java line 59: > >> 57: * @requires (os.family == "linux" & !vm.musl) >> 58: * @summary file descriptor leak with ProcessBuilder.startPipeline >> 59: * @run testng PipelineLeaksFD > > I wonder if this test should be /othervm, just in case there are pipes left > over from previous tests that could interfere with the list found in /proc. Agreed - PR: https://git.openjdk.org/jdk/pull/9414
[jdk19] Integrated: 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 This pull request has now been integrated. Changeset: 732f1065 Author:Jorn Vernee URL: https://git.openjdk.org/jdk19/commit/732f1065fe05ae737a716bea92536cb8edc2b6a0 Stats: 22 lines in 7 files changed: 0 ins; 1 del; 21 mod 8289223: Canonicalize header ids in foreign API javadocs Reviewed-by: mcimadamore - PR: https://git.openjdk.org/jdk19/pull/75
Re: RFR: 8289797: tools/launcher/I18NArgTest.java fails on Japanese Windows enviromnent
On Wed, 6 Jul 2022 06:40:32 GMT, KIRIYAMA Takuya wrote:
> 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?
The change looks fine to me since in its current form the usage of
`JDK_JAVA_OPTIONS` is incorrect.
Having said that and looking at the history of this file, I think, this part of
the test was added to check if the `java` launcher would be able to read an
environment variable (specifically the `JDK_JAVA_OPTIONS`) whose value was in
`MS932` encoding, and pass it along as a correctly encoded String, all the way
to the main method of the application. If we remove this section of the test,
then we would be skipping that code path completely.
Perhaps this part of the test could be modified a bit to let it pass the
`JDK_JAVA_OPTIONS` environment variable with a `MS932` encoded value that acts
some option to the java launcher, instead of being the argument to the main
method? That way, you won't have to specify the main class name in the value of
the environment variable. Specifically, something like:
diff --git a/test/jdk/tools/launcher/I18NArgTest.java
b/test/jdk/tools/launcher/I18NArgTest.java
index fa09736da2f..2ba724d6f1d 100644
--- a/test/jdk/tools/launcher/I18NArgTest.java
+++ b/test/jdk/tools/launcher/I18NArgTest.java
@@ -97,12 +97,17 @@ public class I18NArgTest extends TestHelper {
// Test via JDK_JAVA_OPTIONS
Map env = new HashMap<>();
-String cmd = "-Dtest.src=" + TEST_SOURCES_DIR.getAbsolutePath() +
-" -Dtest.classes=" + TEST_CLASSES_DIR.getAbsolutePath() +
-" -cp " + TEST_CLASSES_DIR.getAbsolutePath() +
-" I18NArgTest " + unicodeStr + " " + hexValue;
-env.put("JDK_JAVA_OPTIONS", cmd);
-tr = doExec(env, javaCmd);
+String sysPropName = "foo.bar";
+// pass "-Dfoo.bar=" through the JDK_JAVA_OPTIONS
environment variable.
+// we expect that system property value to be passed along to the main
method with the
+// correct encoding
+String jdkJavaOpts = "-D" + sysPropName + "=" + unicodeStr;
+env.put("JDK_JAVA_OPTIONS", jdkJavaOpts);
+tr = doExec(env,javaCmd,
+"-Dtest.src=" + TEST_SOURCES_DIR.getAbsolutePath(),
+"-Dtest.classes=" + TEST_CLASSES_DIR.getAbsolutePath(),
+"-cp", TEST_CLASSES_DIR.getAbsolutePath(),
+"I18NArgTest", unicodeStr, hexValue, sysPropName);
System.out.println(tr.testOutput);
if (!tr.isOK()) {
System.err.println(tr);
@@ -125,5 +130,23 @@ public class I18NArgTest extends TestHelper {
"expected:" + expected + " obtained:" + hexValue;
throw new RuntimeException(message);
}
+if (args.length == 3) {
+// verify the value of the system property matches the expected
value
+String sysPropName = args[2];
+String sysPropVal = System.getProperty(sysPropName);
+if (sysPropVal == null) {
+throw new RuntimeException("Missing system property " +
sysPropName);
+}
+String sysPropHexVal = "";
+for (int i = 0; i < sysPropVal.length(); i++) {
+sysPropHexVal =
sysPropHexVal.concat(Integer.toHexString(sysPropVal.charAt(i)));
+}
+System.out.println("System property " + sysPropName + " computed
hex value: "
++ sysPropHexVal);
+if (!sysPropHexVal.equals(expected)) {
+throw new RuntimeException("Unexpected value in system
property, expected "
++ expected + ", but got " + sysPropHexVal);
+}
+}
}
}
I haven't tested this change, so you might have to experiment with it a bit.
What do you think?
-
PR: https://git.openjdk.org/jdk/pull/9389
Re: RFR: 8289643: File descriptor leak with ProcessBuilder.startPipeline
On Thu, 7 Jul 2022 19:08:35 GMT, Roger Riggs wrote:
> The `ProcessBuilder.pipelineStart()` implementation does not close all of the
> file descriptors it uses to create the pipeline of processes.
>
> The process calling `pipelineStart()` is creating the pipes between the
> stages.
> As each process is launched, the file descriptor is inherited by the child
> process and
> the child process `dup`s them to the respective stdin/stdout/stderr fd.
> These copies of inherited file descriptors are handled correctly.
>
> Between the launching of each Process, the file descriptor for the read-side
> of the pipe for the output of the previous process is kept open (in the
> parent process invoking `pipelineStart`). The file descriptor is correctly
> passed to the child and is dup'd to the stdin of the next process.
>
> However, the open file descriptor in the parent is not closed after it has
> been used as the input for the next Process.
> The fix is to close the fd after it has been used as the input of the next
> process.
>
> A new test verifies that after `pipelineStart` is complete, the same file
> descriptors are open for Unix Pipes as before the test.
> The test only runs on Linux using the /proc//fd filesystem to identify
> open file descriptors.
>
> The bug fix is in `ProcessBuilder.pipelineStart` and is applicable to all
> platforms.
src/java.base/share/classes/java/lang/ProcessBuilder.java line 1301:
> 1299: if (prevOutput instanceof RedirectPipeImpl redir) {
> 1300: // Wrap the fd so it can be closed
> 1301: new Process.PipeInputStream(redir.getFd()).close();
It looks unusual to need to create a new input stream to invoke close but I
think this is okay here.
test/jdk/java/lang/ProcessBuilder/PipelineLeaksFD.java line 59:
> 57: * @requires (os.family == "linux" & !vm.musl)
> 58: * @summary file descriptor leak with ProcessBuilder.startPipeline
> 59: * @run testng PipelineLeaksFD
I wonder if this test should be /othervm, just in case there are pipes left
over from previous tests that could interfere with the list found in /proc.
-
PR: https://git.openjdk.org/jdk/pull/9414
RFR: Merge jdk19
Forwardport JDK 19 -> JDK 20
-
Commit messages:
- Merge
- 8289486: Improve XSLT XPath operators count efficiency
- 8289779: Map::replaceAll javadoc has redundant @throws clauses
- 8289558: Need spec clarification of j.l.foreign.*Layout
- 8289196: Pattern domination not working properly for record patterns
- 6509045: {@inheritDoc} only copies one instance of the specified exception
- 8288949:
serviceability/jvmti/vthread/ContStackDepthTest/ContStackDepthTest.java failing
- 8289857: ProblemList jdk/jfr/event/runtime/TestActiveSettingEvent.java
- 8289840: ProblemList
vmTestbase/nsk/jdwp/ThreadReference/ForceEarlyReturn/forceEarlyReturn002/forceEarlyReturn002.java
when run with vthread wrapper
- 8289841: ProblemList vmTestbase/gc/gctests/MemoryEaterMT/MemoryEaterMT.java
with ZGC on windows
The webrevs contain the adjustments done while merging with regards to each
parent branch:
- master: https://webrevs.openjdk.org/?repo=jdk&pr=9419&range=00.0
- jdk19: https://webrevs.openjdk.org/?repo=jdk&pr=9419&range=00.1
Changes: https://git.openjdk.org/jdk/pull/9419/files
Stats: 807 lines in 28 files changed: 669 ins; 52 del; 86 mod
Patch: https://git.openjdk.org/jdk/pull/9419.diff
Fetch: git fetch https://git.openjdk.org/jdk pull/9419/head:pull/9419
PR: https://git.openjdk.org/jdk/pull/9419
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. Marked as reviewed by iris (Reviewer). - PR: https://git.openjdk.org/jdk19/pull/118
Re: RFR: 8289768: Clean up unused code [v3]
On Fri, 8 Jul 2022 07:08:46 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 incrementally with two > additional commits since the last revision: > > - Update copyright > - Remove more unused variables Copyrights updated. - PR: https://git.openjdk.org/jdk/pull/9383
Re: RFR: 8289908: Skip bounds check for cases when String is constructed from entirely used byte[]
On Thu, 7 Jul 2022 18:45:03 GMT, Roger Riggs 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.
But if I roll back the added constructor I'll go through existing public one
`public String(byte[] bytes, int offset, int length, Charset charset)` doing
bounds check twice, won't I?
-
PR: https://git.openjdk.org/jdk/pull/9407
Withdrawn: 8278469: Test java/nio/channels/FileChannel/LargeGatheringWrite.java times out
On Thu, 7 Jul 2022 20:36:43 GMT, Brian Burkhalter wrote: > Modify test directives so that `LargeGatheringWrite` and `MapTest` in > `java/nio/channels/FileChannel` are not run concurrently with other tests. This pull request has been closed without being integrated. - PR: https://git.openjdk.org/jdk/pull/9416
Re: [jdk19] Integrated: 8278469: Test java/nio/channels/FileChannel/LargeGatheringWrite.java times out
On Thu, 7 Jul 2022 22:25:10 GMT, Brian Burkhalter wrote: > Modify test directives so that `LargeGatheringWrite` and `MapTest` in > `java/nio/channels/FileChannel` are not run concurrently with other tests. > > Redirected from https://github.com/openjdk/jdk/pull/9416. Thumbs up. This is a trivial fix. - Marked as reviewed by dcubed (Reviewer). PR: https://git.openjdk.org/jdk19/pull/122
Re: RFR: 8289768: Clean up unused code [v3]
> 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 incrementally with two additional commits since the last revision: - Update copyright - Remove more unused variables - Changes: - all: https://git.openjdk.org/jdk/pull/9383/files - new: https://git.openjdk.org/jdk/pull/9383/files/b4cd5374..da30ef90 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=9383&range=02 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=9383&range=01-02 Stats: 30 lines in 26 files changed: 0 ins; 4 del; 26 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: 8289030: [macos] app image signature invalid when creating DMG or PKG [v3]
On Thu, 7 Jul 2022 19:52:57 GMT, Alexey Semenyuk wrote:
>> We need to add `.package` file during app image creation, since we need to
>> sign it. With your proposed change we will add `.package` file to already
>> signed app image.
>
> Oh, right.
> Anyways let's keep `.package`-related stuff away from
> AbstractAppImageBuilder.java, and AppImageBundler.java.
>
> I'd move `.package`-related logic from AbstractAppImageBuilder to
> MacAppImageBuilder and change
>
> public MacAppBundler() {
> setAppImageSupplier(MacAppImageBuilder::new);
> setParamsValidator(MacAppBundler::doValidate);
> }
>
> to something like this
>
> public MacAppBundler() {
>public MacAppBundler() {
> setAppImageSupplier(imageOutDir -> {
> return new MacAppImageBuilder(imageOutDir, isDependentTask());
> });
> setParamsValidator(MacAppBundler::doValidate);
> }
>
>
> Need to add `AppImageBundler.sDependentTask()` method and change signature if
> MacAppImageBuilder ctor from `MacAppImageBuilder(Path imageOutDir)` to
> `MacAppImageBuilder(Path imageOutDir, boolean withPackageFile)`
Fixed.
-
PR: https://git.openjdk.org/jdk19/pull/89
Re: RFR: 8289834: Add SBCS and DBCS Only EBCDIC charsets
And also there is no reason why db drivers or host connectors should not ship their own charset support (Oracle JDBC for example had nls_charset addons. My employer also ship a custom EBCDIC encoding which includes some compatibility hacks, and that took some effort to adopt it to the missing ext mechanism). Having said that, with JPMS a ‚legacy ebcdic‘ encoding module would be possible while still being optional. Maybe in the future a mechanism for modules which can be added (instead of removed) from standard distribution would make that nicer? Is there a performance restriction for charset if they are not part of a platform module (optimized string access)? Gruss Bernd -- http://bernd.eckenfels.net Von: core-libs-dev im Auftrag von Alan Bateman Gesendet: Thursday, July 7, 2022 11:50:39 AM An: build-...@openjdk.org ; core-libs-dev@openjdk.org ; i18n-...@openjdk.org Betreff: 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: 8278469: Test java/nio/channels/FileChannel/LargeGatheringWrite.java times out
On Thu, 7 Jul 2022 20:36:43 GMT, Brian Burkhalter wrote: > Modify test directives so that `LargeGatheringWrite` and `MapTest` in > `java/nio/channels/FileChannel` are not run concurrently with other tests. Thumbs up. I didn't realize that other test dirs already had the mechanism in place to isolate the resource hogs. I'm looking forward to seeing this in action. I just realized this is targeted at jdk/jdk (JDK20). Can you please close this PR and create a new one for JDK19? - Marked as reviewed by dcubed (Reviewer). PR: https://git.openjdk.org/jdk/pull/9416
Integrated: Merge jdk19
On Thu, 7 Jul 2022 22:31:27 GMT, Jesper Wilhelmsson wrote: > Forwardport JDK 19 -> JDK 20 This pull request has now been integrated. Changeset: 01b9f95c Author:Jesper Wilhelmsson URL: https://git.openjdk.org/jdk/commit/01b9f95c62953e7f9ca10eafd42d21c634413827 Stats: 807 lines in 28 files changed: 669 ins; 52 del; 86 mod Merge - PR: https://git.openjdk.org/jdk/pull/9419
RFR: Merge jdk19
Forwardport JDK 19 -> JDK 20
-
Commit messages:
- Merge
- 8289486: Improve XSLT XPath operators count efficiency
- 8289779: Map::replaceAll javadoc has redundant @throws clauses
- 8289558: Need spec clarification of j.l.foreign.*Layout
- 8289196: Pattern domination not working properly for record patterns
- 6509045: {@inheritDoc} only copies one instance of the specified exception
- 8288949:
serviceability/jvmti/vthread/ContStackDepthTest/ContStackDepthTest.java failing
- 8289857: ProblemList jdk/jfr/event/runtime/TestActiveSettingEvent.java
- 8289840: ProblemList
vmTestbase/nsk/jdwp/ThreadReference/ForceEarlyReturn/forceEarlyReturn002/forceEarlyReturn002.java
when run with vthread wrapper
- 8289841: ProblemList vmTestbase/gc/gctests/MemoryEaterMT/MemoryEaterMT.java
with ZGC on windows
The webrevs contain the adjustments done while merging with regards to each
parent branch:
- master: https://webrevs.openjdk.org/?repo=jdk&pr=9415&range=00.0
- jdk19: https://webrevs.openjdk.org/?repo=jdk&pr=9415&range=00.1
Changes: https://git.openjdk.org/jdk/pull/9415/files
Stats: 803 lines in 28 files changed: 669 ins; 48 del; 86 mod
Patch: https://git.openjdk.org/jdk/pull/9415.diff
Fetch: git fetch https://git.openjdk.org/jdk pull/9415/head:pull/9415
PR: https://git.openjdk.org/jdk/pull/9415
Re: RFR: 8271707: migrate tests to use jdk.test.whitebox.WhiteBox
On Thu, 7 Jul 2022 20:43:09 GMT, Coleen Phillimore wrote: > This change uses sed to change sun.hotspot.WhiteBox to > jdk.test.whitebox.Whitebox, and sun/hotspot/Whitebox similarly. Due to > indirect inclusions of some of the test libraries, changing a few wasn't a > reliable option, and I need the new one for a different change I was looking > at. > The non-sed changes are for jdk/test/whitebox/WhiteBox to add some code for > GC that was only added to the sun version. > Also, the ClassFileInstaller has a label for sun.hotspot.Whitebox so that > didn't change with the edit. > Tested with tiers1-6. Marked as reviewed by lmesnik (Reviewer). - PR: https://git.openjdk.org/jdk/pull/9417
RFR: 8278469: Test java/nio/channels/FileChannel/LargeGatheringWrite.java times out
Modify test directives so that `LargeGatheringWrite` and `MapTest` in `java/nio/channels/FileChannel` are not run concurrently with other tests. - Commit messages: - 8278469: Test java/nio/channels/FileChannel/LargeGatheringWrite.java times out Changes: https://git.openjdk.org/jdk/pull/9416/files Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=9416&range=00 Issue: https://bugs.openjdk.org/browse/JDK-8278469 Stats: 2 lines in 3 files changed: 1 ins; 0 del; 1 mod Patch: https://git.openjdk.org/jdk/pull/9416.diff Fetch: git fetch https://git.openjdk.org/jdk pull/9416/head:pull/9416 PR: https://git.openjdk.org/jdk/pull/9416
[jdk19] Integrated: 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. This pull request has now been integrated. Changeset: 64286074 Author:Alexander Matveev URL: https://git.openjdk.org/jdk19/commit/64286074ba763d4a1e8879d8af69eee34d32cfa6 Stats: 221 lines in 14 files changed: 194 ins; 6 del; 21 mod 8289030: [macos] app image signature invalid when creating DMG or PKG Reviewed-by: asemenyuk - PR: https://git.openjdk.org/jdk19/pull/89
[jdk19] RFR: 8289930: Improve Thread description of inherited AccessControlContext
This is javadoc only change. The "Inheritance when creating threads" section in the Thread description lists the things that are inherited when creating a Thread. This includes the somewhat obscure "inherited AccessControlContext" where the first sentence looks like it wants to be a heading. The proposal change is tiny, it stops the first sentence/prefix "Inherited Access Control Context:" and replaces it with the sentence "The captured caller context is the new thread's inherited AccessControlContext". When the legacy SecurityManager is degraded further then I expect this paragraph will be removed. No semantic changes and I think below the radar for needing a csr. - Commit messages: - Initial commit Changes: https://git.openjdk.org/jdk19/pull/121/files Webrev: https://webrevs.openjdk.org/?repo=jdk19&pr=121&range=00 Issue: https://bugs.openjdk.org/browse/JDK-8289930 Stats: 7 lines in 1 file changed: 1 ins; 1 del; 5 mod Patch: https://git.openjdk.org/jdk19/pull/121.diff Fetch: git fetch https://git.openjdk.org/jdk19 pull/121/head:pull/121 PR: https://git.openjdk.org/jdk19/pull/121
Re: RFR: 8289768: Clean up unused code [v3]
On Fri, 8 Jul 2022 07:08:46 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 incrementally with two > additional commits since the last revision: > > - Update copyright > - Remove more unused variables Marked as reviewed by alanb (Reviewer). src/java.base/windows/native/libnio/fs/WindowsNativeDispatcher.c line 686: > 684: LPCWSTR lpFileName = jlong_to_ptr(pathAddress); > 685: PSECURITY_DESCRIPTOR pSecurityDescriptor = jlong_to_ptr(descAddress); > 686: DWORD lengthNeeded = 0; lengthNeeded, it seems not :-) - PR: https://git.openjdk.org/jdk/pull/9383
Re: RFR: 8289768: Clean up unused code [v3]
On Fri, 8 Jul 2022 07:08:46 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 incrementally with two > additional commits since the last revision: > > - Update copyright > - Remove more unused variables Marked as reviewed by cjplummer (Reviewer). - PR: https://git.openjdk.org/jdk/pull/9383
Re: RFR: 8278469: Test java/nio/channels/FileChannel/LargeGatheringWrite.java times out
On Thu, 7 Jul 2022 20:36:43 GMT, Brian Burkhalter wrote: > Modify test directives so that `LargeGatheringWrite` and `MapTest` in > `java/nio/channels/FileChannel` are not run concurrently with other tests. Are you sure MapTest needs a lot of memory? It's a unit test and doesn't have anything in its jtreg test description to require a lot of resources. I wonder if it's just the victim. - PR: https://git.openjdk.org/jdk/pull/9416
[jdk19] Integrated: 8278469: Test java/nio/channels/FileChannel/LargeGatheringWrite.java times out
On Thu, 7 Jul 2022 22:25:10 GMT, Brian Burkhalter wrote: > Modify test directives so that `LargeGatheringWrite` and `MapTest` in > `java/nio/channels/FileChannel` are not run concurrently with other tests. > > Redirected from https://github.com/openjdk/jdk/pull/9416. This pull request has now been integrated. Changeset: 11319c2a Author:Brian Burkhalter URL: https://git.openjdk.org/jdk19/commit/11319c2aeb16ef2feb0ecab0e2811a52e845739d Stats: 2 lines in 3 files changed: 1 ins; 0 del; 1 mod 8278469: Test java/nio/channels/FileChannel/LargeGatheringWrite.java times out 8289526: java/nio/channels/FileChannel/MapTest.java times out Reviewed-by: dcubed - PR: https://git.openjdk.org/jdk19/pull/122
Re: RFR: 8278469: Test java/nio/channels/FileChannel/LargeGatheringWrite.java times out
On Thu, 7 Jul 2022 22:04:09 GMT, Daniel D. Daugherty wrote: > I just realized this is targeted at jdk/jdk (JDK20). Can you please close > this PR and create a new one for JDK19? Done. - PR: https://git.openjdk.org/jdk/pull/9416
Re: [jdk19] Integrated: 8278469: Test java/nio/channels/FileChannel/LargeGatheringWrite.java times out
On Thu, 7 Jul 2022 22:25:10 GMT, Brian Burkhalter wrote: > Modify test directives so that `LargeGatheringWrite` and `MapTest` in > `java/nio/channels/FileChannel` are not run concurrently with other tests. > > Redirected from https://github.com/openjdk/jdk/pull/9416. It is conjectured that several tests with large memory demands such as these could be failing due to insufficient memory per core on the test node when the tests are run simultaneously. - PR: https://git.openjdk.org/jdk19/pull/122
Withdrawn: Merge jdk19
On Thu, 7 Jul 2022 20:14:12 GMT, Jesper Wilhelmsson wrote: > Forwardport JDK 19 -> JDK 20 This pull request has been closed without being integrated. - PR: https://git.openjdk.org/jdk/pull/9415
Re: [jdk19] RFR: 8289030: [macos] app image signature invalid when creating DMG or PKG [v3]
> 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 incrementally with one additional commit since the last revision: 8289030: [macos] app image signature invalid when creating DMG or PKG [v3] - Changes: - all: https://git.openjdk.org/jdk19/pull/89/files - new: https://git.openjdk.org/jdk19/pull/89/files/4d5d4bce..a84c725d Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk19&pr=89&range=02 - incr: https://webrevs.openjdk.org/?repo=jdk19&pr=89&range=01-02 Stats: 37 lines in 7 files changed: 15 ins; 14 del; 8 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: 8278469: Test java/nio/channels/FileChannel/LargeGatheringWrite.java times out
On Thu, 7 Jul 2022 20:36:43 GMT, Brian Burkhalter wrote: > Modify test directives so that `LargeGatheringWrite` and `MapTest` in > `java/nio/channels/FileChannel` are not run concurrently with other tests. It is conjectured that several tests with large memory demands such as these could be failing due to insufficient memory per core on the test node when the tests are run simultaneously. - PR: https://git.openjdk.org/jdk/pull/9416
Re: RFR: 8278469: Test java/nio/channels/FileChannel/LargeGatheringWrite.java times out
On Thu, 7 Jul 2022 20:36:43 GMT, Brian Burkhalter wrote: > Modify test directives so that `LargeGatheringWrite` and `MapTest` in > `java/nio/channels/FileChannel` are not run concurrently with other tests. Superseded by https://github.com/openjdk/jdk19/pull/122 - PR: https://git.openjdk.org/jdk/pull/9416
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. Marked as reviewed by naoto (Reviewer). - PR: https://git.openjdk.org/jdk19/pull/118
Re: RFR: Merge jdk19 [v2]
> Forwardport JDK 19 -> JDK 20 Jesper Wilhelmsson has updated the pull request incrementally with one additional commit since the last revision: Fix merge error - Changes: - all: https://git.openjdk.org/jdk/pull/9415/files - new: https://git.openjdk.org/jdk/pull/9415/files/0f86db4f..c6949b8f Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=9415&range=01 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=9415&range=00-01 Stats: 4 lines in 1 file changed: 0 ins; 4 del; 0 mod Patch: https://git.openjdk.org/jdk/pull/9415.diff Fetch: git fetch https://git.openjdk.org/jdk pull/9415/head:pull/9415 PR: https://git.openjdk.org/jdk/pull/9415
RFR: 8271707: migrate tests to use jdk.test.whitebox.WhiteBox
This change uses sed to change sun.hotspot.WhiteBox to jdk.test.whitebox.Whitebox, and sun/hotspot/Whitebox similarly. Due to indirect inclusions of some of the test libraries, changing a few wasn't a reliable option, and I need the new one for a different change I was looking at. The non-sed changes are for jdk/test/whitebox/WhiteBox to add some code for GC that was only added to the sun version. Also, the ClassFileInstaller has a label for sun.hotspot.Whitebox so that didn't change with the edit. Tested with tiers1-6. - Commit messages: - 8271707: migrate tests to use jdk.test.whitebox.WhiteBox Changes: https://git.openjdk.org/jdk/pull/9417/files Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=9417&range=00 Issue: https://bugs.openjdk.org/browse/JDK-8271707 Stats: 2995 lines in 984 files changed: 6 ins; 0 del; 2989 mod Patch: https://git.openjdk.org/jdk/pull/9417.diff Fetch: git fetch https://git.openjdk.org/jdk pull/9417/head:pull/9417 PR: https://git.openjdk.org/jdk/pull/9417
[jdk19] Integrated: 8278469: Test java/nio/channels/FileChannel/LargeGatheringWrite.java times out
Modify test directives so that `LargeGatheringWrite` and `MapTest` in `java/nio/channels/FileChannel` are not run concurrently with other tests. Redirected from https://github.com/openjdk/jdk/pull/9416. - Commit messages: - 8278469: Test java/nio/channels/FileChannel/LargeGatheringWrite.java times out Changes: https://git.openjdk.org/jdk19/pull/122/files Webrev: https://webrevs.openjdk.org/?repo=jdk19&pr=122&range=00 Issue: https://bugs.openjdk.org/browse/JDK-8278469 Stats: 2 lines in 3 files changed: 1 ins; 0 del; 1 mod Patch: https://git.openjdk.org/jdk19/pull/122.diff Fetch: git fetch https://git.openjdk.org/jdk19 pull/122/head:pull/122 PR: https://git.openjdk.org/jdk19/pull/122
Re: [jdk19] RFR: 8289030: [macos] app image signature invalid when creating DMG or PKG [v2]
On Thu, 7 Jul 2022 17:11:23 GMT, Alexey Semenyuk wrote:
>> 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
>
> test/jdk/tools/jpackage/macosx/SigningPackageTwoStepTest.java line 87:
>
>> 85: private static void verifyAppImageInDMG(JPackageCommand cmd) {
>> 86: MacHelper.withExplodedDmg(cmd, dmgImage -> {
>> 87: Path launcherPath = dmgImage.resolve(Path.of("Contents",
>> "MacOS", cmd.name()));
>
> I'd replace it with
> `ApplicationLayout.macAppImage().resolveAt(dmgImage).launchersDirectory(cmd.name())`
Fixed.
-
PR: https://git.openjdk.org/jdk19/pull/89
Re: [jdk19] RFR: 8289030: [macos] app image signature invalid when creating DMG or PKG [v3]
On Fri, 8 Jul 2022 00:14:27 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. > > Alexander Matveev has updated the pull request incrementally with one > additional commit since the last revision: > > 8289030: [macos] app image signature invalid when creating DMG or PKG [v3] Marked as reviewed by asemenyuk (Reviewer). - PR: https://git.openjdk.org/jdk19/pull/89
Re: RFR: JDK-8266431: Dual-Pivot Quicksort improvements (Radix sort) [v14]
On Thu, 7 Jul 2022 13:41:17 GMT, Alan Bateman wrote: >> 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 Arrays.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 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. @AlanBateman, I agree that current Arrays.sort() is not simple as it was in JDK 6 with classical Quicksort and insertion sort only. Quicksort (Dual-Pivot Quicksort) works faster than other algorithms but, unfortunately, not on all types of data. For example, if we process sorted or almost sorted data, the winner is merging sort which takes into account already sorted sub-sequences. It is 2-5 times faster, and this advantage can't be ignored. We have heap sort in our "zoo". Many years ago I had discussion with one of the participants of an programming competition. The goal of such competition is to write code which solves given tasks as quickly as possible. The problem was that their code on specific data showed quadratic time because of Quicksort from Arrays.sort(). Therefore, heap sort was added to guarantee n*log(n) complexity on any data. For small-ranged values, like char, short and byte, counting sort works much faster, and it was added also. Another case is computers with many processors, where parallel merge sort is much faster. Of course, we can stay with few algorithms only, but JDK, as world class software, must have perfect implementation. Last step is to add Radix sort which works 5-8 times faster than Quicksort (on 1m elements), and this fact can't be ignored as well. There were several suggestions to introduce Radix sort as a good algorithm for digits here. I can say that each algorithm is on own place and plays key role. Need to add that this PR contains not Radix sort only, but significant improvements of parallel sorting, insertion sorts and merging sort. The fastest implementation of Radix sort is LSD (Least Significant Digit) variant with additional buffer. Our current implementation of sorting also uses additional buffer for merging sort and parallel sort. So, additional buffer is not new. If we try to sort huge array with limited memory now, we get OOME and sorting will not be completed correctly. Suggested PR solves this problem: we switch to in-place algorithms and sorting will not fail. We set max limit for additional buffer, it is applied for all sorts (parallel, merging, radix). Also additional buffer will be reused by merging and radix sort in case of parallel sorting. It means we don't request multiple buffers (#threads * size) at the same time. In few words, current PR doesn't make Arrays.sort() more complicated, but improves performance and memory allocation. - PR: https://git.openjdk.org/jdk/pull/3938
Re: RFR: JDK-8266431: Dual-Pivot Quicksort improvements (Radix sort) [v15]
> 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) * Added JMH - Changes: - all: https://git.openjdk.org/jdk/pull/3938/files - new: https://git.openjdk.org/jdk/pull/3938/files/1cb6fd7a..618bdb5f Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=3938&range=14 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=3938&range=13-14 Stats: 584 lines in 3 files changed: 583 ins; 0 del; 1 mod Patch: https://git.openjdk.org/jdk/pull/3938.diff Fetch: git fetch https://git.openjdk.org/jdk pull/3938/head:pull/3938 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) Are you going to update copyrights? I reviewed src/jdk.hotspot.agent, src/jdk.jdi/, src/jdk.jdwp.agent, and src/jdk.management. Looks good other than copyrights and the suggestion I made for additional cleanup in symtab.c. src/jdk.hotspot.agent/linux/native/libsaproc/symtab.c line 308: > 306: ELF_SHDR* shbuf = NULL; > 307: ELF_SHDR* cursct = NULL; > 308: ELF_PHDR* phbuf = NULL; phbuf is also essentially unused. The only reference is to free it if non-null, but it's always NULL, so that code can be removed too. - Changes requested by cjplummer (Reviewer). PR: https://git.openjdk.org/jdk/pull/9383
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 I second Alan's comment here. I am not sure we could justify those legacy less common EBCDIC charsets making into the `jdk.charsets` module. Instead, utilizing the charset provider mechanism will better fit here to me. - PR: https://git.openjdk.org/jdk/pull/9399
Re: RFR: 8205592: BigDecimal.doubleValue() is depressingly slow
On Thu, 7 Jul 2022 15:20:32 GMT, Raffaello Giulietti wrote: > A reimplementation of `BigDecimal.[double|float]Value()` to enhance > performance, avoiding an intermediate string and its subsequent parsing on > the slow path. These are the improvements over the current implementation: - Much more cases are processed by the fast path. - Most values that will either produce 0 or infinity are detected early in a fast way to avoid expensive computations. - If neither of the above applies, the conversion to `String` and subsequent parsing, as currently done, is replaced by `BigInteger` arithmetic. There's at most one division between `BigInteger`s. Of course, no need for `toString()` nor parsing. - Extensive comments explain all the details. JMH benchmarks show that, on the fast path, the new implementation is on par or way better (>200x for the cases not currently covered). Cases where 0 or infinity are detected early contribute with speedup factors of >200x. `BigInteger` arithmetic contributes with speed factors of 2x-8x on "typical" `BigDecimal`s of precision 18 to 24 and scale 2 to 6. - PR: https://git.openjdk.org/jdk/pull/9410
Re: [jdk19] RFR: 8289030: [macos] app image signature invalid when creating DMG or PKG [v2]
On Thu, 7 Jul 2022 07:27:56 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.
>
> 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 requested by asemenyuk (Reviewer).
src/jdk.jpackage/macosx/classes/jdk/jpackage/internal/MacBaseInstallerBundler.java
line 171:
> 169:Files.deleteIfExists(AppImageFile.getPathInAppImage(appDir));
> 170: }
> 171:
I think there is no need to modify AbstractAppImageBuilder.java, and
AppImageBundler.java.
It is sufficient to modify the condition controlling the creation of `.package`
file:
if (predefinedImage == null ||
(!StandardBundlerParam.isRuntimeInstaller(params) &&
!AppImageFile.load(predefinedImage).isSigned())) {
new PackageFile(APP_NAME.fetchFrom(params)).save(
ApplicationLayout.macAppImage().resolveAt(appDir));
Files.deleteIfExists(AppImageFile.getPathInAppImage(appDir));
}
Besides `.package` file logically doesn't belong to app image, it belongs to
the installed application, so it must not be referenced from the classes
creating app images.
test/jdk/tools/jpackage/macosx/SigningPackageTwoStepTest.java line 87:
> 85: private static void verifyAppImageInDMG(JPackageCommand cmd) {
> 86: MacHelper.withExplodedDmg(cmd, dmgImage -> {
> 87: Path launcherPath = dmgImage.resolve(Path.of("Contents",
> "MacOS", cmd.name()));
I'd replace it with
`ApplicationLayout.macAppImage().resolveAt(dmgImage).launchersDirectory(cmd.name())`
-
PR: https://git.openjdk.org/jdk19/pull/89
Re: [jdk19] RFR: 8289030: [macos] app image signature invalid when creating DMG or PKG [v2]
On Thu, 7 Jul 2022 19:30:07 GMT, Alexander Matveev wrote:
>> src/jdk.jpackage/macosx/classes/jdk/jpackage/internal/MacBaseInstallerBundler.java
>> line 171:
>>
>>> 169:
>>> Files.deleteIfExists(AppImageFile.getPathInAppImage(appDir));
>>> 170: }
>>> 171:
>>
>> I think there is no need to modify AbstractAppImageBuilder.java, and
>> AppImageBundler.java.
>> It is sufficient to modify the condition controlling the creation of
>> `.package` file:
>>
>> if (predefinedImage == null ||
>> (!StandardBundlerParam.isRuntimeInstaller(params) &&
>> !AppImageFile.load(predefinedImage).isSigned())) {
>> new PackageFile(APP_NAME.fetchFrom(params)).save(
>> ApplicationLayout.macAppImage().resolveAt(appDir));
>> Files.deleteIfExists(AppImageFile.getPathInAppImage(appDir));
>> }
>>
>> Besides `.package` file logically doesn't belong to app image, it belongs to
>> the installed application, so it must not be referenced from the classes
>> creating app images.
>
> We need to add `.package` file during app image creation, since we need to
> sign it. With your proposed change we will add `.package` file to already
> signed app image.
Oh, right.
Anyways let's keep `.package`-related stuff away from
AbstractAppImageBuilder.java, and AppImageBundler.java.
I'd move `.package`-related logic from AbstractAppImageBuilder to
MacAppImageBuilder and change
public MacAppBundler() {
setAppImageSupplier(MacAppImageBuilder::new);
setParamsValidator(MacAppBundler::doValidate);
}
to something like this
public MacAppBundler() {
public MacAppBundler() {
setAppImageSupplier(imageOutDir -> {
return new MacAppImageBuilder(imageOutDir, isDependentTask());
});
setParamsValidator(MacAppBundler::doValidate);
}
Need to add `AppImageBundler.sDependentTask()` method and change signature if
MacAppImageBuilder ctor from `MacAppImageBuilder(Path imageOutDir)` to
`MacAppImageBuilder(Path imageOutDir, boolean withPackageFile)`
-
PR: https://git.openjdk.org/jdk19/pull/89
Re: RFR: JDK-8266431: Dual-Pivot Quicksort improvements (Radix sort) [v14]
On Wed, 6 Jul 2022 08:49:45 GMT, Сергей Цыпанов wrote: >> 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` @stsypanov I added JMH test, see class org.openjdk.bench.java.util,ArraysSort.java I performed benchmarking and compared current version from JDK and new implementation, results as well as sources can be found here https://github.com/iaroslavski/sorting/tree/master/micro/result New version shows better performance. Please, let me know if you have questions / comments. - PR: https://git.openjdk.org/jdk/pull/3938
RFR: 8205592: BigDecimal.doubleValue() is depressingly slow
A reimplementation of `BigDecimal.[double|float]Value()` to enhance performance, avoiding an intermediate string and its subsequent parsing on the slow path. - Commit messages: - 8205592: BigDecimal.doubleValue() is depressingly slow Changes: https://git.openjdk.org/jdk/pull/9410/files Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=9410&range=00 Issue: https://bugs.openjdk.org/browse/JDK-8205592 Stats: 585 lines in 2 files changed: 545 ins; 8 del; 32 mod Patch: https://git.openjdk.org/jdk/pull/9410.diff Fetch: git fetch https://git.openjdk.org/jdk pull/9410/head:pull/9410 PR: https://git.openjdk.org/jdk/pull/9410
Re: RFR: JDK-8266431: Dual-Pivot Quicksort improvements (Radix sort) [v15]
On Thu, 7 Jul 2022 15:58:33 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) > > * Added JMH Thanks vladimir, your comment and test look good , even if I would not trust results for size < 200... @AlanBateman If needed, a new system.property or flag could be added to disable heap-allocation or adjust the max heap ratio (1/128) as it may depend on use cases. To insist, current jdk's Array.sort() does not have any heap limit (merge sort only) so this PR is a great achievement to reduce memory footprint compared to the current situation. 1/128th seemed to me a good upper limit as my compromise on speed vs memory, please propose any more conservative or appropriate threshold. Laurent - PR: https://git.openjdk.org/jdk/pull/3938
Re: [jdk19] RFR: 8289486: Improve XSLT XPath operators count efficiency [v2]
> 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 - Changes: - all: https://git.openjdk.org/jdk19/pull/101/files - new: https://git.openjdk.org/jdk19/pull/101/files/330f86ce..ef9cf7d7 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk19&pr=101&range=01 - incr: https://webrevs.openjdk.org/?repo=jdk19&pr=101&range=00-01 Stats: 10 lines in 2 files changed: 10 ins; 0 del; 0 mod Patch: https://git.openjdk.org/jdk19/pull/101.diff Fetch: git fetch https://git.openjdk.org/jdk19 pull/101/head:pull/101 PR: https://git.openjdk.org/jdk19/pull/101
[jdk19] Integrated: 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. This pull request has now been integrated. Changeset: a8eb7286 Author:Stuart Marks URL: https://git.openjdk.org/jdk19/commit/a8eb728680529e81bea0584912dead394c35b040 Stats: 10 lines in 1 file changed: 0 ins; 5 del; 5 mod 8289779: Map::replaceAll javadoc has redundant @throws clauses Reviewed-by: prappo, iris - PR: https://git.openjdk.org/jdk19/pull/111
RFR: 8289643: File descriptor leak with ProcessBuilder.startPipeline
The `ProcessBuilder.pipelineStart()` implementation does not close all of the file descriptors it uses to create the pipeline of processes. The process calling `pipelineStart()` is creating the pipes between the stages. As each process is launched, the file descriptor is inherited by the child process and the child process `dup`s them to the respective stdin/stdout/stderr fd. These copies of inherited file descriptors are handled correctly. Between the launching of each Process, the file descriptor for the read-side of the pipe for the output of the previous process is kept open (in the parent process invoking `pipelineStart`). The file descriptor is correctly passed to the child and is dup'd to the stdin of the next process. However, the open file descriptor in the parent is not closed after it has been used as the input for the next Process. The fix is to close the fd after it has been used as the input of the next process. A new test verifies that after `pipelineStart` is complete, the same file descriptors are open for Unix Pipes as before the test. The test only runs on Linux using the /proc//fd filesystem to identify open file descriptors. The bug fix is in `ProcessBuilder.pipelineStart` and is applicable to all platforms. - Commit messages: - 8289643: File descriptor leak with ProcessBuilder.startPipeline Changes: https://git.openjdk.org/jdk/pull/9414/files Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=9414&range=00 Issue: https://bugs.openjdk.org/browse/JDK-8289643 Stats: 144 lines in 2 files changed: 144 ins; 0 del; 0 mod Patch: https://git.openjdk.org/jdk/pull/9414.diff Fetch: git fetch https://git.openjdk.org/jdk pull/9414/head:pull/9414 PR: https://git.openjdk.org/jdk/pull/9414
Re: [jdk19] RFR: 8289030: [macos] app image signature invalid when creating DMG or PKG [v2]
On Thu, 7 Jul 2022 17:07:51 GMT, Alexey Semenyuk wrote:
>> 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
>
> src/jdk.jpackage/macosx/classes/jdk/jpackage/internal/MacBaseInstallerBundler.java
> line 171:
>
>> 169:Files.deleteIfExists(AppImageFile.getPathInAppImage(appDir));
>> 170: }
>> 171:
>
> I think there is no need to modify AbstractAppImageBuilder.java, and
> AppImageBundler.java.
> It is sufficient to modify the condition controlling the creation of
> `.package` file:
>
> if (predefinedImage == null ||
> (!StandardBundlerParam.isRuntimeInstaller(params) &&
> !AppImageFile.load(predefinedImage).isSigned())) {
> new PackageFile(APP_NAME.fetchFrom(params)).save(
> ApplicationLayout.macAppImage().resolveAt(appDir));
> Files.deleteIfExists(AppImageFile.getPathInAppImage(appDir));
> }
>
> Besides `.package` file logically doesn't belong to app image, it belongs to
> the installed application, so it must not be referenced from the classes
> creating app images.
We need to add `.package` file during app image creation, since we need to sign
it. With your proposed change we will add `.package` file to already signed app
image.
-
PR: https://git.openjdk.org/jdk19/pull/89
