Re: RFR: 8288173: JDK-8202449 fix causes conformance test failure : api/java_util/Random/RandomGenerator/NextFloat.html
On Fri, 10 Jun 2022 08:36:57 GMT, Raffaello Giulietti wrote: > This fixes a bug introduced with JDK-8202449. Marked as reviewed by bpb (Reviewer). - PR: https://git.openjdk.org/jdk/pull/9120
Re: RFR: 6478546: FileInputStream.read() throws OutOfMemoryError when there is plenty available [v5]
> Modify native multi-byte read-write code used by the `java.io` classes to > limit the size of the allocated native buffer thereby decreasing off-heap > memory footprint and increasing throughput. 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 seven additional commits since the last revision: - Merge - 6478546: Add break in write loop on ExceptionOccurred - Merge - 6478546: Clean up io_util.c - Merge - 6478546: Decrease malloc'ed buffer maximum size to 64 kB - 6478546: FileInputStream.read() throws OutOfMemoryError when there is plenty available - Changes: - all: https://git.openjdk.org/jdk/pull/8235/files - new: https://git.openjdk.org/jdk/pull/8235/files/10f14bb3..00521485 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=8235=04 - incr: https://webrevs.openjdk.java.net/?repo=jdk=8235=03-04 Stats: 118890 lines in 1877 files changed: 57455 ins; 51098 del; 10337 mod Patch: https://git.openjdk.org/jdk/pull/8235.diff Fetch: git fetch https://git.openjdk.org/jdk pull/8235/head:pull/8235 PR: https://git.openjdk.org/jdk/pull/8235
Re: RFR: 8273346: Expand library mappings to IEEE 754 operations [v4]
On Mon, 6 Jun 2022 22:24:03 GMT, Joe Darcy wrote: >> Generally add apiNote's to map from Java library methods to particular IEEE >> 754 operations. For now, I only added such notes to java.lang.Math and not >> java.lang.StrictMath. > > Joe Darcy has updated the pull request incrementally with one additional > commit since the last revision: > > Respond to review feedback. Approved assuming comments will be addressed prior to integration. src/java.base/share/classes/java/math/RoundingMode.java line 53: > 51: * two bracketing values is the result. For in-range real numbers, for > 52: * a given set of representable values, a rounding policy maps a > 53: * continuous segment of real number line to a single representable "the" real number line - Marked as reviewed by bpb (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/8876
Re: RFR: 8287541: Files.writeString fails to throw IOException for charset "windows-1252"
On Fri, 3 Jun 2022 16:48:46 GMT, Naoto Sato wrote: > The code path calls `String.getBytesNoRepl()`, but it blindly replaces > unmappable characters with replacements if the encoder is an `ArrayEncoder`. > Changed only to do so if `doReplace` is `true` in > `String.encodeWithEncoder()`. Marked as reviewed by bpb (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/9019
Re: RFR: 8284780: Need methods to create pre-sized HashSet and LinkedHashSet [v17]
On Fri, 27 May 2022 18:40:32 GMT, XenoAmess wrote: >> as title. > > XenoAmess has updated the pull request incrementally with one additional > commit since the last revision: > > do it as naotoj said `java.io` and `java.nio` look all right. - Marked as reviewed by bpb (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/8302
Re: RFR: 4511638: Double.toString(double) sometimes produces incorrect results [v15]
On Wed, 1 Jun 2022 10:37:23 GMT, Raffaello Giulietti wrote: >> Hello, >> >> here's a PR for a patch submitted on March 2020 >> [1](https://cr.openjdk.java.net/~bpb/4511638/webrev.04/) when Mercurial was >> a thing. >> >> The patch has been edited to adhere to OpenJDK code conventions about >> multi-line (block) comments. Nothing in the code proper has changed, except >> for the addition of redundant but clarifying parentheses in some expressions. >> >> >> Greetings >> Raffaello > > Raffaello Giulietti has updated the pull request incrementally with one > additional commit since the last revision: > > 4511638: Double.toString(double) sometimes produces incorrect results I think it's time for this excellent work to advance. - Marked as reviewed by bpb (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/3402
Re: RFR: 4511638: Double.toString(double) sometimes produces incorrect results [v14]
> On Jun 1, 2022, at 3:32 AM, Raffaello Giulietti wrote: > > On Tue, 31 May 2022 22:11:54 GMT, Brian Burkhalter wrote: > >>> Raffaello Giulietti has updated the pull request incrementally with one >>> additional commit since the last revision: >>> >>> 4511638: Double.toString(double) sometimes produces incorrect results >> >> src/java.base/share/classes/jdk/internal/math/FloatToDecimal.java line 97: >> >>> 95: private static final int MASK_28 = (1 << 28) - 1; >>> 96: >>> 97: private static final int NON_SPECIAL= 0; >> >> As these are shared with `DoubleToDecimal` would these constants be better >> moved to a common location, e.g., `MathUtils` whether converted to an `enum` >> or not? > > As long as the values are constant `ints`, moving them to `MathUtils` is less > robust. Changing any value would require remembering to recompile the > "clients". > The move would make sense if these were an enum. > > - > > PR: https://git.openjdk.java.net/jdk/pull/3402 Understood. All of that can wait until later.
Re: RFR: 4511638: Double.toString(double) sometimes produces incorrect results [v14]
On Jun 1, 2022, at 2:25 AM, Raffaello Giulietti
mailto:d...@openjdk.java.net>> wrote:
On Tue, 31 May 2022 21:57:44 GMT, Brian Burkhalter
mailto:b...@openjdk.org>> wrote:
Raffaello Giulietti has updated the pull request incrementally with one
additional commit since the last revision:
4511638: Double.toString(double) sometimes produces incorrect results
src/java.base/share/classes/jdk/internal/math/DoubleToDecimal.java line 118:
116: private int index;
117:
118: private DoubleToDecimal() {
Maybe add a comment like
/**
* Prevent instantiation.
*/
or
// Prevent instantiation of this class.
The constructor _is_ invoked, so the comment would be inappropriate.
-
PR: https://git.openjdk.java.net/jdk/pull/3402
Mea culpa.
Re: RFR: 4511638: Double.toString(double) sometimes produces incorrect results [v14]
On Tue, 31 May 2022 17:07:06 GMT, Raffaello Giulietti wrote: >> Hello, >> >> here's a PR for a patch submitted on March 2020 >> [1](https://cr.openjdk.java.net/~bpb/4511638/webrev.04/) when Mercurial was >> a thing. >> >> The patch has been edited to adhere to OpenJDK code conventions about >> multi-line (block) comments. Nothing in the code proper has changed, except >> for the addition of redundant but clarifying parentheses in some expressions. >> >> >> Greetings >> Raffaello > > Raffaello Giulietti has updated the pull request incrementally with one > additional commit since the last revision: > > 4511638: Double.toString(double) sometimes produces incorrect results src/java.base/share/classes/jdk/internal/math/DoubleToDecimal.java line 72: > 70: static final int E_MAX = 309; > 71: > 72: /* Threshold to detect tiny values, as in section 8.2.1 of [1] */ Comments like this one pointing to specific places in the Schubfach paper are very helpful in understanding the constants and the various steps in the algorithm. - PR: https://git.openjdk.java.net/jdk/pull/3402
Re: RFR: 4511638: Double.toString(double) sometimes produces incorrect results [v14]
On Tue, 31 May 2022 17:07:06 GMT, Raffaello Giulietti
wrote:
>> Hello,
>>
>> here's a PR for a patch submitted on March 2020
>> [1](https://cr.openjdk.java.net/~bpb/4511638/webrev.04/) when Mercurial was
>> a thing.
>>
>> The patch has been edited to adhere to OpenJDK code conventions about
>> multi-line (block) comments. Nothing in the code proper has changed, except
>> for the addition of redundant but clarifying parentheses in some expressions.
>>
>>
>> Greetings
>> Raffaello
>
> Raffaello Giulietti has updated the pull request incrementally with one
> additional commit since the last revision:
>
> 4511638: Double.toString(double) sometimes produces incorrect results
src/java.base/share/classes/jdk/internal/math/FloatToDecimal.java line 40:
> 38: final public class FloatToDecimal {
> 39: /*
> 40: * For full details about this code see the following references:
Same comment about ``.
src/java.base/share/classes/jdk/internal/math/FloatToDecimal.java line 97:
> 95: private static final int MASK_28 = (1 << 28) - 1;
> 96:
> 97: private static final int NON_SPECIAL= 0;
As these are shared with `DoubleToDecimal` would these constants be better
moved to a common location, e.g., `MathUtils` whether converted to an `enum` or
not?
src/java.base/share/classes/jdk/internal/math/FloatToDecimal.java line 118:
> 116: private int index;
> 117:
> 118: private FloatToDecimal() {
Same comment about preventing instantiation.
-
PR: https://git.openjdk.java.net/jdk/pull/3402
Re: RFR: 4511638: Double.toString(double) sometimes produces incorrect results [v14]
On Tue, 31 May 2022 17:07:06 GMT, Raffaello Giulietti
wrote:
>> Hello,
>>
>> here's a PR for a patch submitted on March 2020
>> [1](https://cr.openjdk.java.net/~bpb/4511638/webrev.04/) when Mercurial was
>> a thing.
>>
>> The patch has been edited to adhere to OpenJDK code conventions about
>> multi-line (block) comments. Nothing in the code proper has changed, except
>> for the addition of redundant but clarifying parentheses in some expressions.
>>
>>
>> Greetings
>> Raffaello
>
> Raffaello Giulietti has updated the pull request incrementally with one
> additional commit since the last revision:
>
> 4511638: Double.toString(double) sometimes produces incorrect results
src/java.base/share/classes/jdk/internal/math/DoubleToDecimal.java line 97:
> 95: private static final int MASK_28 = (1 << 28) - 1;
> 96:
> 97: private static final int NON_SPECIAL= 0;
Would these constants be better as an enum?
src/java.base/share/classes/jdk/internal/math/DoubleToDecimal.java line 118:
> 116: private int index;
> 117:
> 118: private DoubleToDecimal() {
Maybe add a comment like
/**
* Prevent instantiation.
*/
or
// Prevent instantiation of this class.
-
PR: https://git.openjdk.java.net/jdk/pull/3402
Re: RFR: 4511638: Double.toString(double) sometimes produces incorrect results [v14]
On Tue, 31 May 2022 17:07:06 GMT, Raffaello Giulietti wrote: >> Hello, >> >> here's a PR for a patch submitted on March 2020 >> [1](https://cr.openjdk.java.net/~bpb/4511638/webrev.04/) when Mercurial was >> a thing. >> >> The patch has been edited to adhere to OpenJDK code conventions about >> multi-line (block) comments. Nothing in the code proper has changed, except >> for the addition of redundant but clarifying parentheses in some expressions. >> >> >> Greetings >> Raffaello > > Raffaello Giulietti has updated the pull request incrementally with one > additional commit since the last revision: > > 4511638: Double.toString(double) sometimes produces incorrect results src/java.base/share/classes/jdk/internal/math/DoubleToDecimal.java line 47: > 45: * [2] IEEE Computer Society, "IEEE Standard for Floating-Point > Arithmetic" > 46: * > 47: * [3] Bouvier & Zimmermann, "Division-Free Binary-to-Decimal > Conversion" Similar comment concerning `` tag as in `MathUtils`. - PR: https://git.openjdk.java.net/jdk/pull/3402
Re: RFR: 4511638: Double.toString(double) sometimes produces incorrect results [v14]
On Tue, 31 May 2022 17:07:06 GMT, Raffaello Giulietti
wrote:
>> Hello,
>>
>> here's a PR for a patch submitted on March 2020
>> [1](https://cr.openjdk.java.net/~bpb/4511638/webrev.04/) when Mercurial was
>> a thing.
>>
>> The patch has been edited to adhere to OpenJDK code conventions about
>> multi-line (block) comments. Nothing in the code proper has changed, except
>> for the addition of redundant but clarifying parentheses in some expressions.
>>
>>
>> Greetings
>> Raffaello
>
> Raffaello Giulietti has updated the pull request incrementally with one
> additional commit since the last revision:
>
> 4511638: Double.toString(double) sometimes produces incorrect results
src/java.base/share/classes/jdk/internal/math/MathUtils.java line 38:
> 36: *
> 37: * Giulietti, "The Schubfach way to render doubles",
> 38: *
> https://drive.google.com/file/d/1gp5xv4CAa78SVgCeWfGqqI4FfYYYuNFb
Even though not public, should the reference use the `` tag and perhaps
be in a `@see` annotation?
@see https://drive.google.com/file/d/1gp5xv4CAa78SVgCeWfGqqI4FfYYYuNFb”>
The Schubfach way to render doubles
src/java.base/share/classes/jdk/internal/math/MathUtils.java line 193:
> 191: */
> 192: private static final long[] g = {
> 193: /* -324 */ 0x4F0C_EDC9_5A71_8DD4L, 0x5B01_E8B0_9AA0_D1B5L,
Not significant, but might this be clearer instead to comment the array
elements on the right?
0x4F0C_EDC9_5A71_8DD4L, 0x5B01_E8B0_9AA0_D1B5L, // -324
-
PR: https://git.openjdk.java.net/jdk/pull/3402
Integrated: 8287003: InputStreamReader::read() can return zero despite writing a char in the buffer
On Wed, 25 May 2022 23:08:38 GMT, Brian Burkhalter wrote: > If only a leftover `char` remains in the stream, do not add `-1` to the > return value in `lockedRead()`. This pull request has now been integrated. Changeset: 6520843f Author: Brian Burkhalter URL: https://git.openjdk.java.net/jdk/commit/6520843f86f638fe4d1e5b3358fab5799daca654 Stats: 33 lines in 2 files changed: 24 ins; 1 del; 8 mod 8287003: InputStreamReader::read() can return zero despite writing a char in the buffer Reviewed-by: jpai, rriggs - PR: https://git.openjdk.java.net/jdk/pull/8895
RFR: 8287003: InputStreamReader::read() can return zero despite writing a char in the buffer
If only a leftover `char` remains in the stream, do not add `-1` to the return value in `lockedRead()`. - Commit messages: - 8287003: InputStreamReader::read() can return zero despite writing a char in the buffer Changes: https://git.openjdk.java.net/jdk/pull/8895/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=8895=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8287003 Stats: 33 lines in 2 files changed: 24 ins; 1 del; 8 mod Patch: https://git.openjdk.java.net/jdk/pull/8895.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8895/head:pull/8895 PR: https://git.openjdk.java.net/jdk/pull/8895
Re: RFR: 6478546: FileInputStream.read() throws OutOfMemoryError when there is plenty available [v3]
On Thu, 12 May 2022 07:59:36 GMT, John Hendrikx wrote:
>> 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 four additional
>> commits since the last revision:
>>
>> - 6478546: Clean up io_util.c
>> - Merge
>> - 6478546: Decrease malloc'ed buffer maximum size to 64 kB
>> - 6478546: FileInputStream.read() throws OutOfMemoryError when there is
>> plenty available
>
> src/java.base/share/native/libjava/io_util.c line 79:
>
>> 77: jint off, jint len, jfieldID fid)
>> 78: {
>> 79: char stackBuf[STACK_BUF_SIZE];
>
> This was in the original code already, but doesn't this always allocate 8k on
> the stack, regardless of whether this buffer will be used (if len > 8k or len
> == 0)? Wouldn't it be better to allocate this only in the `else` case?
>
> Would apply to the write code as well.
This is intentional. We want to avoid having a malloc + free in every call and
so avoid it for small buffers.
> src/java.base/share/native/libjava/io_util.c line 81:
>
>> 79: char stackBuf[STACK_BUF_SIZE];
>> 80: char *buf = NULL;
>> 81: jint buf_size, read_size;;
>
> minor: double semi colon
FIxed in 10f14bb3faef2b55ab59a85016874d12815f3c87.
> src/java.base/share/native/libjava/io_util.c line 129:
>
>> 127: off += n;
>> 128: } else if (n == -1) {
>> 129: JNU_ThrowIOExceptionWithLastError(env, "Read error");
>
> The original code would have `nread` set to `-1` here, and thus exit with
> `-1`. The new code would exit with the last value for `nread` which could be
> anything.
The returned value of `nread` would in this case indicate the number of bytes
actually read so far, which is intentional.
> src/java.base/share/native/libjava/io_util.c line 201:
>
>> 199: write_size = len < buf_size ? len : buf_size;
>> 200: (*env)->GetByteArrayRegion(env, bytes, off, write_size,
>> (jbyte*)buf);
>> 201: if (!(*env)->ExceptionOccurred(env)) {
>
> Wouldn't this result in an infinite loop if `GetByteArrayRegion` triggered an
> exception and `len > 0` ? It would never enter the `if` and `len` is never
> changed then...
Good catch, you are correct. Fixed in 10f14bb3faef2b55ab59a85016874d12815f3c87.
-
PR: https://git.openjdk.java.net/jdk/pull/8235
Re: RFR: 6478546: FileInputStream.read() throws OutOfMemoryError when there is plenty available [v4]
> Modify native multi-byte read-write code used by the `java.io` classes to > limit the size of the allocated native buffer thereby decreasing off-heap > memory footprint and increasing throughput. 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 six additional commits since the last revision: - 6478546: Add break in write loop on ExceptionOccurred - Merge - 6478546: Clean up io_util.c - Merge - 6478546: Decrease malloc'ed buffer maximum size to 64 kB - 6478546: FileInputStream.read() throws OutOfMemoryError when there is plenty available - Changes: - all: https://git.openjdk.java.net/jdk/pull/8235/files - new: https://git.openjdk.java.net/jdk/pull/8235/files/5c3a3446..10f14bb3 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=8235=03 - incr: https://webrevs.openjdk.java.net/?repo=jdk=8235=02-03 Stats: 232900 lines in 3152 files changed: 173230 ins; 42904 del; 16766 mod Patch: https://git.openjdk.java.net/jdk/pull/8235.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8235/head:pull/8235 PR: https://git.openjdk.java.net/jdk/pull/8235
Integrated: 8213045: Add BigDecimal.TWO
On Mon, 16 May 2022 21:29:22 GMT, Brian Burkhalter wrote: > Add constant `java.math.BigDecimal.TWO`. This pull request has now been integrated. Changeset: 1d8e92ae Author: Brian Burkhalter URL: https://git.openjdk.java.net/jdk/commit/1d8e92ae0d2d0d6740e2171abef45545439e6414 Stats: 17 lines in 2 files changed: 12 ins; 2 del; 3 mod 8213045: Add BigDecimal.TWO Reviewed-by: darcy - PR: https://git.openjdk.java.net/jdk/pull/8735
Re: RFR: 8213045: Add BigDecimal.TWO [v2]
> Add constant `java.math.BigDecimal.TWO`. Brian Burkhalter has refreshed the contents of this pull request, and previous commits have been removed. The incremental views will show differences compared to the previous content of the PR. The pull request contains one new commit since the last revision: 8213045: Add BigDecimal.TWO - Changes: - all: https://git.openjdk.java.net/jdk/pull/8735/files - new: https://git.openjdk.java.net/jdk/pull/8735/files/df4cf704..434bc3de Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=8735=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk=8735=00-01 Stats: 0 lines in 0 files changed: 0 ins; 0 del; 0 mod Patch: https://git.openjdk.java.net/jdk/pull/8735.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8735/head:pull/8735 PR: https://git.openjdk.java.net/jdk/pull/8735
RFR: 8213045: Add commonly used symbolic math constants to the JDK
Add constant `java.math.BigDecimal.TWO`. - Commit messages: - 8213045: Add commonly used symbolic math constants to the JDK Changes: https://git.openjdk.java.net/jdk/pull/8735/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=8735=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8213045 Stats: 17 lines in 2 files changed: 12 ins; 2 del; 3 mod Patch: https://git.openjdk.java.net/jdk/pull/8735.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8735/head:pull/8735 PR: https://git.openjdk.java.net/jdk/pull/8735
Re: RFR: 8286810: Use public [Double|Float].PRECISION fields in jdk.internal.math.[Double|Float]Consts [v2]
On Mon, 16 May 2022 15:43:45 GMT, Roger Riggs wrote: >> Raffaello Giulietti has updated the pull request incrementally with one >> additional commit since the last revision: >> >> 8286810: Use public [Double|Float].PRECISION fields in >> jdk.internal.math.[Double|Float]Consts > > src/java.base/share/classes/jdk/internal/math/DoubleConsts.java line 28: > >> 26: package jdk.internal.math; >> 27: >> 28: import static java.lang.Double.*; > > I'd rather see explicit static imports, especially if there is any ambiguity > as to where they come from. > When using ordinary text search in a file, it can quickly identify a static > import as the source of the symbol. > As in this case where the same symbol has different values for Float vs > Double. YMMV I concur. - PR: https://git.openjdk.java.net/jdk/pull/8729
Re: RFR: 8286810: Use public [Double|Float].PRECISION fields in jdk.internal.math.[Double|Float]Consts [v2]
On Mon, 16 May 2022 15:51:43 GMT, Raffaello Giulietti wrote: >> Please review these simple changes in jdk.internal.math.[Double|Float]Consts > > Raffaello Giulietti has updated the pull request incrementally with one > additional commit since the last revision: > > 8286810: Use public [Double|Float].PRECISION fields in > jdk.internal.math.[Double|Float]Consts Marked as reviewed by bpb (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/8729
Integrated: 8286200: SequenceInputStream::read(b, off, 0) returns -1 at EOF
On Wed, 11 May 2022 20:47:52 GMT, Brian Burkhalter wrote: > Modify the specification of `SequenceInputStream.read(byte[],int,int)` to > indicate that `-1` is returned at the EOF of the last stream even if `len` is > zero. This pull request has now been integrated. Changeset: dbd37370 Author: Brian Burkhalter URL: https://git.openjdk.java.net/jdk/commit/dbd3737085d6e343a286f14556b9f49d71b4f959 Stats: 9 lines in 1 file changed: 3 ins; 0 del; 6 mod 8286200: SequenceInputStream::read(b, off, 0) returns -1 at EOF Reviewed-by: rriggs - PR: https://git.openjdk.java.net/jdk/pull/8664
Re: RFR: 8286200: SequenceInputStream::read(b, off, 0) returns -1 at EOF [v3]
On Thu, 12 May 2022 19:00:05 GMT, Roger Riggs wrote: > In the throws clauses, I think I would have put the additional conditional at > the end of the sentence since the existing throws text corresponds to the > exception. But the logic is correct as is. I put it at the beginning as that is the ordering but I see your point. - PR: https://git.openjdk.java.net/jdk/pull/8664
Re: RFR: JDK-8286760: Update citation of "Effective Java" second edition to third edition
On Fri, 13 May 2022 21:17:22 GMT, Joe Darcy wrote: > Update reference to the latest "Effective Java" version and switch to cite > tags. Looks fine. - Marked as reviewed by bpb (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/8707
Re: RFR: 8286559: Re-examine synchronization of mark and reset methods on InflaterInputStream [v2]
On Fri, 13 May 2022 07:14:30 GMT, Jaikiran Pai wrote: >> Can I please get a review of this change that addresses >> https://bugs.openjdk.java.net/browse/JDK-8286559? >> >> The commit here removes the `synchronized` on `mark` and `reset` methods of >> `InflaterInputStream`. The `mark` method is a no-op method and the `reset` >> method only always throws a `IOException`. So `synchronized` isn't adding >> any value here. >> >> Additionally, the commit does a minor change to the javadoc of these methods >> to use `@implNote` to describe what the implementation does. Please let me >> know if the `@implNote` is unnecessary, in which case, I'll revert that part. >> >> This change is similar to what was recently done for `FilterInputStream` >> https://github.com/openjdk/jdk/pull/8309 and `PushbackInputStream` >> https://github.com/openjdk/jdk/pull/8433 >> >> tier1, tier2 and tier3 tests were run and no related failures were noticed. > > Jaikiran Pai has updated the pull request incrementally with one additional > commit since the last revision: > > Incorporate review comment made on CSR by Joe - Change @implNote to > @implSpec Marked as reviewed by bpb (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/8649
Re: RFR: 8286200: SequenceInputStream::read(b, off, 0) returns -1 at EOF [v2]
On Thu, 12 May 2022 15:50:47 GMT, Raffaello Giulietti wrote: > I think the same change shall apply to the `@throws NullPointerException` > clause. Yeah looks like it. - PR: https://git.openjdk.java.net/jdk/pull/8664
Re: RFR: 8286200: SequenceInputStream::read(b, off, 0) returns -1 at EOF [v2]
On Thu, 12 May 2022 15:56:34 GMT, Brian Burkhalter wrote: > > I think the same change shall apply to the `@throws NullPointerException` > > clause. > > Yeah looks like it. Fixed by commit 111ea3e2f4203f05d17431953a5ffaa868176f98. - PR: https://git.openjdk.java.net/jdk/pull/8664
Re: RFR: 8286200: SequenceInputStream::read(b, off, 0) returns -1 at EOF [v3]
> Modify the specification of `SequenceInputStream.read(byte[],int,int)` to > indicate that `-1` is returned at the EOF of the last stream even if `len` is > zero. Brian Burkhalter has updated the pull request incrementally with one additional commit since the last revision: 8286200: Change @throws NPE clause of read(byte[],int,int) to better match specification verbiage - Changes: - all: https://git.openjdk.java.net/jdk/pull/8664/files - new: https://git.openjdk.java.net/jdk/pull/8664/files/7582dbff..111ea3e2 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=8664=02 - incr: https://webrevs.openjdk.java.net/?repo=jdk=8664=01-02 Stats: 2 lines in 1 file changed: 1 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/8664.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8664/head:pull/8664 PR: https://git.openjdk.java.net/jdk/pull/8664
Re: RFR: 8286200: SequenceInputStream::read(b, off, 0) returns -1 at EOF
On Thu, 12 May 2022 08:41:47 GMT, Raffaello Giulietti wrote: > Also, in the current implementation, when the end of the last contained > stream has been reached and `-1` is returned, none of the arguments is > checked, so a caller can pass `null` for `b` or out of bounds indices `off` > and `len`. This is at odd with the `@throws` clauses. Resolved by commit 7582dbff416e1fb164cfe924c128eb5ee73084f4. - PR: https://git.openjdk.java.net/jdk/pull/8664
Re: RFR: 8286200: SequenceInputStream::read(b, off, 0) returns -1 at EOF [v2]
> Modify the specification of `SequenceInputStream.read(byte[],int,int)` to > indicate that `-1` is returned at the EOF of the last stream even if `len` is > zero. Brian Burkhalter has updated the pull request incrementally with one additional commit since the last revision: 8286200: Change @throws IOOBE clause of read(byte[],int,int) to better match specification verbiage - Changes: - all: https://git.openjdk.java.net/jdk/pull/8664/files - new: https://git.openjdk.java.net/jdk/pull/8664/files/b5883b84..7582dbff Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=8664=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk=8664=00-01 Stats: 3 lines in 1 file changed: 1 ins; 0 del; 2 mod Patch: https://git.openjdk.java.net/jdk/pull/8664.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8664/head:pull/8664 PR: https://git.openjdk.java.net/jdk/pull/8664
Re: RFR: JDK-8286615: Small refactor to SerializedLambda
On Thu, 12 May 2022 00:10:28 GMT, Joe Darcy wrote: > Noticed by inspection during a CSR review, small refactoring to use a > message-cause exception constructor when one is available. > > Will update the copyright before a push. Marked as reviewed by bpb (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/8670
Re: RFR: JDK-8286604: Update InputStream and OutputStream to use @implSpec
On Wed, 11 May 2022 20:40:30 GMT, Joe Darcy wrote: > While doing a CSR review of another issue, I noticed some cases in > InputStream and OutputStream what would benefit from being upgraded to > implSpec and related javadoc tags. > > The "A subclass must provide an implementation of this method." statements on > several abstract methods don't add much value, but I chose to leave them in > for this request. > > Please also review the corresponding CSR: > https://bugs.openjdk.java.net/browse/JDK-8286605 Looks fine. - Marked as reviewed by bpb (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/8663
Re: RFR: 8286200: SequenceInputStream::read(b, off, 0) returns -1 at EOF
On Wed, 11 May 2022 20:47:52 GMT, Brian Burkhalter wrote: > Modify the specification of `SequenceInputStream.read(byte[],int,int)` to > indicate that `-1` is returned at the EOF of the last stream even if `len` is > zero. The `InputStream.read(byte[],int,int)` specification indicates If len is zero, then no bytes are read and 0 is returned; otherwise, there is an attempt to read at least one byte. If no byte is available because the stream is at end of file, the value -1 is returned; otherwise, at least one byte is read and stored into b. so that zero should be returned if `len` is zero regardless of anything else. `SequenceInputStream` does not follow this so its specification should be modified to document the existing, longstanding behavior. A CSR will be filed once there is consensus here. - PR: https://git.openjdk.java.net/jdk/pull/8664
RFR: 8286200: SequenceInputStream::read(b, off, 0) returns -1 at EOF
Modify the specification of `SequenceInputStream.read(byte[],int,int)` to indicate that `-1` is returned at the EOF of the last stream even if `len` is zero. - Commit messages: - 8286200: SequenceInputStream::read(b, off, 0) returns -1 at EOF Changes: https://git.openjdk.java.net/jdk/pull/8664/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=8664=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8286200 Stats: 5 lines in 1 file changed: 1 ins; 0 del; 4 mod Patch: https://git.openjdk.java.net/jdk/pull/8664.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8664/head:pull/8664 PR: https://git.openjdk.java.net/jdk/pull/8664
Re: RFR: 8286378: Address possibly lossy conversions in java.base [v3]
On Wed, 11 May 2022 16:30:41 GMT, Roger Riggs wrote: >> PR#8599 8244681: proposes to add compiler warnings for possible lossy >> conversions >> From the CSR: >> >> "If the type of the right-hand operand of a compound assignment is not >> assignment compatible with the type of the variable, a cast is implied and >> possible lossy conversion may silently occur. While similar situations are >> resolved as compilation errors for primitive assignments, there are no >> similar rules defined for compound assignments." >> >> This PR anticipates the warnings and adds explicit casts to replace the >> implicit casts. >> In most cases, the cast matches the type of the right-hand side to type of >> the compound assignment. >> Since these casts are truncating the value, there might be a different >> refactoring that avoids the cast >> but a direct replacement was chosen here. >> >> (Advise and suggestions will inform similar updates to other OpenJDK >> modules). > > Roger Riggs has updated the pull request incrementally with one additional > commit since the last revision: > > Updated copyrights > Fixed cast style to add a space after cast, (where consistent with file > style) > Improved code per review comments in PollSelectors. Marked as reviewed by bpb (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/8642
Re: Can JDK-8190546 be re-opened or "how do I delete a file ending with a space on Windows"
Redirect discussion to nio-dev. Brian > On May 11, 2022, at 7:29 AM, Maxim Kartashev > wrote: > > Win32 documentation [1] kind of discourages the use of space at the very > end of a file name. Based on that, JDK-8190546 (File.toPath() reject > directory names with trailing space) had been closed a long time ago. I > would like to poll the public on the matter of re-opening the bug. > > There isn't anything new that I can throw in support of allowing JDK to > work with such file names. But the old points can perhaps be re-visited. > AFAIU, the only reason to explicitly forbid that (see > WindowsPathParser.normalize()) was that the parsing is based on Windows > documentation like [1]. That documentation says the following: > "Do not end a file or directory name with a space or a period. Although the > underlying file system may support such names, the Windows shell and user > interface does not" > Indeed, it's hard or even impossible to create such a file using "normal" > windows GUI, but you can use that GUI to delete such a file. In Java, you > can't do either. Working in a cygwin terminal, you can fairly easily create > a file name ending with a space. And when you turn to your Java-based IDE > to delete it, you get an error from the very basic level of NIO that you > can't overrule, which seems to be quite unfortunate. > > If there's any interest in resolving this - or at least not enough > opposition to let it be resolved - I am willing to make the necessary > changes and open a PR. > > References > [1] > https://docs.microsoft.com/en-us/windows/win32/fileio/naming-a-file#naming-conventions > [2] https://bugs.openjdk.java.net/browse/JDK-8190546
Re: RFR: 8286287: Reading file as UTF-16 causes Error which "shouldn't happen"
On Tue, 10 May 2022 20:22:39 GMT, Naoto Sato wrote: > `String.decodeWithDecoder()` method requires the `CharsetDecoder` parameter > replaces on malformed/unmappable characters with replacements. However, there > was a code path that lacked to set the `CodingErrorAction.REPLACE` on the > decoder. Unrelated, one typo in a test was also fixed. Marked as reviewed by bpb (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/8640
Re: RFR: 8286378: Address possibly lossy conversions in java.base
On Tue, 10 May 2022 21:32:10 GMT, Roger Riggs wrote: > PR#8599 8244681: proposes to add compiler warnings for possible lossy > conversions > From the CSR: > > "If the type of the right-hand operand of a compound assignment is not > assignment compatible with the type of the variable, a cast is implied and > possible lossy conversion may silently occur. While similar situations are > resolved as compilation errors for primitive assignments, there are no > similar rules defined for compound assignments." > > This PR anticipates the warnings and adds explicit casts to replace the > implicit casts. > In most cases, the cast matches the type of the right-hand side to type of > the compound assignment. > Since these casts are truncating the value, there might be a different > refactoring that avoids the cast > but a direct replacement was chosen here. > > (Advise and suggestions will inform similar updates to other OpenJDK modules). IO, math, and NIO changes look fine. - Marked as reviewed by bpb (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/8642
Integrated: 8286363: BigInteger.parallelMultiply missing @since 19
On Mon, 9 May 2022 15:26:20 GMT, Brian Burkhalter wrote: > Add missing `@since 19` tag. This pull request has now been integrated. Changeset: 04bba07d Author: Brian Burkhalter URL: https://git.openjdk.java.net/jdk/commit/04bba07d6588cb96e371f3acdb49d735c9e6536d Stats: 1 line in 1 file changed: 1 ins; 0 del; 0 mod 8286363: BigInteger.parallelMultiply missing @since 19 Reviewed-by: alanb, darcy - PR: https://git.openjdk.java.net/jdk/pull/8598
RFR: 8286363: BigInteger.parallelMultiply missing @since 19
Add missing `@since 19` tag. - Commit messages: - 8286363: BigInteger.parallelMultiply missing @since 19 Changes: https://git.openjdk.java.net/jdk/pull/8598/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=8598=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8286363 Stats: 1 line in 1 file changed: 1 ins; 0 del; 0 mod Patch: https://git.openjdk.java.net/jdk/pull/8598.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8598/head:pull/8598 PR: https://git.openjdk.java.net/jdk/pull/8598
Re: RFR: 8286298: Remove unused methods in sun.invoke.util.VerifyType
On Fri, 6 May 2022 11:32:25 GMT, Claes Redestad wrote: > A few untested and unused methods in `VerifyType` which can be removed. > (Possibly used by native JSR 292 implementations in JDK 7). Looks fine. - Marked as reviewed by bpb (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/8570
Re: RFR: 8274517: java/util/DoubleStreamSums/CompensatedSums.java fails with expected [true] but found [false]
On Tue, 19 Apr 2022 08:40:51 GMT, Raffaello Giulietti wrote: > Please review these small changes to address intermittent failures, as of > JDK-8274517. > > - Usage of jdk.test.lib.RandomFactory for reproducible random generation. > - Slightly less restrictive assertion about badParallelStreamError on L94 > (former L88). > - Verbatim copy of computeFinalSum() from j.u.s.Collectors 18. > > While these changes do not necessarily guarantee absence of intermittent > failures, the usage of jdk.test.lib.RandomFactory should at least help to pin > down specific double sequences that do not pass the test. > > There is still an inherent variability due to the use of parallel streams, > though. As double addition is not perfectly associative, even a fully known > sequence of doubles may lead to slightly different results with > parallelization. Marked as reviewed by bpb (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/8290
Re: RFR: 6478546: FileInputStream.read() throws OutOfMemoryError when there is plenty available [v2]
On Thu, 28 Apr 2022 20:02:36 GMT, Brian Burkhalter wrote: >> Modify native multi-byte read-write code used by the `java.io` classes to >> limit the size of the allocated native buffer thereby decreasing off-heap >> memory footprint and increasing throughput. > > Brian Burkhalter has updated the pull request incrementally with one > additional commit since the last revision: > > 6478546: Decrease malloc'ed buffer maximum size to 64 kB Further performance testing was conducted for the case where the native read and write functions used a fixed, stack-allocated buffer of size 8192. The loops were moved up into the Java code of `FileInputStream`, `FileOutputStream` and `RandomAccessFile`. Note that there was code duplication because RAF needs both read and write methods as well. The performance of writing with this approach was approximately half what it had been, so for writing the approach was abandoned. Here are some updated performance measurements: https://user-images.githubusercontent.com/71468245/167041493-6d4c421c-c2ec-4a8a-8b32-09b2a902a77c.png;> https://user-images.githubusercontent.com/71468245/167041541-94e5806c-de86-4e62-a117-4cfafac82e87.png;> The performance measurements shown are for the following cases: 1. Master: unmodified code as it exists in the mainline 2. Java: fixed-size stack buffer in native read, read loops in Java, write as in the mainline but with malloc buffer size limit 3. Native: read loop in native read with malloc buffer size limit, write as in the mainline but with malloc buffer size limit The horizontal axis represents a variety of lengths from 8192 to 1GB; the vertical axis is throughput (ops/s) on a log 10 scale. The native lines in the charts are for the code proposed to be integrated. As can be seen, the performance of reading is quite similar up to larger lengths. The mainline version presumably starts to suffer the effect of large allocations. The native read loop performs the best throughout, being for lengths 10 MB and above from 50% to 3X faster than the mainline version. The native read loop is about 40% faster than the Java read loop for these larger lengths. Due to the log scale of the charts, the reading performance detail cannot be seen exactly and so is given here for the larger lengths: Throughput of read(byte[]) (ops/s) Length Master JavaNative 104857611341.39 6124.48211371.091 10485760 356.893 376.326 557.906 251503002 10.036 14.2719.869 5242880005.0056.8579.552 101.6753.5274.997 The performance of writing is about the same for the Java and Native versions, as it should be since the implementations are the same. Any difference is likely due to measurement noise. The mainline version again suffers for larger lengths. As the native write loop was already present in the mainline code, the principal complexity proposed to be added is the native read loop. Given the improved throughput and vastly reduced native memory allocation this seems to be justified. - PR: https://git.openjdk.java.net/jdk/pull/8235
Re: RFR: 6478546: FileInputStream.read() throws OutOfMemoryError when there is plenty available [v3]
> Modify native multi-byte read-write code used by the `java.io` classes to > limit the size of the allocated native buffer thereby decreasing off-heap > memory footprint and increasing throughput. 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 four additional commits since the last revision: - 6478546: Clean up io_util.c - Merge - 6478546: Decrease malloc'ed buffer maximum size to 64 kB - 6478546: FileInputStream.read() throws OutOfMemoryError when there is plenty available - Changes: - all: https://git.openjdk.java.net/jdk/pull/8235/files - new: https://git.openjdk.java.net/jdk/pull/8235/files/40d46f8f..5c3a3446 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=8235=02 - incr: https://webrevs.openjdk.java.net/?repo=jdk=8235=01-02 Stats: 36827 lines in 1404 files changed: 26452 ins; 4333 del; 6042 mod Patch: https://git.openjdk.java.net/jdk/pull/8235.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8235/head:pull/8235 PR: https://git.openjdk.java.net/jdk/pull/8235
Re: RFR: JDK-8285977: Add links to IEEE 754 specification
On Mon, 2 May 2022 22:55:43 GMT, Joe Darcy wrote: > Please review the addition of @-see links from classes that mention the IEEE > 754 floating-point standard to an IEEE page about the standard. The URL in > the initial version of the PR is the top search result on the IEEE home page > for "754 standard". > > Another candidate page to use is > > https://ieeexplore.ieee.org/servlet/opac?punumber=8766227 > > which is (apparently) a stable citation page for the standard. > > These links may be upgraded to the the forthcoming @-spec facility in the > future. ( JDK-6251738). Marked as reviewed by bpb (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/8511
Integrated: 8285745: Re-examine PushbackInputStream mark/reset
On Wed, 27 Apr 2022 20:10:03 GMT, Brian Burkhalter wrote: > Please review this request to remove the `synchronized` keyword from the > `mark(int)` and `reset()` methods of `java.io.PushbackInputStream`. This pull request has now been integrated. Changeset: 9d8c3bf9 Author: Brian Burkhalter URL: https://git.openjdk.java.net/jdk/commit/9d8c3bf9f8bc2083c44b7203e81c007d685b9b61 Stats: 3 lines in 1 file changed: 0 ins; 0 del; 3 mod 8285745: Re-examine PushbackInputStream mark/reset Reviewed-by: jpai, alanb - PR: https://git.openjdk.java.net/jdk/pull/8433
Re: RFR: 8285956: (fs) Excessive default poll interval in PollingWatchService
On Sat, 30 Apr 2022 00:14:29 GMT, Tyler Steele wrote: > PollingWatchService.java contains the WatchService and WatchKey > implementation for AIX and BSD. When a Path is > [register](https://docs.oracle.com/en/java/javase/17/docs/api/java.base/java/nio/file/Path.html#register(java.nio.file.WatchService,java.nio.file.WatchEvent.Kind...))ed > this implementation creates a polling thread to monitor for file system > changes. Currently, this thread waits 10 seconds before it's first poll, and > then waits 10 seconds between subsequent polls. This interval leads to > sluggish performance. > > This PR makes the following changes: > - Sets the initial interval to 1 second regardless of the period. > - Change the default period to 1 second. > > All tests in `test/jdk/java/nio/file/WatchService` passing. Do you have any performance measurements to share? Note that the sensitivity can be set as shown in the [SensitivityModifier](https://github.com/openjdk/jdk/blob/41de506ed6c9dc0331c2b6ae99c11623df05f34a/test/jdk/java/nio/file/WatchService/SensitivityModifier.java) test. - PR: https://git.openjdk.java.net/jdk/pull/8479
Re: RFR: 6478546: FileInputStream.read() throws OutOfMemoryError when there is plenty available [v2]
On Thu, 28 Apr 2022 20:13:48 GMT, Uwe Schindler wrote: > By the way: FileOutputStream has exactly the same problem with > `write(byte[])`. I see no test for it, but I assume this is now also fixed. > That's a longstanding issue in Lucene (this is why we use a > ChunkedOutputStream when writing files. Yes, `write(byte[])` is also fixed: [io_util.c#L164](https://github.com/bplb/jdk/blob/40d46f8f4463dbd7dbe10651910826e90ca4dbdd/src/java.base/share/native/libjava/io_util.c#L164). - PR: https://git.openjdk.java.net/jdk/pull/8235
Re: RFR: 6478546: FileInputStream.read() throws OutOfMemoryError when there is plenty available [v2]
> Modify native multi-byte read-write code used by the `java.io` classes to > limit the size of the allocated native buffer thereby decreasing off-heap > memory footprint and increasing throughput. Brian Burkhalter has updated the pull request incrementally with one additional commit since the last revision: 6478546: Decrease malloc'ed buffer maximum size to 64 kB - Changes: - all: https://git.openjdk.java.net/jdk/pull/8235/files - new: https://git.openjdk.java.net/jdk/pull/8235/files/8bb1e14f..40d46f8f Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=8235=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk=8235=00-01 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/8235.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8235/head:pull/8235 PR: https://git.openjdk.java.net/jdk/pull/8235
Re: RFR: 8285523: Improve test java/io/FileOutputStream/OpenNUL.java
On Mon, 25 Apr 2022 04:35:13 GMT, Sergey Bylokhov wrote: > The new test added as part of the > [JDK-8285445](https://bugs.openjdk.java.net/browse/JDK-8285445) cannot > trigger that bug and pass w/ and w/o fix. > > An updated test validates the "default" case when the `jdk.io.File.enableADS` > property is not set, in this case the ADS should be > [accepted](https://github.com/openjdk/jdk/blob/9d9f4e502f6ddc3116ed9b80f7168a1edfce839e/src/java.base/windows/classes/java/io/WinNTFileSystem.java#L59). Marked as reviewed by bpb (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/8379
Re: RFR: 8284161: Implementation of Virtual Threads (Preview) [v8]
On Wed, 27 Apr 2022 14:24:20 GMT, Alan Bateman wrote: >> This is the implementation of JEP 425: Virtual Threads (Preview); TBD which >> JDK version to target. >> >> We will refresh this PR periodically to pick up changes and fixes from the >> loom repo. >> >> Most of the new mechanisms in the HotSpot VM are disabled by default and >> require running with `--enable-preview` to enable. >> >> The patch has support for x64 and aarch64 on the usual operating systems >> (Linux, macOS, and Windows). There are stubs (calling Unimplemented) for >> zero and some of the other ports. Additional ports can be contributed via >> PRs against the fibers branch in the loom repo. >> >> There are changes in many areas. To reduce notifications/mails, the labels >> have been trimmed down for now to hotspot, serviceability and core-libs. >> We'll add the complete set of labels when the PR is further along. >> >> The changes include a refresh of java.util.concurrent and ForkJoinPool from >> Doug Lea's CVS. These changes will probably be proposed and integrated in >> advance of this PR. >> >> The changes include some non-exposed and low-level infrastructure to support >> the (in draft) JEPs for Structured Concurrency and Scope Locals. This is to >> make life a bit easier and avoid having to separate VM changes and juggle >> branches at this time. > > Alan Bateman has updated the pull request incrementally with one additional > commit since the last revision: > > Refresh 7965cc6b168e567ac2596f2fbc3b00a7d99b7e1e The changes to the `java.io` package and related files in `libjava`, and the changes to the non-networking parts of the `java.nio.channels`, `sun.nio.ch`, and `sun.nio.fs` packages and related files in `libnio` all look fine to me. - Marked as reviewed by bpb (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/8166
Re: RFR: 8285745: Re-examine PushbackInputStream mark/reset
On Wed, 27 Apr 2022 20:10:03 GMT, Brian Burkhalter wrote: > Please review this request to remove the `synchronized` keyword from the > `mark(int)` and `reset()` methods of `java.io.PushbackInputStream`. Please see also https://github.com/openjdk/jdk/pull/8309. - PR: https://git.openjdk.java.net/jdk/pull/8433
RFR: 8285745: Re-examine PushbackInputStream mark/reset
Please review this request to remove the `synchronized` keyword from the `mark(int)` and `reset()` methods of `java.io.PushbackInputStream`. - Commit messages: - 8285745: Re-examine PushbackInputStream mark/reset Changes: https://git.openjdk.java.net/jdk/pull/8433/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=8433=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8285745 Stats: 3 lines in 1 file changed: 0 ins; 0 del; 3 mod Patch: https://git.openjdk.java.net/jdk/pull/8433.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8433/head:pull/8433 PR: https://git.openjdk.java.net/jdk/pull/8433
Re: RFR: 8285658: Fix two typos in the spec of j.u.random.RandomGenerator [v3]
On Wed, 27 Apr 2022 07:34:29 GMT, Raffaello Giulietti wrote: >> The spec of the interface `java.util.random.RandomGenerator` is slightly >> incorrect when it discusses `float` and `double` random values. > > Raffaello Giulietti has updated the pull request incrementally with one > additional commit since the last revision: > > 8285658: Fix two typos in the spec of j.u.random.RandomGenerator Marked as reviewed by bpb (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/8404
Re: RFR: 8285658: Fix two typos in the spec of j.u.random.RandomGenerator [v2]
On Tue, 26 Apr 2022 16:55:44 GMT, Raffaello Giulietti wrote: >> The spec of the interface `java.util.random.RandomGenerator` is slightly >> incorrect when it discusses `float` and `double` random values. > > Raffaello Giulietti has updated the pull request incrementally with one > additional commit since the last revision: > > 8285658: Fix two typos in the spec of j.u.random.RandomGenerator Marked as reviewed by bpb (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/8404
Integrated: 8284930: Re-examine FilterInputStream mark/reset
On Tue, 19 Apr 2022 23:26:44 GMT, Brian Burkhalter wrote: > Remove the `synchronized` keyword from the `mark(int)` and `reset()` methods > of `java.io.FilterInputStream`. This pull request has now been integrated. Changeset: a3b78814 Author: Brian Burkhalter URL: https://git.openjdk.java.net/jdk/commit/a3b788144ecc37262a3560e9c234bc8fb41ca3df Stats: 6 lines in 2 files changed: 0 ins; 0 del; 6 mod 8284930: Re-examine FilterInputStream mark/reset Reviewed-by: alanb, jpai, dfuchs, lancea - PR: https://git.openjdk.java.net/jdk/pull/8309
Integrated: 8285445: cannot open file "NUL:"
On Sat, 23 Apr 2022 01:11:56 GMT, Brian Burkhalter wrote: > Change the default value of the `jdk.io.File.enableADS` property to `true`. This pull request has now been integrated. Changeset: 03cbb48e Author: Brian Burkhalter URL: https://git.openjdk.java.net/jdk/commit/03cbb48e6a1d806f204a39bbdbb4bc9be9e57a41 Stats: 57 lines in 2 files changed: 52 ins; 1 del; 4 mod 8285445: cannot open file "NUL:" Reviewed-by: mikael - PR: https://git.openjdk.java.net/jdk/pull/8373
Re: RFR: 8285445: cannot open file "NUL:"
On Sat, 23 Apr 2022 01:11:56 GMT, Brian Burkhalter wrote: > Change the default value of the `jdk.io.File.enableADS` property to `true`. This topic will be examined further under JDK-8285511. - PR: https://git.openjdk.java.net/jdk/pull/8373
RFR: 8285445: cannot open file "NUL:"
Change the default value of the `jdk.io.File.enableADS` property to `true`. - Commit messages: - 8285445: cannot open file "NUL:" Changes: https://git.openjdk.java.net/jdk/pull/8373/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=8373=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8285445 Stats: 57 lines in 2 files changed: 52 ins; 1 del; 4 mod Patch: https://git.openjdk.java.net/jdk/pull/8373.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8373/head:pull/8373 PR: https://git.openjdk.java.net/jdk/pull/8373
Re: RFR: 8284930: Re-examine FilterInputStream mark/reset [v2]
On Thu, 21 Apr 2022 10:13:11 GMT, Lance Andersen wrote: > Looks fine Brian. Any thoughts as to whether a release note is warranted? Thanks, @LanceAndersen. The issue is labelled as needing a release note so you are spot on. - PR: https://git.openjdk.java.net/jdk/pull/8309
Re: RFR: 8284930: Re-examine FilterInputStream mark/reset [v2]
On Thu, 21 Apr 2022 00:00:47 GMT, Stuart Marks wrote: > > I think it's a vanishingly small possibility that anything is relying on the > synchronization in these methods. Synchronization here would provide proper > memory visibility effects across threads. To use input streams from multiple > threads without interleaving operations, one would have to provide some other > means of coordination among those threads, which itself would likely ensure > proper memory visibility. I'm hard pressed to see how threads could > coordinate solely via use of the `mark` and `reset` methods. Therefore, I > think it's safe to remove synchronization from them. > > This reasoning also applies to `InputStream`. Perhaps synchronization can be > removed from there too. > > I agree that a CSR is probably warranted to capture the behavior change and > analysis. Commit f210cbf5f6bf58326a379b4b3f55fddf49d30f5c removed the synchronization from `InputStream`'s `mark()` and `reset()`; a CSR is on file. - PR: https://git.openjdk.java.net/jdk/pull/8309
Re: RFR: JDK-8280594: Refactor annotation invocation handler handling to use Objects.toIdentityString
On Tue, 19 Apr 2022 23:34:01 GMT, Joe Darcy wrote: > Simple refactoring to use new-in19 library code. Marked as reviewed by bpb (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/8310
Re: RFR: 8284930: Re-examine FilterInputStream mark/reset [v2]
> Remove the `synchronized` keyword from the `mark(int)` and `reset()` methods > of `java.io.FilterInputStream`. Brian Burkhalter has updated the pull request incrementally with one additional commit since the last revision: 8284930: Also remove synchronized keyword from mark() and reset() of InputStream - Changes: - all: https://git.openjdk.java.net/jdk/pull/8309/files - new: https://git.openjdk.java.net/jdk/pull/8309/files/85546c9e..f210cbf5 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=8309=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk=8309=00-01 Stats: 3 lines in 1 file changed: 0 ins; 0 del; 3 mod Patch: https://git.openjdk.java.net/jdk/pull/8309.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8309/head:pull/8309 PR: https://git.openjdk.java.net/jdk/pull/8309
RFR: 8284930: Re-examine FilterInputStream mark/reset
Remove the `synchronized` keyword from the `mark(int)` and `reset()` methods of `java.io.FilterInputStream`. - Commit messages: - 8284930: Re-examine FilterInputStream mark/reset Changes: https://git.openjdk.java.net/jdk/pull/8309/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=8309=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8284930 Stats: 3 lines in 1 file changed: 0 ins; 0 del; 3 mod Patch: https://git.openjdk.java.net/jdk/pull/8309.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8309/head:pull/8309 PR: https://git.openjdk.java.net/jdk/pull/8309
Re: RFR: 6478546: FileInputStream.read() throws OutOfMemoryError when there is plenty available
On Thu, 14 Apr 2022 05:57:54 GMT, Daniel Jeliński wrote: > The benchmark results are quite unexpected. Would we benefit from reducing > the buffer size even further? I tested with smaller and smaller buffer sizes until the performance started to be affected which was about 64 KB. I have not changed this value in the patch just yet. - PR: https://git.openjdk.java.net/jdk/pull/8235
Re: RFR: 6478546: FileInputStream.read() throws OutOfMemoryError when there is plenty available
On Thu, 14 Apr 2022 06:23:24 GMT, Alan Bateman wrote:
>> Modify native multi-byte read-write code used by the `java.io` classes to
>> limit the size of the allocated native buffer thereby decreasing off-heap
>> memory footprint and increasing throughput.
>
> src/java.base/share/native/libjava/io_util.c line 133:
>
>> 131: if (nread == 0)
>> 132: nread = -1;
>> 133: break;
>
> Can you prototype doing the loop in Java rather than in native code so that
> there is less native code to maintain?
I prototyped doing the read loop in Java and the performance seemed to be about
the same. The problem is that the loop needs to exit when the native `read()`
function performs a short read, i.e., fewer bytes are read than were requested.
Without this, at least one regression test fails. The reason is not completely
clear.
To detect such a short read in the Java layer would require some way for the
native layer to notify the Java layer. One potential such method is
boolean readBytes(byte[] b, int off, int len, int[] nread) {}
where the return value indicates whether all or only some of the `len` bytes
were read. If not all were read, then `nread[0]` would contain the number
actually read or `-1`.
Another possibility is
int readBytes(byte[] b, int off, int len, int maxBufferSize) {}
which is identical to the current `readBytes()` except that the maximum
transfer buffer size is specified as a method parameter instead of being
defined by a symbolic constant in the native layer. In this case a short read
would be detected if `len >= maxBufferSize` and the returned value is less than
`maxBufferSize`.
As for the read loop being in native code, note that the write loop is also
already in native code, so if the read loop were moved to Java, then probably
the write loop should be as well.
One advantage of the loops being in native code is that there is only one
`malloc()` per Java `read(byte[],int,int)` or `write(byte[],int,int)`
invocation.
-
PR: https://git.openjdk.java.net/jdk/pull/8235
Re: RFR: 8284922: Fix some doc-comment issues on methods with package access in JDK API
On Fri, 15 Apr 2022 19:34:33 GMT, Pavel Rappo wrote: > People rarely include JDK elements with package access in a javadoc run. That > is why bugs in those elements' doc comments tend to remain unnoticed. > > There are many more bugs in the doc comments of the JDK elements with the > package access than are addressed by this PR; I only included the simplest > ones. Marked as reviewed by bpb (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/8267
Re: RFR: 6478546: FileInputStream.read() throws OutOfMemoryError when there is plenty available
On Thu, 14 Apr 2022 01:40:50 GMT, Brian Burkhalter wrote: > Modify native multi-byte read-write code used by the `java.io` classes to > limit the size of the allocated native buffer thereby decreasing off-heap > memory footprint and increasing throughput. Currently for `java.io.FileInputStream.read(byte[],int,int)` and `java.io.FileOutputStream.write(byte[],int,int)`, for example, if the number of bytes respectively to be read or written exceeds 8192, an array of the total length of the read or write is allocated. For large reads or writes this could be significant. It is proposed to limit the maximum allocation size to 1 MB and perform the read or write by looping with this buffer. For reading or writing less than 1 MB, there is no effective change in the implementation. This change both saves off-heap memory and increases throughput. An allocation of 1 MB is only 0.42% the size of the buffer in the JBS issue, 501 x 501 x 501 x 2 (= 251,503,002), so for this case the memory reduction is drastic. Reading throughput is almost doubled and writing throughput improved by about 50%. As measured on macOS, the throughput of the methods mentioned above before the change was: Benchmark Mode Cnt Score Error Units ReadWrite.read thrpt5 10.108 ± 0.264 ops/s ReadWrite.write thrpt5 7.188 ± 0.431 ops/s and that after is: Benchmark Mode Cnt Score Error Units ReadWrite.read thrpt5 20.112 ± 0.262 ops/s ReadWrite.write thrpt5 10.644 ± 4.568 ops/s - PR: https://git.openjdk.java.net/jdk/pull/8235
RFR: 6478546: FileInputStream.read() throws OutOfMemoryError when there is plenty available
Modify native multi-byte read-write code used by the `java.io` classes to limit the size of the allocated native buffer thereby decreasing off-heap memory footprint and increasing throughput. - Commit messages: - 6478546: FileInputStream.read() throws OutOfMemoryError when there is plenty available Changes: https://git.openjdk.java.net/jdk/pull/8235/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=8235=00 Issue: https://bugs.openjdk.java.net/browse/JDK-6478546 Stats: 93 lines in 2 files changed: 52 ins; 8 del; 33 mod Patch: https://git.openjdk.java.net/jdk/pull/8235.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8235/head:pull/8235 PR: https://git.openjdk.java.net/jdk/pull/8235
Re: RFR: 8284853: Fix varios 'expected' typo
On Wed, 13 Apr 2022 20:36:48 GMT, Andrey Turbanov wrote: > Found various typos of expected: `exepected`, `exept`, `epectedly`, > `expeced`, `Unexpeted`, etc. Expect the Unexpeted. - Marked as reviewed by bpb (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/8231
Re: RFR: 8283083: java.util.random L128X256MixRandom constructor fails to use byte[] seed
On Tue, 12 Apr 2022 15:37:19 GMT, Raffaello Giulietti wrote: > Please review this tiny fix. > > A test similar to the code proposed by the bug reporter has been added for > the LXM group. It does not pass before the fix and passes after. Marked as reviewed by bpb (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/8207
Re: RFR: 8284702: Add @since for java.time.LocalDate.EPOCH
On Tue, 12 Apr 2022 03:21:00 GMT, Glavo wrote: > `java.time.LocalDate.EPOCH` was introduced in Java 9, but there is no > corresponding `@since` in the javadoc. The absence of `@since` makes it > impossible for IDEs to check for misuse of it, it may be misused by users > targeting Java 8 and crash at runtime. Marked as reviewed by bpb (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/8192
Re: RFR: 8284638: store skip buffers in InputStream Object
On Sat, 9 Oct 2021 19:02:17 GMT, XenoAmess wrote: >>> in extream situation, when doing this.skipBuffer = skipBuffer in Thread B, >>> it might make this.skipBuffer to a byte[6] in thread A, and thus cause a >>> IndexOutofBoundException in Thread A. >> >> No, it won't. The later `skipBuffer` references are made to the local >> variable; so even though the `new byte[6]` may replace the cached `new >> byte[10]` skip buffer in instance field, in thread A, it is still using the >> old `new byte[10]` which is stored in the local variable/stack, and >> everything just proceeds fine (only shortcoming is that the 10-length skip >> buffer is wasted and recycled) > >> > in extream situation, when doing this.skipBuffer = skipBuffer in Thread B, >> > it might make this.skipBuffer to a byte[6] in thread A, and thus cause a >> > IndexOutofBoundException in Thread A. >> >> No, it won't. The later `skipBuffer` references are made to the local >> variable; so even though the `new byte[6]` may replace the cached `new >> byte[10]` skip buffer in instance field, in thread A, it is still using the >> old `new byte[10]` which is stored in the local variable/stack, and >> everything just proceeds fine (only shortcoming is that the 10-length skip >> buffer is wasted and recycled) > > you are correct. I forgot you use same name local variable hiding the field > variable, so the later skipBuffer passed to read() is local variable. > > (sigh) I feel like super stupid when facing multithread programming. Isn't it the case that the length of the global `skipBuffer` is non-decreasing? Thus skipping 6 bytes after skipping 10 will not result in a new buffer allocation. Even so, it does appear that prior buffer allocations could be "lost" due to concurrent use of `skip` resulting in a "minor" memory leak in the heap. - PR: https://git.openjdk.java.net/jdk/pull/5872
Re: RFR: JDK-8283084 RandomGenerator nextDouble(double, double) is documented incorrectly [v2]
On Mon, 11 Apr 2022 12:32:26 GMT, Jim Laskey wrote:
>> `default float nextFloat(float origin, float bound); ` and `default double
>> nextDouble(double origin, double bound); ` are documented incorrectly. The
>> default method checks (origin < bound) and (bound - origin) < +infinity.
>>
>> The exception conditions are incorrect:
>> "if {@code origin} is not finite,
>> or {@code bound} is not finite, or {@code origin}
>> is greater than or equal to {@code bound}"
>>
>> This is not true. Calling with -Double.MAX_VALUE and Double.MAX_VALUE
>> satisfies the documented requirements but actually throws an exception.
>
> Jim Laskey has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Add between
Marked as reviewed by bpb (Reviewer).
-
PR: https://git.openjdk.java.net/jdk/pull/8109
Re: RFR: 5087440: java.io bulk read(...) end-of-stream return value descriptions ambiguous [v2]
> Minimal version of possible fixes: make the bulk read `@return` verbiage > consistent in the `java.io` package. Brian Burkhalter has refreshed the contents of this pull request, and previous commits have been removed. The incremental views will show differences compared to the previous content of the PR. The pull request contains one new commit since the last revision: 5087440: java.io bulk read(...) end-of-stream return value descriptions ambiguous - Changes: - all: https://git.openjdk.java.net/jdk/pull/8044/files - new: https://git.openjdk.java.net/jdk/pull/8044/files/360a5a9c..3de7caa8 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=8044=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk=8044=00-01 Stats: 0 lines in 0 files changed: 0 ins; 0 del; 0 mod Patch: https://git.openjdk.java.net/jdk/pull/8044.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8044/head:pull/8044 PR: https://git.openjdk.java.net/jdk/pull/8044
Re: RFR: 8283996: Reduce cost of year and month calculations
On Wed, 30 Mar 2022 12:06:39 GMT, Claes Redestad wrote: > A few integer divisions and multiplications can be replaced with test + > addition, leading to a decent speed-up on java.time microbenchmarks such as > `GetYearBench`. Numbers from my local x86 workstation, seeing similar > speed-up on aarch64 and other x86 setups. > > Baseline: > > BenchmarkMode Cnt Score > Error Units > GetYearBench.getYearFromMillisZoneOffsetthrpt 15 18.492 ± > 0.017 ops/ms > GetYearBench.getYearFromMillisZoneRegionthrpt 15 6.121 ± > 0.135 ops/ms > GetYearBench.getYearFromMillisZoneRegionNormalized thrpt 15 18.936 ± > 0.012 ops/ms > GetYearBench.getYearFromMillisZoneRegionUTC thrpt 15 9.283 ± > 0.222 ops/ms > > Patched: > > BenchmarkMode Cnt Score > Error Units > GetYearBench.getYearFromMillisZoneOffsetthrpt 15 20.931 ± > 0.013 ops/ms > GetYearBench.getYearFromMillisZoneRegionthrpt 15 6.858 ± > 0.167 ops/ms > GetYearBench.getYearFromMillisZoneRegionNormalized thrpt 15 20.923 ± > 0.017 ops/ms > GetYearBench.getYearFromMillisZoneRegionUTC thrpt 15 10.028 ± > 0.182 ops/ms > > > Testing: java.time tests locally, CI tier1+2 ongoing. Looks all right assuming tests pass. - Marked as reviewed by bpb (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/8039
RFR: 5087440: (ch spec) java.io,nio bulk read(...) end-of-stream return value descriptions ambiguous
Minimal version of possible fixes: make the bulk read `@return` verbiage consistent in the `java.io` package. - Commit messages: - 5087440: (ch spec) java.io,nio bulk read(...) end-of-stream return value descriptions ambiguous Changes: https://git.openjdk.java.net/jdk/pull/8044/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=8044=00 Issue: https://bugs.openjdk.java.net/browse/JDK-5087440 Stats: 14 lines in 3 files changed: 5 ins; 0 del; 9 mod Patch: https://git.openjdk.java.net/jdk/pull/8044.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8044/head:pull/8044 PR: https://git.openjdk.java.net/jdk/pull/8044
Re: RFR: 8283846: Remove unused jdk.internal.reflect.SignatureIterator
On Tue, 29 Mar 2022 09:15:01 GMT, Andrey Turbanov wrote: > It was no longer used due to JDK-4479184 long ago. It builds so looks fine. - Marked as reviewed by bpb (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/8013
Re: RFR: 8283801: Cleanup confusing String.toString calls
On Sun, 20 Mar 2022 13:20:31 GMT, Andrey Turbanov wrote: > String.toString() calls doesn't make much sense. Only one place, where it > could be used - to generate NPE. But in a few places of JDK codebase it's > called, even when NPE will happen anyway. > I propose to cleanup such places. > Found by IntelliJ IDEA inspection `Redundant 'String' operation`. Likewise, please add a `noreg-` label to the issue, perhaps `noreg-cleanup`. Otherwise fine. - Marked as reviewed by bpb (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/7878
Re: RFR: 8283800: Simplify String.indexOf/lastIndexOf calls
On Sun, 20 Mar 2022 12:45:34 GMT, Andrey Turbanov wrote: > In a few places String.indexOf/lastIndexOf methods are called with default > parameter for index: `0` for `indexOf`, length() for `lastIndexOf`. > I propose to cleanup such calls. It makes code a bit easier to read. In case > of `indexOf` it even could be faster, as there is separate intrinsic for > `indexOf` call without index argument. Marked as reviewed by bpb (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/7877
Re: RFR: 8283465: Character.UnicodeBlock.NUM_ENTITIES is out of date [v2]
On Tue, 22 Mar 2022 22:02:22 GMT, Naoto Sato wrote: >> Fixing the out-of-date number of entries in >> `Character.UnicodeBlock.NUM_ENTITIES` field. The regression test has been >> renamed and now repurposed just to examine whether the `NUM_ENTITIES` >> correctly has the `map.size()` value. > > Naoto Sato has updated the pull request incrementally with one additional > commit since the last revision: > > Comment adjusted per review suggestion Added comments look good. - Marked as reviewed by bpb (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/7909
Re: RFR: 8283465: Character.UnicodeBlock.NUM_ENTITIES is out of date
On Tue, 22 Mar 2022 18:44:09 GMT, Naoto Sato wrote: > Fixing the out-of-date number of entries in > `Character.UnicodeBlock.NUM_ENTITIES` field. The regression test has been > renamed and now repurposed just to examine whether the `NUM_ENTITIES` > correctly has the `map.size()` value. Marked as reviewed by bpb (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/7909
Re: RFR: JDK-8283124: Add constant for tau to Math and StrictMath
On Mon, 14 Mar 2022 20:52:39 GMT, Joe Darcy wrote: > Add a constant for tau, 2*pi, to Math and StrictMath. Since 2*pi is a very > common value in mathematical formulas, it is helpful to give it a distinct > constant. > > Please also review the CSR https://bugs.openjdk.java.net/browse/JDK-8283136 Marked as reviewed by bpb (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/7813
Integrated: 8058924: FileReader(String) documentation is insufficient
On Thu, 10 Mar 2022 02:30:35 GMT, Brian Burkhalter wrote: > Add a statement to the `java.io` package documentation clarifying how a > `String` representing a _pathname string_ is interpreted in the package. This pull request has now been integrated. Changeset: 13cebffe Author: Brian Burkhalter URL: https://git.openjdk.java.net/jdk/commit/13cebffe618255ae29310c95fd1b91576e576751 Stats: 6 lines in 1 file changed: 4 ins; 0 del; 2 mod 8058924: FileReader(String) documentation is insufficient Reviewed-by: naoto, lancea - PR: https://git.openjdk.java.net/jdk/pull/7767
Re: RFR: 8058924: FileReader(String) documentation is insufficient
On Thu, 10 Mar 2022 02:30:35 GMT, Brian Burkhalter wrote: > Add a statement to the `java.io` package documentation clarifying how a > `String` representing a _pathname string_ is interpreted in the package. CSR created: https://bugs.openjdk.java.net/browse/JDK-8282992 - PR: https://git.openjdk.java.net/jdk/pull/7767
Integrated: 8254574: PrintWriter handling of InterruptedIOException should be removed
On Wed, 16 Feb 2022 22:32:21 GMT, Brian Burkhalter wrote: > Remove reference to `java.io.InterruptedIOException` from > `java.io.PrintStream`, and make the specifications of `checkError()`, > `setError()`, and `clearError()` consistent between `java.io.PrintStream` and > `java.io.PrintWriter`. This pull request has now been integrated. Changeset: b13cacc5 Author:Brian Burkhalter URL: https://git.openjdk.java.net/jdk/commit/b13cacc575f58c206c928f2756698b027ee07b6f Stats: 21 lines in 2 files changed: 0 ins; 11 del; 10 mod 8254574: PrintWriter handling of InterruptedIOException should be removed Reviewed-by: alanb - PR: https://git.openjdk.java.net/jdk/pull/7507
RFR: 8058924: FileReader(String) documentation is insufficient
Add a statement to the `java.io` package documentation clarifying how a `String` representing a _pathname string_ is interpreted in the package. - Commit messages: - 8058924: FileReader(String) documentation is insufficient Changes: https://git.openjdk.java.net/jdk/pull/7767/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=7767=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8058924 Stats: 6 lines in 1 file changed: 4 ins; 0 del; 2 mod Patch: https://git.openjdk.java.net/jdk/pull/7767.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/7767/head:pull/7767 PR: https://git.openjdk.java.net/jdk/pull/7767
Re: RFR: 4511638: Double.toString(double) sometimes produces incorrect results [v6]
On Tue, 8 Feb 2022 22:11:34 GMT, Raffaello Giulietti wrote: >> Hello, >> >> here's a PR for a patch submitted on March 2020 >> [1](https://cr.openjdk.java.net/~bpb/4511638/webrev.04/) when Mercurial was >> a thing. >> >> The patch has been edited to adhere to OpenJDK code conventions about >> multi-line (block) comments. Nothing in the code proper has changed, except >> for the addition of redundant but clarifying parentheses in some expressions. >> >> >> Greetings >> Raffaello > > Raffaello Giulietti has updated the pull request with a new target base due > to a merge or a rebase. The pull request now contains 12 commits: > > - 4511638: Double.toString(double) sometimes produces incorrect results > >Adapted hashes in ElementStructureTest.java > - 4511638: Double.toString(double) sometimes produces incorrect results > >Merge branch 'master' into JDK-4511638 > - 4511638: Double.toString(double) sometimes produces incorrect results > >Enhanced intervals in MathUtils. >Updated references to Schubfach v4. > - 4511638: Double.toString(double) sometimes produces incorrect results > >Merge branch 'master' into JDK-4511638 > - 4511638: Double.toString(double) sometimes produces incorrect results > >Slight adjustments to Javadoc as suggested in the JDK-8202555 (CSR) > comments. > - 4511638: Double.toString(double) sometimes produces incorrect results > >Adjusted hashes in test/langtools/tools/javac/sym/ElementStructureTest.java > - 4511638: Double.toString(double) sometimes produces incorrect results > >Merge branch 'master' into JDK-4511638 > - 4511638: Double.toString(double) sometimes produces incorrect results > - 4511638: Double.toString(double) sometimes produces incorrect results > >Refactored test classes to better match OpenJDK conventions. >Added tests recommended by Guy Steele and Paul Zimmermann. > - Changed MAX_CHARS to static > - ... and 2 more: > https://git.openjdk.java.net/jdk/compare/92f4f40d...c29dff76 Keep PR open. - PR: https://git.openjdk.java.net/jdk/pull/3402
Re: RFR: 8254574: PrintWriter handling of InterruptedIOException PrintWriter handling of InterruptedIOException should be removed [v2]
> Remove reference to `java.io.InterruptedIOException` from > `java.io.PrintStream`, and make the specifications of `checkError()`, > `setError()`, and `clearError()` consistent between `java.io.PrintStream` and > `java.io.PrintWriter`. Brian Burkhalter has refreshed the contents of this pull request, and previous commits have been removed. The incremental views will show differences compared to the previous content of the PR. The pull request contains one new commit since the last revision: 8254574: PrintWriter handling of InterruptedIOException should be removed - Changes: - all: https://git.openjdk.java.net/jdk/pull/7507/files - new: https://git.openjdk.java.net/jdk/pull/7507/files/4015d9c0..95f15465 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=7507=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk=7507=00-01 Stats: 0 lines in 0 files changed: 0 ins; 0 del; 0 mod Patch: https://git.openjdk.java.net/jdk/pull/7507.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/7507/head:pull/7507 PR: https://git.openjdk.java.net/jdk/pull/7507
Re: RFR: 8264160: Regex \b is not consistent with \w without UNICODE_CHARACTER_CLASS [v5]
On Wed, 9 Mar 2022 17:49:11 GMT, Ian Graves wrote: >> Proposed change in behavior to correct inconsistencies between `\w` and `\b` >> metacharacters > > Ian Graves has updated the pull request incrementally with one additional > commit since the last revision: > > Removing superfluous 'if' Marked as reviewed by bpb (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/7539
Re: RFR: 8264160: Regex \b is not consistent with \w without UNICODE_CHARACTER_CLASS [v4]
On Wed, 9 Mar 2022 01:33:43 GMT, Ian Graves wrote:
>> Proposed change in behavior to correct inconsistencies between `\w` and `\b`
>> metacharacters
>
> Ian Graves has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Updating with additional descriptors. Removing DataProvider import
src/java.base/share/classes/java/util/regex/Pattern.java line 161:
> 159: * Any character (may or may not
> match line terminators)
> 160: * id="digit">{@code \d}
> 161: * A digit: {@code [0-9]} if if
>
Looks like there is a superfluous `if` here.
-
PR: https://git.openjdk.java.net/jdk/pull/7539
Re: RFR: JDK-8282144 RandomSupport.convertSeedBytesToLongs sign extension overwrites previous bytes
On Thu, 24 Feb 2022 14:47:50 GMT, Jim Laskey wrote: > Class: ./java.base/share/classes/jdk/internal/util/random/RandomSupport.java > Method: public static long[] convertSeedBytesToLongs(byte[] seed, int n, int > z) > > The method attempts to create an array of longs by consuming the input bytes > most significant bit first. New bytes are appended to the existing long using > the OR operator on the signed byte. Due to sign extension this will overwrite > all the existing bits from 63 to 8 if the next byte is negative. Marked as reviewed by bpb (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/7614
Re: RFR: JDK-8282144 RandomSupport.convertSeedBytesToLongs sign extension overwrites previous bytes
On Sun, 27 Feb 2022 22:30:44 GMT, Jim Laskey wrote:
>> test/jdk/java/util/Random/T8282144.java line 39:
>>
>>> 37: public class T8282144 {
>>> 38: public static void main(String[] args) {
>>> 39: RandomGenerator rng =
>>> RandomGeneratorFactory.of("L64X128MixRandom").create(42);
>>
>> Does `rng` always produce the same sequence? If so, then perhaps the seed,
>> `42`, should be a random value that is printed.
>
> 42 was chosen because its is known to produce negative byte values, other
> random values might not.
OK
>> test/jdk/java/util/Random/T8282144.java line 52:
>>
>>> 50: for (int k = 0; k < existing.length; k++) {
>>> 51: if (existing[k] != testing[k]) {
>>> 52: throw new
>>> RuntimeException("convertSeedBytesToLongs incorrect");
>>
>> Should `i`, `j`, and `k` be included in the exception message?
>
> Correctness is binary - either it works or it doesn't. The values of i, j, k
> would not assist in isolating issues. Might add to confusion if displayed.
Understood.
-
PR: https://git.openjdk.java.net/jdk/pull/7614
Re: RFR: JDK-8282144 RandomSupport.convertSeedBytesToLongs sign extension overwrites previous bytes
On Thu, 24 Feb 2022 14:47:50 GMT, Jim Laskey wrote:
> Class: ./java.base/share/classes/jdk/internal/util/random/RandomSupport.java
> Method: public static long[] convertSeedBytesToLongs(byte[] seed, int n, int
> z)
>
> The method attempts to create an array of longs by consuming the input bytes
> most significant bit first. New bytes are appended to the existing long using
> the OR operator on the signed byte. Due to sign extension this will overwrite
> all the existing bits from 63 to 8 if the next byte is negative.
test/jdk/java/util/Random/T8282144.java line 39:
> 37: public class T8282144 {
> 38: public static void main(String[] args) {
> 39: RandomGenerator rng =
> RandomGeneratorFactory.of("L64X128MixRandom").create(42);
Does `rng` always produce the same sequence? If so, then perhaps the seed,
`42`, should be a random value that is printed.
test/jdk/java/util/Random/T8282144.java line 52:
> 50: for (int k = 0; k < existing.length; k++) {
> 51: if (existing[k] != testing[k]) {
> 52: throw new
> RuntimeException("convertSeedBytesToLongs incorrect");
Should `i`, `j`, and `k` be included in the exception message?
-
PR: https://git.openjdk.java.net/jdk/pull/7614
Re: RFR: 8282131: java.time.ZoneId should be a sealed abstract class
On Fri, 25 Feb 2022 19:02:55 GMT, Naoto Sato wrote: > Refactoring `java.time.ZoneId` class to be a sealed class. A CSR has also > been drafted. Marked as reviewed by bpb (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/7625
Re: RFR: 8282188: Unused static field MathContext.DEFAULT_DIGITS
On Fri, 18 Feb 2022 19:07:15 GMT, Andrey Turbanov wrote: > 8282188: Unused static field MathContext.DEFAULT_DIGITS Marked as reviewed by bpb (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/7538
Re: RFR: 8276686: Malformed Javadoc inline tags in JDK source in /java/util/regex/Pattern.java
On Thu, 17 Feb 2022 18:02:20 GMT, Ian Graves wrote: > Adding a missing period per this doc bug. Marked as reviewed by bpb (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/7521
Re: RFR: 8254574: PrintWriter handling of InterruptedIOException is not documented
On Wed, 16 Feb 2022 22:32:21 GMT, Brian Burkhalter wrote: > Remove reference to `java.io.InterruptedIOException` from > `java.io.PrintStream`, and make the specifications of `checkError()`, > `setError()`, and `clearError()` consistent between `java.io.PrintStream` and > `java.io.PrintWriter`. Thanks. I was holding off on that label until the PR approached consensus. - PR: https://git.openjdk.java.net/jdk/pull/7507
Re: RFR: 8254574: PrintWriter handling of InterruptedIOException is not documented
On Wed, 16 Feb 2022 22:32:21 GMT, Brian Burkhalter wrote:
> Remove reference to `java.io.InterruptedIOException` from
> `java.io.PrintStream`, and make the specifications of `checkError()`,
> `setError()`, and `clearError()` consistent between `java.io.PrintStream` and
> `java.io.PrintWriter`.
In the various methods which print there is still this sort of construct
try {
// print operation which can throw IOException
}
catch (InterruptedIOException x) {
Thread.currentThread().interrupt();
}
catch (IOException x) {
trouble = true;
}
where an `InterruptedIOException` causes the current thread to be interrupted.
Should this PR also excise these interrupts (as vestigial)? There is no longer
any description of this in the specifications of the two print stream classes
although in theory an, e.g., `OutputStream` which throws
`InterruptedIOException` could be passed in.
-
PR: https://git.openjdk.java.net/jdk/pull/7507
