Re: RFR: 8171382: java.time.Duration missing isPositive method

2021-07-23 Thread Naoto Sato
On Fri, 23 Jul 2021 19:37:44 GMT, Stephen Colebourne wrote: >> Please review this PR to introduce `java.time.Duration.isPositive()` method. >> A CSR is also drafted. > > src/java.base/share/classes/java/time/Duration.java line 596: > >> 594: */ >> 595: public boolean isPositive() {

Re: RFR: 8171382: java.time.Duration missing isPositive method

2021-07-23 Thread Stephen Colebourne
On Fri, 23 Jul 2021 17:27:27 GMT, Naoto Sato wrote: > Please review this PR to introduce `java.time.Duration.isPositive()` method. > A CSR is also drafted. Marked as reviewed by scolebourne (Author). src/java.base/share/classes/java/time/Duration.java line 596: > 594: */ > 595:

Integrated: 8199594: Add doc describing how (?x) ignores spaces in character classes

2021-07-23 Thread Ian Graves
On Mon, 28 Jun 2021 20:50:42 GMT, Ian Graves wrote: > 8199594: Add doc describing how (?x) ignores spaces in character classes This pull request has now been integrated. Changeset: a1c0a6aa Author:Ian Graves URL:

Re: RFR: 8171382: java.time.Duration missing isPositive method

2021-07-23 Thread Brian Burkhalter
On Fri, 23 Jul 2021 17:27:27 GMT, Naoto Sato wrote: > Please review this PR to introduce `java.time.Duration.isPositive()` method. > A CSR is also drafted. Marked as reviewed by bpb (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/4892

Re: RFR: 8199594: Add doc describing how (?x) ignores spaces in character classes [v3]

2021-07-23 Thread Brian Burkhalter
On Thu, 22 Jul 2021 01:46:09 GMT, Ian Graves wrote: >> 8199594: Add doc describing how (?x) ignores spaces in character classes > > Ian Graves has updated the pull request incrementally with one additional > commit since the last revision: > > Rewording repetitive phrase Marked as reviewed

Re: RFR: 8171382: java.time.Duration missing isPositive method

2021-07-23 Thread Iris Clark
On Fri, 23 Jul 2021 17:27:27 GMT, Naoto Sato wrote: > Please review this PR to introduce `java.time.Duration.isPositive()` method. > A CSR is also drafted. Looks good! I've also Reviewed the associated CSR. - Marked as reviewed by iris (Reviewer). PR:

Re: RFR: 8199594: Add doc describing how (?x) ignores spaces in character classes [v3]

2021-07-23 Thread Lance Andersen
On Thu, 22 Jul 2021 01:46:09 GMT, Ian Graves wrote: >> 8199594: Add doc describing how (?x) ignores spaces in character classes > > Ian Graves has updated the pull request incrementally with one additional > commit since the last revision: > > Rewording repetitive phrase Marked as reviewed

Re: RFR: 8270321: Startup regressions in 18-b5 caused by JDK-8266310

2021-07-23 Thread Rémi Forax
On Fri, 23 Jul 2021 18:03:31 GMT, Sergey Chernyshev wrote: > Dear colleagues, > > Please review the patch that replaces the lambdas with anonymous classes > which solves the startup time regression as shown below. > > I attached the Bytestacks flamegraphs for both original (regression) and

Re: RFR: 8199594: Add doc describing how (?x) ignores spaces in character classes [v3]

2021-07-23 Thread Iris Clark
On Thu, 22 Jul 2021 01:46:09 GMT, Ian Graves wrote: >> 8199594: Add doc describing how (?x) ignores spaces in character classes > > Ian Graves has updated the pull request incrementally with one additional > commit since the last revision: > > Rewording repetitive phrase Marked as reviewed

Re: RFR: 8171382: java.time.Duration missing isPositive method

2021-07-23 Thread Joe Wang
On Fri, 23 Jul 2021 17:27:27 GMT, Naoto Sato wrote: > Please review this PR to introduce `java.time.Duration.isPositive()` method. > A CSR is also drafted. Marked as reviewed by joehw (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/4892

RFR: 8270321: Startup regressions in 18-b5 caused by JDK-8266310

2021-07-23 Thread Sergey Chernyshev
Dear colleagues, Please review the patch that replaces the lambdas with anonymous classes which solves the startup time regression as shown below. I attached the Bytestacks flamegraphs for both original (regression) and fixed versions. The flamegraphs clearly show the lambdas were causing the

Re: RFR: 8171382: java.time.Duration missing isPositive method

2021-07-23 Thread Naoto Sato
On Fri, 23 Jul 2021 17:27:27 GMT, Naoto Sato wrote: > Please review this PR to introduce `java.time.Duration.isPositive()` method. > A CSR is also drafted. Thanks, Roger. Modified the CSR as suggested. - PR: https://git.openjdk.java.net/jdk/pull/4892

Re: RFR: 8171382: java.time.Duration missing isPositive method

2021-07-23 Thread Roger Riggs
On Fri, 23 Jul 2021 17:27:27 GMT, Naoto Sato wrote: > Please review this PR to introduce `java.time.Duration.isPositive()` method. > A CSR is also drafted. In the CSR, I would omit the "which is odd" from the "Problem". and in the Solution, replace "integrity" with "completeness".

RFR: 8171382: java.time.Duration missing isPositive method

2021-07-23 Thread Naoto Sato
Please review this PR to introduce `java.time.Duration.isPositive()` method. A CSR is also drafted. - Commit messages: - 8171382: java.time.Duration missing isPositive method Changes: https://git.openjdk.java.net/jdk/pull/4892/files Webrev:

Re: RFR: 8268873: Unnecessary Vector usage in java.base

2021-07-23 Thread Sean Mullan
On Fri, 18 Jun 2021 18:28:26 GMT, Sean Mullan wrote: >> Usage of thread-safe collection `Vector` is unnecessary. It's recommended to >> use `ArrayList` if a thread-safe implementation is not needed. In >> post-BiasedLocking times, this is gets worse, as every access is >> synchronized. >> I

Re: RFR: 8265474: Dubious 'null' assignment in CompactByteArray.expand

2021-07-23 Thread Naoto Sato
On Wed, 23 Dec 2020 16:06:30 GMT, Andrey Turbanov wrote: > I propose to remove 'null' assignment of field CompactByteArray.values in > `expand` method. In the next statement this field is overridden with another > value - `tempArray`. > This code was there from initial load of OpenJDK

Re: RFR: 8190753: (zipfs): Accessing a large entry (> 2^31 bytes) leads to a negative initial size for ByteArrayOutputStream [v11]

2021-07-23 Thread Lance Andersen
On Fri, 23 Jul 2021 15:23:07 GMT, Jaikiran Pai wrote: > Thank you for the review Alan. > > @LanceAndersen, I've run the tier1 tests locally with the latest PR and they > have passed without any regressions. Given that we changed the implementation > to wrap ByteArrayOutputStream instead of

Re: RFR: 8268873: Unnecessary Vector usage in java.base

2021-07-23 Thread Alan Bateman
On Fri, 18 Jun 2021 18:28:26 GMT, Sean Mullan wrote: >> Usage of thread-safe collection `Vector` is unnecessary. It's recommended to >> use `ArrayList` if a thread-safe implementation is not needed. In >> post-BiasedLocking times, this is gets worse, as every access is >> synchronized. >> I

Re: RFR: 8190753: (zipfs): Accessing a large entry (> 2^31 bytes) leads to a negative initial size for ByteArrayOutputStream [v11]

2021-07-23 Thread Jaikiran Pai
On Fri, 23 Jul 2021 14:58:01 GMT, Jaikiran Pai wrote: >> Can I please get a review for this proposed fix for the issue reported in >> https://bugs.openjdk.java.net/browse/JDK-8190753? >> >> The commit here checks for the size of the zip entry before trying to create >> a

Re: RFR: 8236569: -Xss not multiple of 4K does not work for the main thread on macOS [v6]

2021-07-23 Thread Alan Bateman
On Tue, 8 Jun 2021 16:53:38 GMT, Henry Jen wrote: >> …d on macOS >> >> This patch simply round up the specified stack size to multiple of the >> system page size. >> >> Test is trivial, simply run java with -Xss option against following code. On >> MacOS, before the fix, running with

Re: RFR: 8265474: Dubious 'null' assignment in CompactByteArray.expand

2021-07-23 Thread Alan Bateman
On Wed, 23 Dec 2020 16:06:30 GMT, Andrey Turbanov wrote: > I propose to remove 'null' assignment of field CompactByteArray.values in > `expand` method. In the next statement this field is overridden with another > value - `tempArray`. > This code was there from initial load of OpenJDK

Re: RFR: 8190753: (zipfs): Accessing a large entry (> 2^31 bytes) leads to a negative initial size for ByteArrayOutputStream [v9]

2021-07-23 Thread Alan Bateman
On Fri, 23 Jul 2021 11:57:34 GMT, Jaikiran Pai wrote: > The updated version of this PR now fixes this part to open it just once. I > had reviewed this `transferTo` multiple times before, but clearly I > overlooked this part of the implementation. > > Thank you for these inputs. The updated PR

Re: RFR: 8190753: (zipfs): Accessing a large entry (> 2^31 bytes) leads to a negative initial size for ByteArrayOutputStream [v11]

2021-07-23 Thread Jaikiran Pai
> Can I please get a review for this proposed fix for the issue reported in > https://bugs.openjdk.java.net/browse/JDK-8190753? > > The commit here checks for the size of the zip entry before trying to create > a `ByteArrayOutputStream` for that entry's content. A new jtreg test has been >

Re: RFR: JDK-8266490: Extend the OSContainer API to support the pids controller of cgroups [v6]

2021-07-23 Thread Severin Gehwolf
On Fri, 23 Jul 2021 12:28:47 GMT, Severin Gehwolf wrote: > Could this be some setting configured on your box that is picked up as an > additional limit ? Possibly. Not sure where to look, though. - PR: https://git.openjdk.java.net/jdk/pull/4518

Re: RFR: JDK-8266490: Extend the OSContainer API to support the pids controller of cgroups [v6]

2021-07-23 Thread Severin Gehwolf
On Fri, 23 Jul 2021 11:02:13 GMT, Matthias Baesken wrote: > > No, I don't know what to do about it. All I can see it comes back with a > > pids limit of `38019` when set to `-1`. It does seem like a bug or an > > intentional setting so as to avoid fork bombs. > > Very strange indeed, I have

Re: RFR: 8251329: (zipfs) Files.walkFileTree walks infinitely if zip has dir named "." inside [v4]

2021-07-23 Thread Jaikiran Pai
On Fri, 23 Jul 2021 10:36:51 GMT, Alan Bateman wrote: > I wasn't suggesting there is a patch attached to that issue. Rather I was > just pointing out that JDK-8251329 was being worked on already before this PR > was created. Ok, that makes sense. Thank you for the details. - PR:

Re: RFR: 8190753: (zipfs): Accessing a large entry (> 2^31 bytes) leads to a negative initial size for ByteArrayOutputStream [v9]

2021-07-23 Thread Jaikiran Pai
On Fri, 23 Jul 2021 10:28:02 GMT, Alan Bateman wrote: > So I think the approach looks good but I think the synchronization needs to > be re-checked it is not obvious that is correct or needed. Are there any > cases where FileRolloverOutputStream is returned to user code? I don't think > so,

Re: [jdk17] RFR: 8271155: Wrong path separator in env variable

2021-07-23 Thread Kevin Rushforth
On Thu, 22 Jul 2021 19:35:59 GMT, Alexey Semenyuk wrote: > Replace `";"` with `FileUtils::pathSeparator` in the expression adding 'app' > dir to env variable in jpackage app launcher. Alexey filed [JDK-8271170](https://bugs.openjdk.java.net/browse/JDK-8271170) to cover this. -

Re: RFR: 8190753: (zipfs): Accessing a large entry (> 2^31 bytes) leads to a negative initial size for ByteArrayOutputStream [v10]

2021-07-23 Thread Jaikiran Pai
> Can I please get a review for this proposed fix for the issue reported in > https://bugs.openjdk.java.net/browse/JDK-8190753? > > The commit here checks for the size of the zip entry before trying to create > a `ByteArrayOutputStream` for that entry's content. A new jtreg test has been >

Re: RFR: 8247373: ArraysSupport.newLength doc, test, and exception message [v5]

2021-07-23 Thread Pavel Rappo
On Tue, 2 Mar 2021 18:07:24 GMT, Stuart Marks wrote: >> test/jdk/jdk/internal/util/ArraysSupport/NewLength.java line 100: >> >>> 98: int r = ArraysSupport.newLength(old, min, pref); >>> 99: fail("expected OutOfMemoryError, got normal return value of >>> " + r); >>> 100:

Re: RFR: JDK-8266490: Extend the OSContainer API to support the pids controller of cgroups [v6]

2021-07-23 Thread Matthias Baesken
On Fri, 23 Jul 2021 08:39:37 GMT, Severin Gehwolf wrote: > > No, I don't know what to do about it. All I can see it comes back with a pids > limit of `38019` when set to `-1`. It does seem like a bug or an intentional > setting so as to avoid fork bombs. Very strange indeed, I have docker

Re: RFR: 8251329: (zipfs) Files.walkFileTree walks infinitely if zip has dir named "." inside [v4]

2021-07-23 Thread Alan Bateman
On Fri, 23 Jul 2021 08:47:46 GMT, Jaikiran Pai wrote: > This part I didn't understand. Did you mean to refer some other JBS issue? > Because from what I see in https://bugs.openjdk.java.net/browse/JDK-8251329 > there's no patch attached to it (unless of course it's restricted to specific >

Re: RFR: 8190753: (zipfs): Accessing a large entry (> 2^31 bytes) leads to a negative initial size for ByteArrayOutputStream [v9]

2021-07-23 Thread Alan Bateman
On Thu, 22 Jul 2021 16:01:30 GMT, Jaikiran Pai wrote: >> Can I please get a review for this proposed fix for the issue reported in >> https://bugs.openjdk.java.net/browse/JDK-8190753? >> >> The commit here checks for the size of the zip entry before trying to create >> a

Withdrawn: 8251329: (zipfs) Files.walkFileTree walks infinitely if zip has dir named "." inside

2021-07-23 Thread Jaikiran Pai
On Sun, 27 Jun 2021 13:13:42 GMT, Jaikiran Pai wrote: > Can I please get a review of this proposed fix for > https://bugs.openjdk.java.net/browse/JDK-8251329? > > As noted in that issue, if a zip filesystem created on top of a jar > containing a "./" entry is "walked" using the

Re: RFR: 8251329: (zipfs) Files.walkFileTree walks infinitely if zip has dir named "." inside [v4]

2021-07-23 Thread Jaikiran Pai
On Fri, 2 Jul 2021 11:06:40 GMT, Jaikiran Pai wrote: >> Can I please get a review of this proposed fix for >> https://bugs.openjdk.java.net/browse/JDK-8251329? >> >> As noted in that issue, if a zip filesystem created on top of a jar >> containing a "./" entry is "walked" using the

Re: RFR: 8251329: (zipfs) Files.walkFileTree walks infinitely if zip has dir named "." inside [v4]

2021-07-23 Thread Jaikiran Pai
On Fri, 2 Jul 2021 11:06:40 GMT, Jaikiran Pai wrote: >> Can I please get a review of this proposed fix for >> https://bugs.openjdk.java.net/browse/JDK-8251329? >> >> As noted in that issue, if a zip filesystem created on top of a jar >> containing a "./" entry is "walked" using the

Re: RFR: JDK-8266490: Extend the OSContainer API to support the pids controller of cgroups [v6]

2021-07-23 Thread Severin Gehwolf
On Fri, 23 Jul 2021 06:49:15 GMT, Matthias Baesken wrote: >> Matthias Baesken has updated the pull request incrementally with one >> additional commit since the last revision: >> >> Minor adjustments, handling of Unlimited > >> @MBaesken Thanks. We need a solution for [#4518 >>

Re: RFR: 8251329: (zipfs) Files.walkFileTree walks infinitely if zip has dir named "." inside [v4]

2021-07-23 Thread Alan Bateman
On Fri, 2 Jul 2021 11:08:43 GMT, Lance Andersen wrote: >> Jaikiran Pai 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 >>

Re: [jdk17] RFR: 8271155: Wrong path separator in env variable

2021-07-23 Thread Alan Bateman
On Thu, 22 Jul 2021 19:35:59 GMT, Alexey Semenyuk wrote: > Replace `";"` with `FileUtils::pathSeparator` in the expression adding 'app' > dir to env variable in jpackage app launcher. Is there a test planned for this change or is it covered by an existing test? Just asking as this has been

Re: RFR: JDK-8266490: Extend the OSContainer API to support the pids controller of cgroups [v6]

2021-07-23 Thread Matthias Baesken
On Thu, 22 Jul 2021 12:18:20 GMT, Matthias Baesken wrote: >> Hello, please review this PR; it extend the OSContainer API in order to also >> support the pids controller of cgroups. >> >> I noticed that unlike the other controllers "cpu", "cpuset", "cpuacct", >> "memory" on some older Linux