RFR: 8336895: BufferedReader doesn't read full \r\n line ending when it doesn't fit in buffer
Add some verbiage stating that two buffered readers or input streams should not be used to read from the same reader or input stream, respectively. - Commit messages: - 8336895: BufferedReader doesn't read full \r\n line ending when it doesn't fit in buffer Changes: https://git.openjdk.org/jdk/pull/20320/files Webrev: https://webrevs.openjdk.org/?repo=jdk=20320=00 Issue: https://bugs.openjdk.org/browse/JDK-8336895 Stats: 12 lines in 2 files changed: 11 ins; 0 del; 1 mod Patch: https://git.openjdk.org/jdk/pull/20320.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/20320/head:pull/20320 PR: https://git.openjdk.org/jdk/pull/20320
Re: RFR: 8337143: (fc, fs) Move filesystem-related native objects from libnio to libjava
On Wed, 24 Jul 2024 19:11:42 GMT, Brian Burkhalter wrote: > This proposed change would move the native objects required for NIO file > interaction from the libnio native library to the libjava native library on > Linux, macOS, and Windows. This change passes CI tiers 1-5 jobs on all platforms. With the change in place, one can remove `libnio` from the JDK and still be able to copy a file using FileChannel. Without the change, doing this will result in throwing a `java.lang.UnsatisfiedLinkError`. - PR Comment: https://git.openjdk.org/jdk/pull/20317#issuecomment-2248732983
RFR: 8337143: (fc, fs) Move filesystem-related native objects from libnio to libjava
This proposed change would move the native objects required for NIO file interaction from the libnio native library to the libjava native library on Linux, macOS, and Windows. - Commit messages: - 8337143: (fc, fs) Move filesystem-related native objects from libnio to libjava Changes: https://git.openjdk.org/jdk/pull/20317/files Webrev: https://webrevs.openjdk.org/?repo=jdk=20317=00 Issue: https://bugs.openjdk.org/browse/JDK-8337143 Stats: 1265 lines in 75 files changed: 631 ins; 523 del; 111 mod Patch: https://git.openjdk.org/jdk/pull/20317.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/20317/head:pull/20317 PR: https://git.openjdk.org/jdk/pull/20317
Re: RFR: 8336679: Add @implSpec for the default implementations in Process.waitFor() [v4]
On Wed, 24 Jul 2024 16:41:46 GMT, Naoto Sato wrote: >> This is a simple doc-only change that follows up >> [JDK-8336479](https://bugs.openjdk.org/browse/JDK-8336479). A corresponding >> CSR has also been drafted. > > Naoto Sato has updated the pull request incrementally with two additional > commits since the last revision: > > - Update src/java.base/share/classes/java/lang/Process.java > >Co-authored-by: ExE Boss <3889017+exe-b...@users.noreply.github.com> > - Update src/java.base/share/classes/java/lang/Process.java > >Co-authored-by: ExE Boss <3889017+exe-b...@users.noreply.github.com> Marked as reviewed by bpb (Reviewer). - PR Review: https://git.openjdk.org/jdk/pull/20285#pullrequestreview-2197270404
Re: RFR: 8336679: Add @implSpec for the default implementations in Process.waitFor() [v3]
On Tue, 23 Jul 2024 20:58:01 GMT, Naoto Sato wrote: >> This is a simple doc-only change that follows up >> [JDK-8336479](https://bugs.openjdk.org/browse/JDK-8336479). A corresponding >> CSR has also been drafted. > > Naoto Sato has updated the pull request incrementally with one additional > commit since the last revision: > > apiNote -> implNote Re-reviewed. - Marked as reviewed by bpb (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/20285#pullrequestreview-2197094696
Re: RFR: 8336679: Add @implSpec for the default implementations in Process.waitFor()
On Mon, 22 Jul 2024 22:10:07 GMT, Naoto Sato wrote: > This is a simple doc-only change that follows up > [JDK-8336479](https://bugs.openjdk.org/browse/JDK-8336479). A corresponding > CSR has also been drafted. Marked as reviewed by bpb (Reviewer). - PR Review: https://git.openjdk.org/jdk/pull/20285#pullrequestreview-2192656420
Re: RFR: 8334758: Incorrect note in Javadoc for a few RandomGenerator methods [v2]
On Fri, 19 Jul 2024 17:08:38 GMT, Raffaello Giulietti wrote: > I guess that the approval of the CSR, once done, would "de-facto" count as a > 2nd review? I agree. - PR Comment: https://git.openjdk.org/jdk/pull/20152#issuecomment-2239863660
Re: RFR: 8334758: Incorrect note in Javadoc for a few RandomGenerator methods [v2]
On Fri, 19 Jul 2024 16:50:50 GMT, Raffaello Giulietti wrote: >> Small corrections to @implSpec notes in a few methods in RandomGenerator. > > Raffaello Giulietti has updated the pull request incrementally with one > additional commit since the last revision: > > Improved wording. Looks fine but maybe a second Reviewer should agree? - Marked as reviewed by bpb (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/20152#pullrequestreview-2188720979
Re: RFR: 8334758: Incorrect note in Javadoc for a few RandomGenerator methods
On Fri, 12 Jul 2024 06:28:15 GMT, Raffaello Giulietti
wrote:
> Small corrections to @implSpec notes in a few methods in RandomGenerator.
src/java.base/share/classes/java/util/random/RandomGenerator.java line 816:
> 814: * distribution in the range 0 (inclusive)
> 815: * to {@code bound} (exclusive).
> 816: * It assumes the distribution of {@link #nextInt()} to be uniform
> in turn.
Do you need "in turn" here (and in the other changes)?
-
PR Review Comment: https://git.openjdk.org/jdk/pull/20152#discussion_r1684617106
Re: RFR: 8315034 : File.mkdirs() occasionally fails to create folders on Windows shared folder
On Thu, 18 Jul 2024 19:16:03 GMT, Weibing Xiao wrote: > It was not backport to jdk8u. https://bugs.openjdk.org/browse/JDK-8321863 is > for 8u411-perf, which is not same as jdk8u release. Thanks for the clarification. - PR Comment: https://git.openjdk.org/jdk/pull/16502#issuecomment-2237363760
Re: RFR: 8315034 : File.mkdirs() occasionally fails to create folders on Windows shared folder
On Thu, 11 Jul 2024 13:10:32 GMT, Olivier Masseau wrote: > Hello, Has this been backported to Java 8 ? I can't really find the info. According to the issue [JDK-8321863](https://bugs.openjdk.org/browse/JDK-8321863) it looks like it has been so backported. - PR Comment: https://git.openjdk.org/jdk/pull/16502#issuecomment-2237262955
Re: RFR: 8331907: BigInteger and BigDecimal should use optimized division [v4]
On Fri, 10 May 2024 16:16:18 GMT, Daniel Jeliński wrote: >> Replace the custom unsigned divide / remainder functions with the >> compiler-optimized Long.divideUnsigned / remainderUnsigned. >> >> No new tests. Existing tier1-3 tests continue to pass. >> >> I added a new set of divide benchmarks. Results in thread. >> >> I also removed the BigDecimal.toString benchmarks. They no longer serve >> their purpose - toString caches the returned value, so we were only >> benchmarking the cache access time. > > Daniel Jeliński has updated the pull request incrementally with one > additional commit since the last revision: > > Integer divide I have nothing to add to the comments already resolved. +1 - Marked as reviewed by bpb (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/19134#pullrequestreview-2054040985
Re: RFR: 8332029: Provide access to initial value of stderr via JavaLangAccess
On Fri, 10 May 2024 07:57:40 GMT, Alan Bateman wrote: > In preparation for JEP 471 and JEP 472, provide access to the initial value > of System.err from JavaLangAccess. The initial value of System.in is already > exposed to code in java.base with this shared secret. +1 - Marked as reviewed by bpb (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/19171#pullrequestreview-2050422897
Re: RFR: 8332064: Implementation of Structured Concurrency (Third Preview)
On Fri, 10 May 2024 10:58:42 GMT, Alan Bateman wrote: > There aren't any API or implementations changes in third preview but the JEP > number/title needs to be bumped for the javadoc page. Marked as reviewed by bpb (Reviewer). - PR Review: https://git.openjdk.org/jdk/pull/19175#pullrequestreview-2050415510
Re: Need for a sponsor for JDK-8313674
Hello, This is the PR [1]. This is the sequence that should be followed: 1. Align the name title of the PR with the issue. (you) 2. Continue the conversation until at least one Reviewer approves it. (one or more of us) 3. Comment with /integrate command. (you) 4. Comment with /sponsor command. (one of us). Alan might have more comments on this, and given that it is after close of business in his time zone, I don’t think we’ll see further progress before Monday. Thanks, Brian [1] https://github.com/openjdk/jdk/pull/19021 On Apr 24, 2024, at 9:23 PM, Iñigo Mediavilla wrote: For my first contribution to OpenJDK, I've started looking into JDK-8313674, an issue that falls into core-libs. According to the OpenJDK Developers’ Guide I'd need a sponsor to help me through the contribution process, would anyone be available to help me ?
Integrated: 8330748: ByteArrayOutputStream.writeTo(OutputStream) pins carrier
On Mon, 22 Apr 2024 19:07:04 GMT, Brian Burkhalter wrote: > Prevent blocking due to a carrier thread not being released when > `ByteArrayOutputStream.writeTo` is invoked from a virtual thread. This pull request has now been integrated. Changeset: 819f3d6f Author: Brian Burkhalter URL: https://git.openjdk.org/jdk/commit/819f3d6fc70ff6fe54ac5f9033c17c3dd4326aa5 Stats: 142 lines in 2 files changed: 139 ins; 0 del; 3 mod 8330748: ByteArrayOutputStream.writeTo(OutputStream) pins carrier Reviewed-by: alanb - PR: https://git.openjdk.org/jdk/pull/18901
Re: RFR: 8331108: Unused Math.abs call in java.lang.FdLibm.Expm1#compute
On Thu, 25 Apr 2024 21:32:03 GMT, Joe Darcy wrote: > Remove unnecessary setting of variable y, found by an IDE inspection noted in > the bug report. Marked as reviewed by bpb (Reviewer). - PR Review: https://git.openjdk.org/jdk/pull/18963#pullrequestreview-2023708504
Re: RFR: 8330748: ByteArrayOutputStream.writeTo(OutputStream) pins carrier [v4]
On Wed, 24 Apr 2024 15:45:58 GMT, Brian Burkhalter wrote: >> Prevent blocking due to a carrier thread not being released when >> `ByteArrayOutputStream.writeTo` is invoked from a virtual thread. > > Brian Burkhalter 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: > > - 8330748: Modify writeTo() not to invoke toByteArray() > - Merge > - 8330748: Add vthread1.join() in test > - Correct ID in test @bug tag > - 8330748: ByteArrayOutputStream.writeTo(OutputStream) pins carrier Running tests now. Assuming those pan out, will likely integrate tomorrow. - PR Comment: https://git.openjdk.org/jdk/pull/18901#issuecomment-2075392686
Re: RFR: 8330748: ByteArrayOutputStream.writeTo(OutputStream) pins carrier [v4]
> Prevent blocking due to a carrier thread not being released when > `ByteArrayOutputStream.writeTo` is invoked from a virtual thread. Brian Burkhalter 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: - 8330748: Modify writeTo() not to invoke toByteArray() - Merge - 8330748: Add vthread1.join() in test - Correct ID in test @bug tag - 8330748: ByteArrayOutputStream.writeTo(OutputStream) pins carrier - Changes: - all: https://git.openjdk.org/jdk/pull/18901/files - new: https://git.openjdk.org/jdk/pull/18901/files/1dd59b7b..8076291f Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=18901=03 - incr: https://webrevs.openjdk.org/?repo=jdk=18901=02-03 Stats: 8598 lines in 278 files changed: 4899 ins; 2693 del; 1006 mod Patch: https://git.openjdk.org/jdk/pull/18901.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/18901/head:pull/18901 PR: https://git.openjdk.org/jdk/pull/18901
Re: RFR: 8330748: ByteArrayOutputStream.writeTo(OutputStream) pins carrier [v2]
On Wed, 24 Apr 2024 14:54:34 GMT, Viktor Klang wrote:
>> Currently we have
>>
>> public void writeTo(OutputStream out) throws IOException {
>> if (Thread.currentThread().isVirtual()) {
>> out.write(toByteArray());
>> } else synchronized (this) {
>> out.write(buf, 0, count);
>> }
>> }
>>
>> where `toByteArray()` is `synchronized`, but here I would think that we'd
>> want to replace it with simply `Arrays.copyOf(buf, count)` without the
>> `synchronized`, no?
>
> @bplb My interpretation was that we didn't want to hold the monitor *during*
> out.write().
Please see 8076291fb3d097ef67d0b59b9be3c8b762aad7cf.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/18901#discussion_r1578116805
Re: RFR: 8330748: ByteArrayOutputStream.writeTo(OutputStream) pins carrier [v2]
On Wed, 24 Apr 2024 07:08:20 GMT, Alan Bateman wrote:
>> So do we think it better not to invoke `toByteArray` here?
>
> Using a subclass to count the number of invocations of toByteArray seems a
> bit strange but in general it is more robust to not rely on a method that may
> be overridden by a subclass. So I think the suggestion is good.
Currently we have
public void writeTo(OutputStream out) throws IOException {
if (Thread.currentThread().isVirtual()) {
out.write(toByteArray());
} else synchronized (this) {
out.write(buf, 0, count);
}
}
where `toByteArray()` is `synchronized`, but here I would think that we'd want
to replace it with simply `Arrays.copyOf(buf, count)` without the
`synchronized`, no?
-
PR Review Comment: https://git.openjdk.org/jdk/pull/18901#discussion_r1578031656
Re: RFR: 8330748: ByteArrayOutputStream.writeTo(OutputStream) pins carrier [v2]
On Tue, 23 Apr 2024 06:20:47 GMT, Alan Bateman wrote:
>> Brian Burkhalter has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> Correct ID in test @bug tag
>
> test/jdk/java/io/ByteArrayOutputStream/WriteToReleasesCarrier.java line 78:
>
>> 76: }
>> 77: } finally {
>> 78: LockSupport.unpark(vthread1);
>
> It might be clearer if you add vthread1.join() after the unpark. It's not
> strictly needed here as scheduler::close will block until the carrier
> terminates so that will guarantee that the virtual thread has unmounted.
Added `vthread1.join()` after the unpark in
1dd59b7bf7aa2087845ad7806a5afbed2b5ea1b5.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/18901#discussion_r1577076109
Re: RFR: 8330748: ByteArrayOutputStream.writeTo(OutputStream) pins carrier [v3]
> Prevent blocking due to a carrier thread not being released when > `ByteArrayOutputStream.writeTo` is invoked from a virtual thread. Brian Burkhalter has updated the pull request incrementally with one additional commit since the last revision: 8330748: Add vthread1.join() in test - Changes: - all: https://git.openjdk.org/jdk/pull/18901/files - new: https://git.openjdk.org/jdk/pull/18901/files/57e4917b..1dd59b7b Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=18901=02 - incr: https://webrevs.openjdk.org/?repo=jdk=18901=01-02 Stats: 1 line in 1 file changed: 1 ins; 0 del; 0 mod Patch: https://git.openjdk.org/jdk/pull/18901.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/18901/head:pull/18901 PR: https://git.openjdk.org/jdk/pull/18901
Re: RFR: 8330748: ByteArrayOutputStream.writeTo(OutputStream) pins carrier [v2]
On Tue, 23 Apr 2024 17:35:42 GMT, Jason Mehrens wrote: >> A good question. The buf/count fields are protected so the subclass has >> direct access to the bytes. So while it could Arrays.copy the bytes, it >> doesn't help with a buggy subclass that is changing bytes without >> synchronization. > > I was thinking more of a subclass that counted invocations to public methods > or metering which would cause subclass to double the counts when calling via > virtual thread. So do we think it better not to invoke `toByteArray` here? - PR Review Comment: https://git.openjdk.org/jdk/pull/18901#discussion_r1576686390
Re: RFR: 8329593: Drop adjustments to target parallelism when virtual threads do I/O on files opened for buffered I/O [v2]
On Tue, 23 Apr 2024 08:09:20 GMT, Alan Bateman wrote:
>> src/java.base/share/classes/java/io/FileInputStream.java line 211:
>>
>>> 209: * @param name the name of the file
>>> 210: */
>>> 211: private void open(String name) throws FileNotFoundException {
>>
>> If method such as this is private, and only delegates to the 0-suffixed
>> native method, would't it be better to just call the 0-suffixed method
>> directly?
>
> Historically these native methods were wrapped in order to support
> instrumentation, I didn't want to touch in this PR.
And presumably all these 0-suffixed methods will eventually be replaced with
FFM calls anyway.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/18598#discussion_r1576561165
Re: RFR: 8330748: ByteArrayOutputStream.writeTo(OutputStream) pins carrier [v2]
On Mon, 22 Apr 2024 19:49:44 GMT, Brian Burkhalter wrote: >> Prevent blocking due to a carrier thread not being released when >> `ByteArrayOutputStream.writeTo` is invoked from a virtual thread. > > Brian Burkhalter has updated the pull request incrementally with one > additional commit since the last revision: > > Correct ID in test @bug tag Tiers 1-3 passed on the usual platforms. - PR Comment: https://git.openjdk.org/jdk/pull/18901#issuecomment-2071221654
Re: RFR: 8330748: ByteArrayOutputStream.writeTo(OutputStream) pins carrier [v2]
> Prevent blocking due to a carrier thread not being released when > `ByteArrayOutputStream.writeTo` is invoked from a virtual thread. Brian Burkhalter has updated the pull request incrementally with one additional commit since the last revision: Correct ID in test @bug tag - Changes: - all: https://git.openjdk.org/jdk/pull/18901/files - new: https://git.openjdk.org/jdk/pull/18901/files/08bb9cb6..57e4917b Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=18901=01 - incr: https://webrevs.openjdk.org/?repo=jdk=18901=00-01 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.org/jdk/pull/18901.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/18901/head:pull/18901 PR: https://git.openjdk.org/jdk/pull/18901
RFR: 8330748: ByteArrayOutputStream.writeTo(OutputStream) pins carrier
Prevent blocking due to a carrier thread not being released when `ByteArrayOutputStream.writeTo` is invoked from a virtual thread. - Commit messages: - 8330748: ByteArrayOutputStream.writeTo(OutputStream) pins carrier Changes: https://git.openjdk.org/jdk/pull/18901/files Webrev: https://webrevs.openjdk.org/?repo=jdk=18901=00 Issue: https://bugs.openjdk.org/browse/JDK-8330748 Stats: 137 lines in 2 files changed: 134 ins; 0 del; 3 mod Patch: https://git.openjdk.org/jdk/pull/18901.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/18901/head:pull/18901 PR: https://git.openjdk.org/jdk/pull/18901
Re: RFR: 8329593: Drop adjustments to target parallelism when virtual threads do I/O on files opened for buffered I/O [v2]
On Mon, 22 Apr 2024 12:45:53 GMT, Alan Bateman wrote: >> This change drops the adjustments to the virtual thread scheduler's target >> parallelism when virtual threads do file operations on files that are opened >> for buffered I/O. These operations are usually just too short to have any >> benefit and may have a negative benefit when reading/writing a small number >> of bytes. There is no change for read/write operations on files opened for >> direct I/O or when writing to files that are opened with options for >> synchronized I/O file integrity (O_SYNC/O_DSYNC and equivalents). Sergery >> Kuksenko is polishing benchmarks that includes this area, this is for a >> future PR. >> >> In addition, the blocker mechanism is updated to handle reentrancy (as can >> happen if debugging code is added to ForkJoinPool) and preemption when >> compensating (as can happen when substituting a heap buffer with a direct >> buffer in some I/O operations). This part is a pre-requisite to the changes >> to better support object monitor there are more places where preemption is >> possible and this quickly leads to unbalanced begin/end. >> >> The changes have been baking in loom repo for some time. > > Alan Bateman 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 > - Sync up from loom repo, update copyright headers > - Merge > - Merge > - Initial commit Looks fine. I think dropping the `transferTo` overloads for now is good. - Marked as reviewed by bpb (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/18598#pullrequestreview-2015549163
Re: RFR: 8329593: Drop adjustments to target parallelism when virtual threads do I/O on files opened for buffered I/O
On Wed, 3 Apr 2024 10:04:36 GMT, Alan Bateman wrote: > This change drops the adjustments to the virtual thread scheduler's target > parallelism when virtual threads do file operations on files that are opened > for buffered I/O. These operations are usually just too short to have any > benefit and may have a negative benefit when reading/writing a small number > of bytes. There is no change for read/write operations on files opened for > direct I/O or when writing to files that are opened with options for > synchronized I/O file integrity (O_SYNC/O_DSYNC and equivalents). Sergery > Kuksenko is polishing benchmarks that includes this area, this is for a > future PR. > > In addition, the blocker mechanism is updated to handle reentrancy as can > happen if debugging code is added to ForkJoinPool, if there is preemption > when attempting to compensate, or potentially forced preemption in the > future. This part is a pre-requisite to the changes to better support object > monitor there are more places where preemption is possible and this quickly > leads to unbalanced begin/end. > > The changes have been baking in the loom repo for several months. The `CarrierThread` changes look to be the most complicated here but I don't see any problems. Otherwise, I don't have any comments aside from some that @jaikiran already made hence not worth repeating. - Marked as reviewed by bpb (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/18598#pullrequestreview-1990512527
Re: RFR: 8329787: Fix typo in CLDRConverter
On Fri, 5 Apr 2024 17:22:36 GMT, Naoto Sato wrote: > Fix a file/class name in the `CLDRConverter` build tool, with some clean-up > for a switch statement. +1 - Marked as reviewed by bpb (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/18662#pullrequestreview-1984005299
Integrated: 8328183: Minor mistakes in docs of PrintStream.append()
On Wed, 3 Apr 2024 19:01:14 GMT, Brian Burkhalter wrote: > Clarify the behavior of `append` for a `null` `CharSequence` parameter and > clean up a couple of other typos. This pull request has now been integrated. Changeset: 040c9356 Author: Brian Burkhalter URL: https://git.openjdk.org/jdk/commit/040c93565c0dff6270911eb9e58d78aa01bbb925 Stats: 17 lines in 5 files changed: 5 ins; 1 del; 11 mod 8328183: Minor mistakes in docs of PrintStream.append() Reviewed-by: iris, naoto - PR: https://git.openjdk.org/jdk/pull/18607
RFR: 8328183: Minor mistakes in docs of PrintStream.append()
Clarify the behavior of `append` for a `null` `CharSequence` parameter and clean up a couple of other typos. - Commit messages: - 8328183: Minor mistakes in docs of PrintStream.append() Changes: https://git.openjdk.org/jdk/pull/18607/files Webrev: https://webrevs.openjdk.org/?repo=jdk=18607=00 Issue: https://bugs.openjdk.org/browse/JDK-8328183 Stats: 17 lines in 5 files changed: 5 ins; 1 del; 11 mod Patch: https://git.openjdk.org/jdk/pull/18607.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/18607/head:pull/18607 PR: https://git.openjdk.org/jdk/pull/18607
Re: RFR: 8328183: Minor mistakes in docs of PrintStream.append()
On Wed, 3 Apr 2024 19:01:14 GMT, Brian Burkhalter wrote: > Clarify the behavior of `append` for a `null` `CharSequence` parameter and > clean up a couple of other typos. Item 4 in [this comment](https://bugs.openjdk.org/browse/JDK-8328183?focusedId=14657943=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#comment-14657943) is not addressed as this would be a change to very longstanding behavior and might cause unexpected results. - PR Comment: https://git.openjdk.org/jdk/pull/18607#issuecomment-2035372741
Re: RFR: JDK-8329557: Fix statement around MathContext.DECIMAL128 rounding
On Tue, 2 Apr 2024 23:43:24 GMT, Joe Darcy wrote: > Happened to notice a semantic typo in the description of > MathContext.DECIMAL128, use of "decimal64" where "decimal128" was intended, > and added some additional information to make the related descriptions more > informative. Looks fine. - Marked as reviewed by bpb (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/18587#pullrequestreview-1975341809
Re: RFR: 8328700: Unused import and variable should be deleted in regex package
On Fri, 22 Mar 2024 17:58:25 GMT, Raffaello Giulietti wrote: > Make use of an unused local variable probably intended to replace later casts. Marked as reviewed by bpb (Reviewer). - PR Review: https://git.openjdk.org/jdk/pull/18460#pullrequestreview-1955519288
Re: CFV: New Core Libraries Group Member: Per-Ake Minborg
Vote: yes
Re: RFR: 8327786: Add fluent setAccessible()
On Mon, 11 Mar 2024 23:06:53 GMT, Sergey wrote: > What is the "standard protocol" to find a reviewer? Reviewers will generally chime in of their own accord depending on their expertise, interest, availability, the perceived importance of the issue, and so on. - PR Comment: https://git.openjdk.org/jdk/pull/17578#issuecomment-1989628572
Re: RFR: 8327705: Remove mention of "applet" from java.text package description [v2]
On Mon, 11 Mar 2024 16:32:31 GMT, Naoto Sato wrote: >> Removing left over "applet" mention in the package-info doc. > > Naoto Sato has updated the pull request incrementally with one additional > commit since the last revision: > > Addressing review comments Marked as reviewed by bpb (Reviewer). - PR Review: https://git.openjdk.org/jdk/pull/18173#pullrequestreview-1928572972
Re: RFR: 8327705: Remove mention of "applet" from java.text package description
On Fri, 8 Mar 2024 19:54:31 GMT, Naoto Sato wrote: > Removing left over "applet" mention in the package-info doc. Farewell applets. - Marked as reviewed by bpb (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/18173#pullrequestreview-1925759402
Re: RFR: 8327167: Add add() example for Leap Day in Calendar's doc
On Thu, 7 Mar 2024 19:28:13 GMT, Naoto Sato wrote: > A simple doc update to include a leap day example in the `Calendar` class. Looks good. - Marked as reviewed by bpb (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/18158#pullrequestreview-1923550002
Re: RFR: JDK-8327444: simplify RESTARTABLE macro usage in JDK codebase [v4]
On Thu, 7 Mar 2024 08:16:26 GMT, Matthias Baesken wrote: >> We define the RESTARTABLE macro again and again at a lot of places in the >> JDK native codebase. This could be centralized to avoid repeating it again >> and again ! > > Matthias Baesken has updated the pull request incrementally with one > additional commit since the last revision: > > adjust COPYRIGHT year info +1 - Marked as reviewed by bpb (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/18132#pullrequestreview-1923092889
Re: RFR: JDK-8327444: simplify RESTARTABLE macro usage in JDK codebase [v3]
On Wed, 6 Mar 2024 16:30:23 GMT, Matthias Baesken wrote: >> We define the RESTARTABLE macro again and again at a lot of places in the >> JDK native codebase. This could be centralized to avoid repeating it again >> and again ! > > Matthias Baesken has updated the pull request incrementally with one > additional commit since the last revision: > > Introduce windows jni_util_md.h src/java.base/unix/native/libnio/ch/Net.c line 2: > 1: /* > 2: * Copyright (c) 2001, 2024, Oracle and/or its affiliates. All rights > reserved. Is the year change needed as it looks like nothing was changed? - PR Review Comment: https://git.openjdk.org/jdk/pull/18132#discussion_r1515273416
Re: RFR: 8327261: Parsing test for Double/Float succeeds w/o testing all bad cases [v2]
On Tue, 5 Mar 2024 17:26:11 GMT, Naoto Sato wrote: >> Fixing test cases. For bad test cases, only the first case was run, and the >> rest were ignored. > > Naoto Sato has updated the pull request incrementally with one additional > commit since the last revision: > > Reflects review comments +1 - Marked as reviewed by bpb (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/18113#pullrequestreview-1917870069
Re: RFR: 8327225: Revert DataInputStream.readUTF to static final
On Mon, 4 Mar 2024 13:55:15 GMT, Claes Redestad wrote: > [JDK-8325340](https://bugs.openjdk.org/browse/JDK-8325340) accidentally > removed `final` from the `static final DataInputStream.readUTF` method. This > has a minor compatibility impact (allows hiding the method in a subclass, > while before that would throw an exception at compile time) and since it was > not the intent of the prior change to alter any behavioral semantics here I > want to revert that change. Marked as reviewed by bpb (Reviewer). - PR Review: https://git.openjdk.org/jdk/pull/18107#pullrequestreview-191489
Result: New Core Libraries Group Member: Raffaello Giulietti
The vote for Raffaello Giulietti [1] is now closed. Yes: 14 Veto: 0 Abstain: 0 According to the Bylaws definition of Lazy Consensus, this is sufficient to approve the nomination. Brian Burkhalter [1] https://mail.openjdk.org/pipermail/core-libs-dev/2024-February/119208.html
Re: RFR: JDK-8326530: Widen allowable error bound of Math.tan
On Thu, 22 Feb 2024 22:00:26 GMT, Joe Darcy wrote: > Widen acceptable error bound of Math.tan to accommodate the worst-case > observed error which is slightly outside of the allowable range. Looks fine. - Marked as reviewed by bpb (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/17973#pullrequestreview-1897113120
Re: CFV: New Core Libraries Group Member: Viktor Klang
Vote: yes
Re: RFR: 8325340: Add ASCII fast-path to Data-/ObjectInputStream.readUTF [v7]
On Fri, 16 Feb 2024 15:31:07 GMT, Claes Redestad wrote: >> Adding a fast-path for ASCII-only modified UTF-8 strings deserialied via >> Data- and ObjectInputStream >> >> Testing: tier1-3 > > Claes Redestad has updated the pull request incrementally with two additional > commits since the last revision: > > - Update test/micro/org/openjdk/bench/java/io/DataInputStreamTest.java > >Co-authored-by: Raffaello Giulietti > - Update test/micro/org/openjdk/bench/java/io/ObjectInputStreamTest.java > >Co-authored-by: Raffaello Giulietti Marked as reviewed by bpb (Reviewer). - PR Review: https://git.openjdk.org/jdk/pull/17734#pullrequestreview-1885630906
Integrated: 8325990: Remove use of snippet @replace annotation in java.base
On Thu, 15 Feb 2024 18:54:46 GMT, Brian Burkhalter wrote: > Revert `@replace` snippet annotation construct to value of `replacement` > attribute. This pull request has now been integrated. Changeset: 7a762520 Author: Brian Burkhalter URL: https://git.openjdk.org/jdk/commit/7a76252007b603b4346fad61818d488999644f80 Stats: 5 lines in 2 files changed: 0 ins; 0 del; 5 mod 8325990: Remove use of snippet @replace annotation in java.base Reviewed-by: jlu, naoto - PR: https://git.openjdk.org/jdk/pull/17882
RFR: 8325990: Remove use of snippet @replace annotation in java.base
Revert `@replace` snippet annotation construct to value of `replacement` attribute. - Commit messages: - 8325990: Remove use of snippet @replace annotation in java.base Changes: https://git.openjdk.org/jdk/pull/17882/files Webrev: https://webrevs.openjdk.org/?repo=jdk=17882=00 Issue: https://bugs.openjdk.org/browse/JDK-8325990 Stats: 5 lines in 2 files changed: 0 ins; 0 del; 5 mod Patch: https://git.openjdk.org/jdk/pull/17882.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/17882/head:pull/17882 PR: https://git.openjdk.org/jdk/pull/17882
Re: RFR: 8325340: Add ASCII fast-path to Data-/ObjectInputStream.readUTF
On Tue, 6 Feb 2024 16:17:21 GMT, Claes Redestad wrote: > Adding a fast-path for ASCII-only modified UTF-8 strings deserialied via > Data- and ObjectInputStream > > Testing: tier1-3 As there are no regression tests added by this request, I assume that existing tests must sufficiently cover this area. If so, however, the issue has no `noreg-` label. - PR Comment: https://git.openjdk.org/jdk/pull/17734#issuecomment-1944507481
Re: RFR: 8325340: Add ASCII fast-path to Data-/ObjectInputStream.readUTF
On Wed, 14 Feb 2024 10:41:08 GMT, Raffaello Giulietti
wrote:
>> Adding a fast-path for ASCII-only modified UTF-8 strings deserialied via
>> Data- and ObjectInputStream
>>
>> Testing: tier1-3
>
> src/java.base/share/classes/java/io/DataInputStream.java line 585:
>
>> 583: DataInputStream dis = null;
>> 584: if (in instanceof DataInputStream) {
>> 585: dis = (DataInputStream)in;
>
> I guess that not making use of `instanceof` pattern matching is to enable
> backporting before JDK 16?
I have the same question here.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/17734#discussion_r1489994945
CFV: New Core Libraries Group Member: Raffaello Giulietti
I hereby nominate Raffaello Giulietti to Membership in the Core Libraries Group. Raffaello has been working in the Core Library team at Oracle since April, 2022. He has authored more than 50 contributions to OpenJDK in a number of areas including numerics, formatting, regular expressions, and random number generation. Votes are due by Wednesday, February 28, 2023. Only current Members of the Core Libraries Group [1] are eligible to vote on this nomination. Votes must be cast in the open by replying to this mailing list For Lazy Consensus voting instructions, see [2]. Brian Burkhalter [1] https://openjdk.org/census#core-libs [2] https://openjdk.org/groups/#member-vote
Integrated: 8325152: Clarify specification of java.io.RandomAccessFile.setLength
On Fri, 2 Feb 2024 00:38:51 GMT, Brian Burkhalter wrote: > Modify the specification verbiage of `java.io.RandomAccessFile.setLength` to > account for the effect of the method on the file offset as returned by > `getFilePointer`. This pull request has now been integrated. Changeset: 4b1e367e Author: Brian Burkhalter URL: https://git.openjdk.org/jdk/commit/4b1e367edabb3c12359abc2d7815559b9ece9fe3 Stats: 20 lines in 1 file changed: 9 ins; 2 del; 9 mod 8325152: Clarify specification of java.io.RandomAccessFile.setLength Reviewed-by: alanb - PR: https://git.openjdk.org/jdk/pull/17679
Integrated: 8324960: Unsafe.allocateMemory documentation incorrect regarding zero return value
On Wed, 31 Jan 2024 01:17:07 GMT, Brian Burkhalter wrote: > Align the specification of `Unsafe.allocateMemory` with its implementation > and with the specification of `Unsafe.reallocateMemory`. This pull request has now been integrated. Changeset: 209d87a8 Author: Brian Burkhalter URL: https://git.openjdk.org/jdk/commit/209d87a856b1a7bd60910b517d8ff5beb322ec0b Stats: 10 lines in 2 files changed: 2 ins; 0 del; 8 mod 8324960: Unsafe.allocateMemory documentation incorrect regarding zero return value Reviewed-by: rriggs - PR: https://git.openjdk.org/jdk/pull/17643
Re: RFR: 8325152: Clarify specification of java.io.RandomAccessFile.setLength [v2]
> Modify the specification verbiage of `java.io.RandomAccessFile.setLength` to > account for the effect of the method on the file offset as returned by > `getFilePointer`. Brian Burkhalter has updated the pull request incrementally with one additional commit since the last revision: 8325152: Minor change to paragraph about file offset - Changes: - all: https://git.openjdk.org/jdk/pull/17679/files - new: https://git.openjdk.org/jdk/pull/17679/files/80e562f9..5eaa8dda Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=17679=01 - incr: https://webrevs.openjdk.org/?repo=jdk=17679=00-01 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.org/jdk/pull/17679.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/17679/head:pull/17679 PR: https://git.openjdk.org/jdk/pull/17679
RFR: 8325152: Clarify specification of java.io.RandomAccessFile.setLength
Modify the specification verbiage of `java.io.RandomAccessFile.setLength` to account for the effect of the method on the file offset as returned by `getFilePointer`. - Commit messages: - 8325152: Clarify specification of java.io.RandomAccessFile.setLength Changes: https://git.openjdk.org/jdk/pull/17679/files Webrev: https://webrevs.openjdk.org/?repo=jdk=17679=00 Issue: https://bugs.openjdk.org/browse/JDK-8325152 Stats: 20 lines in 1 file changed: 9 ins; 2 del; 9 mod Patch: https://git.openjdk.org/jdk/pull/17679.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/17679/head:pull/17679 PR: https://git.openjdk.org/jdk/pull/17679
Re: RFR: 8324960: Unsafe.allocateMemory documentation incorrect regarding zero return value
On Wed, 31 Jan 2024 01:17:07 GMT, Brian Burkhalter wrote: > Align the specification of `Unsafe.allocateMemory` with its implementation > and with the specification of `Unsafe.reallocateMemory`. CSR created. - PR Comment: https://git.openjdk.org/jdk/pull/17643#issuecomment-1922281572
Re: RFR: 8324960: Unsafe.allocateMemory documentation incorrect regarding zero return value
On Thu, 1 Feb 2024 17:01:11 GMT, Roger Riggs wrote: > lgtm Thanks @RogerRiggs. - PR Comment: https://git.openjdk.org/jdk/pull/17643#issuecomment-1922050326
RFR: 8324960: Unsafe.allocateMemory documentation incorrect regarding zero return value
Align the specification of `Unsafe.allocateMemory` with its implementation and with the specification of `Unsafe.reallocateMemory`. - Commit messages: - 8324960: Unsafe.allocateMemory documentation incorrect regarding zero return value Changes: https://git.openjdk.org/jdk/pull/17643/files Webrev: https://webrevs.openjdk.org/?repo=jdk=17643=00 Issue: https://bugs.openjdk.org/browse/JDK-8324960 Stats: 10 lines in 2 files changed: 2 ins; 0 del; 8 mod Patch: https://git.openjdk.org/jdk/pull/17643.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/17643/head:pull/17643 PR: https://git.openjdk.org/jdk/pull/17643
Re: RFR: JDK-8323186: A faster algorithm for MutablebigInteger.divWord(long, int) [v4]
On Mon, 22 Jan 2024 21:21:53 GMT, fabioromano1 wrote:
>> test/jdk/java/math/BigInteger/TestDivWord.java line 5:
>>
>>> 3: import java.util.Random;
>>> 4:
>>> 5: public class TestDivWord {
>>
>> Where is this used in actually jtreg testing?
>
> @bplb It is not used in jtreg testing.
So it is only for verification purposes in the context of this PR?
-
PR Review Comment: https://git.openjdk.org/jdk/pull/17291#discussion_r1462441622
Re: RFR: 8320759: Creation of random BigIntegers can be made faster [v3]
On Sat, 2 Dec 2023 15:42:18 GMT, fabioromano1 wrote: >> A faster and simpler way to generate random BigIntegers, avoiding eventually >> trimming of leading zeros in magnitude array. > > fabioromano1 has updated the pull request incrementally with one additional > commit since the last revision: > > Create RandomBigIntegers.java > > Create a benchmark to measure the performance of BigInteger(int, Random) > constructor implementation. Will more work be done on this PR? - PR Comment: https://git.openjdk.org/jdk/pull/16817#issuecomment-1904812784
Re: RFR: JDK-8323186: A faster algorithm for MutablebigInteger.divWord(long, int) [v4]
On Mon, 22 Jan 2024 18:52:43 GMT, fabioromano1 wrote:
>> The method `MutableBigInteger.divWord(long, int)` can use the algorithm of
>> Hacker's Delight (2nd ed), section 9.3, the same used in
>> `Long.divideUnsigned(long, long)` and `Long.remainderUnsigned(long, long)`,
>> to get the computation faster.
>
> fabioromano1 has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Update TestDivWord.java
>
> Added method to check results of divWord
test/jdk/java/math/BigInteger/TestDivWord.java line 5:
> 3: import java.util.Random;
> 4:
> 5: public class TestDivWord {
Where is this used in actually jtreg testing?
-
PR Review Comment: https://git.openjdk.org/jdk/pull/17291#discussion_r1462355967
Re: RFR: 8320788: The system properties page is missing some properties [v2]
On Tue, 9 Jan 2024 19:23:53 GMT, Naoto Sato wrote: >> Adding an explanation of the locale-related system properties in the >> `System.getProperties()` method. Corresponding CSR has also been drafted. > > Naoto Sato has updated the pull request incrementally with one additional > commit since the last revision: > > Reflects review comments +1 - Marked as reviewed by bpb (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/17317#pullrequestreview-1811961077
Re: RFR: JDK-8323186: A faster algorithm for MutablebigInteger.divWord(long, int)
On Sat, 6 Jan 2024 18:01:01 GMT, fabioromano1 wrote: > The method `MutableBigInteger.divWord(long, int)` can use the algorithm of > Hacker's Delight (2nd ed), section 9.3, the same used in > `Long.divideUnsigned(long, long)` and `Long.remainderUnsigned(long, long)`, > to get the computation faster. Do you have any benchmark results demonstrating the increased throughput? - PR Comment: https://git.openjdk.org/jdk/pull/17291#issuecomment-1881601270
Re: RFR: 8322877: java/io/BufferedInputStream/TransferTo.java failed with IndexOutOfBoundsException
On Thu, 4 Jan 2024 18:37:59 GMT, Markus KARG wrote: >> No: the third param of >> [Arrays.copyOfRange](https://docs.oracle.com/en/java/javase/21/docs/api/java.base/java/util/Arrays.html#copyOfRange(byte[],int,int)) >> is `to`, not `len`. > > Ah, this explains why it did not fail originally, but only after adding the > "isTrusted" case! See https://github.com/openjdk/jdk/pull/17250#issuecomment-1875761630 above. - PR Review Comment: https://git.openjdk.org/jdk/pull/17250#discussion_r1442126261
Re: RFR: 8322877: java/io/BufferedInputStream/TransferTo.java failed with IndexOutOfBoundsException
On Thu, 4 Jan 2024 18:32:55 GMT, Markus KARG wrote:
>> The final position instead of the number of bytes to write was being passed
>> to `ByteArrayOuputStream.write(byte[],int,int)`.
>
> src/java.base/share/classes/java/io/BufferedInputStream.java line 650:
>
>> 648: } else {
>> 649: // Prevent poisoning and leaking of buf
>> 650: byte[] buffer = Arrays.copyOfRange(getBufIfOpen(),
>> pos, count);
>
> @bplb Shouldn't it be `avail` *here*, too?
No: the third param of
[Arrays.copyOfRange](https://docs.oracle.com/en/java/javase/21/docs/api/java.base/java/util/Arrays.html#copyOfRange(byte[],int,int))
is `to`, not `len`.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/17250#discussion_r1442122428
Integrated: 8322877: java/io/BufferedInputStream/TransferTo.java failed with IndexOutOfBoundsException
On Wed, 3 Jan 2024 18:01:59 GMT, Brian Burkhalter wrote: > The final position instead of the number of bytes to write was being passed > to `ByteArrayOuputStream.write(byte[],int,int)`. This pull request has now been integrated. Changeset: 54b3ceec Author: Brian Burkhalter URL: https://git.openjdk.org/jdk/commit/54b3ceeca27b67f4270d8b700b072f46959dba65 Stats: 3 lines in 2 files changed: 0 ins; 1 del; 2 mod 8322877: java/io/BufferedInputStream/TransferTo.java failed with IndexOutOfBoundsException Reviewed-by: alanb, stsypanov - PR: https://git.openjdk.org/jdk/pull/17250
RFR: 8322877: java/io/BufferedInputStream/TransferTo.java failed with IndexOutOfBoundsException
The final position instead of the number of bytes to write was being passed to `ByteArrayOuputStream.write(byte[],int,int)`. - Commit messages: - 8322877: java/io/BufferedInputStream/TransferTo.java failed with IndexOutOfBoundsException Changes: https://git.openjdk.org/jdk/pull/17250/files Webrev: https://webrevs.openjdk.org/?repo=jdk=17250=00 Issue: https://bugs.openjdk.org/browse/JDK-8322877 Stats: 3 lines in 2 files changed: 0 ins; 1 del; 2 mod Patch: https://git.openjdk.org/jdk/pull/17250.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/17250/head:pull/17250 PR: https://git.openjdk.org/jdk/pull/17250
Re: RFR: 8322877: java/io/BufferedInputStream/TransferTo.java failed with IndexOutOfBoundsException
On Wed, 3 Jan 2024 18:01:59 GMT, Brian Burkhalter wrote: > The final position instead of the number of bytes to write was being passed > to `ByteArrayOuputStream.write(byte[],int,int)`. Everyone was apparently caught off guard as previously `Arrays.copyOfRange(byte[],int,int)` had been used here, and its third parameter is the `to` position, not the number of bytes to copy. - PR Comment: https://git.openjdk.org/jdk/pull/17250#issuecomment-1875761630
Re: Integrated: 8322963: ProblemList java/io/BufferedInputStream/TransferTo.java
On Wed, 3 Jan 2024 17:13:02 GMT, Daniel D. Daugherty wrote: > A trivial fix to ProblemList java/io/BufferedInputStream/TransferTo.java. Marked as reviewed by bpb (Reviewer). - PR Review: https://git.openjdk.org/jdk/pull/17249#pullrequestreview-1802749494
Integrated: 8322868: java/io/BufferedInputStream/TransferToTrusted.java has bad copyright header
On Tue, 2 Jan 2024 20:27:29 GMT, Brian Burkhalter wrote: > Add missing comma after 2024. This pull request has now been integrated. Changeset: c2477a5c Author: Brian Burkhalter URL: https://git.openjdk.org/jdk/commit/c2477a5cad6539e6e38cc0732383aaa2a8df801f Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod 8322868: java/io/BufferedInputStream/TransferToTrusted.java has bad copyright header Reviewed-by: dcubed - PR: https://git.openjdk.org/jdk/pull/17228
RFR: 8322868: java/io/BufferedInputStream/TransferToTrusted.java has bad copyright header
Add missing comma after 2024. - Commit messages: - 8322868: java/io/BufferedInputStream/TransferToTrusted.java has bad copyright header Changes: https://git.openjdk.org/jdk/pull/17228/files Webrev: https://webrevs.openjdk.org/?repo=jdk=17228=00 Issue: https://bugs.openjdk.org/browse/JDK-8322868 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.org/jdk/pull/17228.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/17228/head:pull/17228 PR: https://git.openjdk.org/jdk/pull/17228
Re: RFR: 8320971: Use BufferedInputStream.buf directly when param of implTransferTo() is trusted [v20]
On Tue, 2 Jan 2024 20:00:36 GMT, Sergey Tsypanov wrote: >> It looks like we can skip copying of `byte[]` in >> `BufferedInputStream.implTransferTo()` for `OutputStreams` residing in >> `java.io`. >> >> See comment by @vlsi in >> https://github.com/openjdk/jdk/pull/10525/files#diff-e19c508d1bb6ee78697ecca66947c395adda0d9c49a85bf696e677ecbd977af1R612 > > Sergey Tsypanov has updated the pull request incrementally with one > additional commit since the last revision: > > Update TransferToTrusted.java Marked as reviewed by bpb (Reviewer). - PR Review: https://git.openjdk.org/jdk/pull/16879#pullrequestreview-1800941486
Re: RFR: 8320971: Use BufferedInputStream.buf directly when param of implTransferTo() is trusted [v16]
On Thu, 21 Dec 2023 17:29:16 GMT, Brian Burkhalter wrote: >> Sergey Tsypanov 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 20 additional >> commits since the last revision: >> >> - 8322292: Remove TransferToTrusted.checkedOutputStream() >> - 8320971: Adjust JavaDoc >> - Merge branch 'master' into 8320971 >> - 8320971: Revert irrelevant changes >> - 8320971: Add more tests >> - 8320971: Fix JavaDoc >> - 8320971: Whitespaces >> - 8320971: Fix build >> - 8320971: Move IOStreams to com.sun.io >> - Merge branch 'master' into 8320971 >> - ... and 10 more: https://git.openjdk.org/jdk/compare/aed73e30...84686bc6 > > The test does not fail when run against the mainline (HEAD: Thu Dec 21 > 15:20:01 2023 + UTC). As previously mentioned elsewhere, normally a > deterministic test should fail before the proposed source patch is applied, > and succeed thereafter. > @bplb has been reviewing the test, I'll stay out of that and let Brian finish > his review. I think that the test looks all right, but the copyright might need to be changed to `2023, 2024` instead of just `2023` as it is a new file. The source file does not need `2024` if no changes have been made in the last two days. - PR Comment: https://git.openjdk.org/jdk/pull/16879#issuecomment-1874366815
Re: RFR: 8320971: Use BufferedInputStream.buf directly when param of implTransferTo() is trusted [v16]
On Tue, 19 Dec 2023 10:18:11 GMT, Sergey Tsypanov wrote: >> It looks like we can skip copying of `byte[]` in >> `BufferedInputStream.implTransferTo()` for `OutputStreams` residing in >> `java.io`. >> >> See comment by @vlsi in >> https://github.com/openjdk/jdk/pull/10525/files#diff-e19c508d1bb6ee78697ecca66947c395adda0d9c49a85bf696e677ecbd977af1R612 > > Sergey Tsypanov 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 20 additional > commits since the last revision: > > - 8322292: Remove TransferToTrusted.checkedOutputStream() > - 8320971: Adjust JavaDoc > - Merge branch 'master' into 8320971 > - 8320971: Revert irrelevant changes > - 8320971: Add more tests > - 8320971: Fix JavaDoc > - 8320971: Whitespaces > - 8320971: Fix build > - 8320971: Move IOStreams to com.sun.io > - Merge branch 'master' into 8320971 > - ... and 10 more: https://git.openjdk.org/jdk/compare/54ca8c60...84686bc6 The test does not fail when run against the mainline (HEAD: Thu Dec 21 15:20:01 2023 + UTC). As previously mentioned elsewhere, normally a deterministic test should fail before the proposed source patch is applied, and succeed thereafter. - PR Comment: https://git.openjdk.org/jdk/pull/16879#issuecomment-1866685521
Re: RFR: JDK-8322141: SequenceInputStream.transferTo should not return as soon as Long.MAX_VALUE bytes have been transferred [v4]
On Wed, 20 Dec 2023 22:46:32 GMT, Markus KARG wrote: > So I did not do something wrong but finally I learned Github forensics now. If no crime was committed, I would call it archaeology. - PR Comment: https://git.openjdk.org/jdk/pull/17119#issuecomment-1865249242
Re: RFR: JDK-8322141: SequenceInputStream.transferTo should not return as soon as Long.MAX_VALUE bytes have been transferred [v4]
On Wed, 20 Dec 2023 18:28:27 GMT, Markus KARG wrote: > this icon only says that Alan made change requests I think it's if the "changes requested" button is selected in the "Submit Review" popup under "Files Changed." - PR Comment: https://git.openjdk.org/jdk/pull/17119#issuecomment-1864976239
Re: RFR: JDK-8322141: SequenceInputStream.transferTo should not return as soon as Long.MAX_VALUE bytes have been transferred [v4]
On Sat, 16 Dec 2023 14:07:52 GMT, Markus KARG wrote: >> Fixes JDK-8322141 >> >> As suggested by @vlsi and documented by @bplb this PR does not return, but >> only sets the maximum result value. > > Markus KARG has updated the pull request incrementally with one additional > commit since the last revision: > > Test was using Integer but must use Long > But where do you see a change request from Alan _before_ I posted > `/integrate`? Mouse-over the icon to the right of his name at the top right under "Reviewers." > BTW, here is the "Ready" label you did not see: [#17119 > (comment)](https://github.com/openjdk/jdk/pull/17119#event-11256913866) Ack. - PR Comment: https://git.openjdk.org/jdk/pull/17119#issuecomment-1864901444
Re: RFR: JDK-8322141: SequenceInputStream.transferTo should not return as soon as Long.MAX_VALUE bytes have been transferred [v4]
On Wed, 20 Dec 2023 00:56:16 GMT, Jaikiran Pai wrote: > The updated source change looks fine to me. @jaikiran Thanks for the corroboration. - PR Comment: https://git.openjdk.org/jdk/pull/17119#issuecomment-1864827299
Re: RFR: JDK-8322141: SequenceInputStream.transferTo should not return as soon as Long.MAX_VALUE bytes have been transferred [v4]
On Wed, 20 Dec 2023 16:52:38 GMT, Brian Burkhalter wrote: >>> Approved. >>> >>> Please note in future that it is better not to `/integrate` until the >>> request has actually been approved by a Reviewer. >> >> Alan actually did approve them on December 15. > >> Alan actually did approve them on December 15. > > I know, with requested changes. I didn't see the request transition to > "Ready" which is where it needs to be to `/integrate`. > @bplb Apparently Skara wants you to repeat your sponsoring again. Yes, because the PR was updated since the previous `/integrate`. - PR Comment: https://git.openjdk.org/jdk/pull/17119#issuecomment-1864817465
Re: RFR: JDK-8322141: SequenceInputStream.transferTo should not return as soon as Long.MAX_VALUE bytes have been transferred [v4]
On Wed, 20 Dec 2023 09:47:05 GMT, Markus KARG wrote: > Alan actually did approve them on December 15. I know, with requested changes. I didn't see the request transition to "Ready" which is where it needs to be to `/integrate`. - PR Comment: https://git.openjdk.org/jdk/pull/17119#issuecomment-1864815013
Re: RFR: JDK-8322141: SequenceInputStream.transferTo should not return as soon as Long.MAX_VALUE bytes have been transferred [v4]
On Sat, 16 Dec 2023 14:07:52 GMT, Markus KARG wrote: >> Fixes JDK-8322141 >> >> As suggested by @vlsi and documented by @bplb this PR does not return, but >> only sets the maximum result value. > > Markus KARG has updated the pull request incrementally with one additional > commit since the last revision: > > Test was using Integer but must use Long I verified that the test now fails without the source change but passes with it. - PR Comment: https://git.openjdk.org/jdk/pull/17119#issuecomment-1863284534
Re: RFR: JDK-8322141: SequenceInputStream.transferTo should not return as soon as Long.MAX_VALUE bytes have been transferred [v4]
On Sat, 16 Dec 2023 14:07:52 GMT, Markus KARG wrote: >> Fixes JDK-8322141 >> >> As suggested by @vlsi and documented by @bplb this PR does not return, but >> only sets the maximum result value. > > Markus KARG has updated the pull request incrementally with one additional > commit since the last revision: > > Test was using Integer but must use Long Approved. Please note in future that it is better not to `/integrate` until the request has actually been approved by a Reviewer. - Marked as reviewed by bpb (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/17119#pullrequestreview-1789475686
Re: RFR: 8320971: Use BufferedInputStream.buf directly when param of implTransferTo() is trusted [v15]
On Tue, 19 Dec 2023 10:14:44 GMT, Sergey Tsypanov wrote: > Shouldn't we keep at least the method for the classes checked in > `BIS.isTrusted()` You can keep it there or inline it. As Alan noted, the main thing is not to add something like a new package. - PR Review Comment: https://git.openjdk.org/jdk/pull/16879#discussion_r1431779354
Re: RFR: 8259637: java.io.File.getCanonicalPath() returns different values for same path [v2]
> Modify the `collapse()` function to remove each instance of ".." when the > path is absolute and there is no preceding name. Brian Burkhalter has updated the pull request incrementally with one additional commit since the last revision: 8259637: Use Stream.of in test - Changes: - all: https://git.openjdk.org/jdk/pull/17089/files - new: https://git.openjdk.org/jdk/pull/17089/files/d2f755b0..cd2b3776 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=17089=01 - incr: https://webrevs.openjdk.org/?repo=jdk=17089=00-01 Stats: 14 lines in 1 file changed: 0 ins; 7 del; 7 mod Patch: https://git.openjdk.org/jdk/pull/17089.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/17089/head:pull/17089 PR: https://git.openjdk.org/jdk/pull/17089
Re: RFR: 8259637: java.io.File.getCanonicalPath() returns different values for same path [v2]
On Sun, 17 Dec 2023 08:53:15 GMT, Alan Bateman wrote:
>> Brian Burkhalter has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> 8259637: Use Stream.of in test
>
> test/jdk/java/io/File/GetCanonicalPath.java line 98:
>
>> 96: "/b/c"));
>> 97:
>> 98: return list.stream();
>
> You could use Stream.of here, e.g.
>
>
> return Stream.of(
> Arguments.of("/../../../../../a/b/c", "/a/b/c"),
> Arguments.of("/../../../../../a/../b/c", "/b/c"),
> Arguments.of("/../../../../../a/../../b/c", "/b/c"),
> Arguments.of("/../../../../../a/../../../b/c", "/b/c"),
> Arguments.of("/../../../../../a/../../../../b/c", "/b/c")
> );
So changed in cd2b3776463ddc95b36f7b65be12da7ac2686c53.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/17089#discussion_r1430388844
Integrated: 8259637: java.io.File.getCanonicalPath() returns different values for same path
On Wed, 13 Dec 2023 19:37:15 GMT, Brian Burkhalter wrote: > Modify the `collapse()` function to remove each instance of ".." when the > path is absolute and there is no preceding name. This pull request has now been integrated. Changeset: b98d13fc Author:Brian Burkhalter URL: https://git.openjdk.org/jdk/commit/b98d13fc3c7f6747d9201eb884cf9d3181671ccb Stats: 37 lines in 2 files changed: 30 ins; 1 del; 6 mod 8259637: java.io.File.getCanonicalPath() returns different values for same path Reviewed-by: alanb - PR: https://git.openjdk.org/jdk/pull/17089
Re: RFR: 8320971: Use BufferedInputStream.buf directly when param of implTransferTo() is trusted [v15]
On Fri, 15 Dec 2023 19:35:18 GMT, Sergey Tsypanov wrote: > If we drop the method for now I have to write it later again, I guess Maybe, but it's archived so it's easy enough. - PR Review Comment: https://git.openjdk.org/jdk/pull/16879#discussion_r1428601842
Re: RFR: JDK-8322141: SequenceInputStream.transferTo should not return as soon as Long.MAX_VALUE bytes have been transferred [v3]
On Fri, 15 Dec 2023 18:34:42 GMT, Markus KARG wrote: > I quickly drafted an absolute minimal test for this in [2aaac63] [...]. This test does not fail for me when run against my local build of the current mainline. Usually, unless randomness is involved, a test should fail before the proposed patch is applied but succeed thereafter. - PR Comment: https://git.openjdk.org/jdk/pull/17119#issuecomment-1858574102
Re: RFR: JDK-8322141: SequenceInputStream.transferTo should not return as soon as Long.MAX_VALUE bytes have been transferred [v2]
On Fri, 15 Dec 2023 09:55:36 GMT, Alan Bateman wrote: > [...] maybe we need to come up with tests that quickly check the handling at > this limit. @mkarg Do you intend to provide any such test as part of this request? - PR Comment: https://git.openjdk.org/jdk/pull/17119#issuecomment-1858308908
Re: [jdk22] RFR: 8321958: @param/@return descriptions of ZoneRules#isDaylightSavings() are incorrect
On Fri, 15 Dec 2023 17:41:43 GMT, Naoto Sato wrote: > 8321958: @param/@return descriptions of ZoneRules#isDaylightSavings() are > incorrect Marked as reviewed by bpb (Reviewer). - PR Review: https://git.openjdk.org/jdk22/pull/16#pullrequestreview-1784673248
Re: RFR: 8320971: Use BufferedInputStream.buf directly when param of implTransferTo() is trusted [v15]
On Fri, 15 Dec 2023 09:25:00 GMT, Sergey Tsypanov wrote: > I think this would be doing double job, wouldn't it? Sorry, I don't understand. - PR Review Comment: https://git.openjdk.org/jdk/pull/16879#discussion_r1428236374
Re: RFR: 8297632: InputStream.transferTo() method should specify what the return value should be when the number of bytes transfered is larger than Long.MAX_VALUE [v7]
On Thu, 14 Dec 2023 21:54:53 GMT, Markus KARG wrote: >> I mean SequenceInputStream, not the base class: >> https://github.com/openjdk/jdk/blob/c328f9589ddc3a981a2c63801bd991f8e593e69f/src/java.base/share/classes/java/io/SequenceInputStream.java#L249 >> >> Could you please double-check? > > IMHO @vlsi is right. It is incorrect that we stop the transfer in the > overflow case. We should fix it as he suggested. I can do that if you like. Right, the base class. The suggested change was made for `InputStream` in 254d6990bcf75700f1c3aa18e0f8b73b639d9bad. It looks like it should be in `SequenceInputStream` as well. I created [JDK-8322141](https://bugs.openjdk.org/browse/JDK-8322141) to track this. - PR Review Comment: https://git.openjdk.org/jdk/pull/11403#discussion_r1427398726
Re: RFR: 8320971: Use BufferedInputStream.buf directly when param of implTransferTo() is trusted [v15]
On Thu, 14 Dec 2023 08:47:03 GMT, Sergey Tsypanov wrote:
>> It looks like we can skip copying of `byte[]` in
>> `BufferedInputStream.implTransferTo()` for `OutputStreams` residing in
>> `java.io`.
>>
>> See comment by @vlsi in
>> https://github.com/openjdk/jdk/pull/10525/files#diff-e19c508d1bb6ee78697ecca66947c395adda0d9c49a85bf696e677ecbd977af1R612
>
> Sergey Tsypanov has updated the pull request incrementally with one
> additional commit since the last revision:
>
> 8320971: Revert irrelevant changes
src/java.base/share/classes/java/io/BufferedInputStream.java line 646:
> 644: int avail = count - pos;
> 645: if (avail > 0) {
> 646: if (isTrusted(out)) {
It might be cleaner for now to drop `isTrusted()` and just do the class check
explicitly here. That still takes care of the main intent of not passing the
buffer to an untrustworthy stream. Further cleanup and consolidation can be
done later.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/16879#discussion_r1427391530
Re: RFR: 8297632: InputStream.transferTo() method should specify what the return value should be when the number of bytes transfered is larger than Long.MAX_VALUE [v7]
On Thu, 14 Dec 2023 06:12:40 GMT, Vladimir Sitnikov wrote: > What do you think? I think that you are looking at an obsolete repository: https://github.com/openjdk/jdk/blob/c328f9589ddc3a981a2c63801bd991f8e593e69f/src/java.base/share/classes/java/io/InputStream.java#L802 - PR Review Comment: https://git.openjdk.org/jdk/pull/11403#discussion_r1427171307
Re: RFR: 8259637: java.io.File.getCanonicalPath() returns different values for same path
On Wed, 13 Dec 2023 19:37:15 GMT, Brian Burkhalter wrote: > Modify the `collapse()` function to remove each instance of ".." when the > path is absolute and there is no preceding name. Without this change the updated test fails as: FAILED GetCanonicalPath::goodPathsUnix '[3] /../../../../../a/../../b/c, /b/c' org.opentest4j.AssertionFailedError: expected: but was: FAILED GetCanonicalPath::goodPathsUnix '[5] /../../../../../a/../../../../b/c, /b/c' org.opentest4j.AssertionFailedError: expected: but was: - PR Comment: https://git.openjdk.org/jdk/pull/17089#issuecomment-1854591185
RFR: 8259637: java.io.File.getCanonicalPath() returns different values for same path
Modify the `collapse()` function to remove each instance of ".." when the path is absolute and there is no preceding name. - Commit messages: - 8259637: java.io.File.getCanonicalPath() returns different values for same path Changes: https://git.openjdk.org/jdk/pull/17089/files Webrev: https://webrevs.openjdk.org/?repo=jdk=17089=00 Issue: https://bugs.openjdk.org/browse/JDK-8259637 Stats: 44 lines in 2 files changed: 37 ins; 1 del; 6 mod Patch: https://git.openjdk.org/jdk/pull/17089.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/17089/head:pull/17089 PR: https://git.openjdk.org/jdk/pull/17089
Re: RFR: 8320759: Creation of random BigIntegers can be made faster [v3]
On Tue, 5 Dec 2023 22:01:04 GMT, Brian Burkhalter wrote: >> fabioromano1 has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Create RandomBigIntegers.java >> >> Create a benchmark to measure the performance of BigInteger(int, Random) >> constructor implementation. > > So, item 1 is a non-issue and item 2 is not likely a problem in practice. > What, if anything, will be done about item 3? > @bplb Maybe an assertion at the end of `randomBits(int, Random)` method, or a > test class. @fabioromano1 A test class would be better as then we would be able to catch any problems during routine testing. Thanks! - PR Comment: https://git.openjdk.org/jdk/pull/16817#issuecomment-1843266956
