Re: RFR: 8283620: System.out does not use the encoding/charset specified in the Javadoc [v2]
On Fri, 22 Apr 2022 18:10:27 GMT, Naoto Sato wrote:
>> src/java.base/share/classes/java/lang/System.java line 780:
>>
>>> 778: * The property may be set on the command line to the value
>>> 779: * {@code UTF-8}. Setting the property to a value other than
>>> {@code UTF-8}
>>> 780: * leads to unspecified behavior.
>>
>> I think the proposal to introduce two standard properties is good and is
>> consistent with the recently introduced native.encoding. I'm not 100% sure
>> that the sentence "The property may be set on the command line ..." is
>> appropriate for the spec of standard properties. We got away with that for
>> file.encoding in implNote but that isn't spec. I think we may have to
>> replace this with something that says that the Java runtime can be started
>> with the system property set to "UTF-8", starting it with the property set
>> to another value clears to undefined behavior.
>
> Thanks, Alan. Modified them as suggested.
I think Alan has a typo: "clears" -> "leads"
-
PR: https://git.openjdk.java.net/jdk/pull/8270
Re: RFR: 8283620: System.out does not use the encoding/charset specified in the Javadoc [v2]
On Fri, 22 Apr 2022 18:14:18 GMT, Naoto Sato wrote:
>> Promoting the internal system properties for `System.out` and `System.err`
>> so that users can override the encoding used for those streams to `UTF-8`,
>> aligning to the `Charset.defaultCharset()`. A CSR has also been drafted.
>
> Naoto Sato has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Modified the spec for the new system properties.
src/java.base/share/classes/java/lang/System.java line 774:
> 772: * Character encoding name for {@link System#out System.out}.
> 773: * The Java runtime can be started with the system property set
> to {@code UTF-8},
> 774: * starting it with the property set to another value clears to
> undefined behavior.
_leads_ to undefined behavior
-
PR: https://git.openjdk.java.net/jdk/pull/8270
Re: RFR: 8284642: Unexpected behavior of -XX:MaxDirectMemorySize=0 [v2]
On Fri, 22 Apr 2022 18:14:27 GMT, Harold Seigel wrote: >> Please review this small fix for JDK-8284642. The fix was tested by running >> Mach5 tiers 1-2 on Linux, Mac OS, and Windows, and Mach5 tiers 3-5 on Linux >> x64. Additionally, the modified test and the test in the bug report were >> run locally. >> >> Thanks, Harold > > Harold Seigel has updated the pull request incrementally with one additional > commit since the last revision: > > restore source, fix man page I didn't realize that text was from the manpage. It is fine but I think we also need something in globals.hpp: product(uint64_t, MaxDirectMemorySize, 0, \ "Maximum total size of NIO direct-buffer allocations. "\ "Ignored if not explicitly set.") range(0, max_jlong) \ - PR: https://git.openjdk.java.net/jdk/pull/8222
Re: RFR: 8285445: cannot open file "NUL:"
Is that really a needed fix? enabling ADS and less strict parsing might introduce vulnerability but on the other hand NUL: should be allowed for it is a drive style not a ADS. I would also think the number of users who want use NUL: is smaller than the number of users who benefit from ADS being forbidden. Especially since \\.\NUL or NUL should already work. Gruss Bernd -- http://bernd.eckenfels.net Von: core-libs-dev im Auftrag von Brian Burkhalter Gesendet: Saturday, April 23, 2022 3:20:02 AM An: core-libs-dev@openjdk.java.net Betreff: 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: 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`. Marked as reviewed by mikael (Reviewer). - 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: 8283660: Convert com/sun/jndi/ldap/AbstractLdapNamingEnumeration.java finalizer to Cleaner [v2]
> Please review this change to replace the finalizer in > `AbstractLdapNamingEnumeration` with a Cleaner. > > The pieces of state required for cleanup (`LdapCtx homeCtx`, `LdapResult > res`, and `LdapClient enumClnt`) are moved to a static inner class . From > there, the change is fairly mechanical. > > Details of note: > 1. Some operations need to change the state values (the update() method is > probably the most interesting). > 2. Subclasses need to access `homeCtx`; I added a `homeCtx()` method to read > `homeCtx` from the superclass's `state`. > > The test case is based on a copy of > `com/sun/jndi/ldap/blits/AddTests/AddNewEntry.java`. A more minimal test case > might be possible, but this was done for expediency. > > The test only confirms that the new Cleaner use does not keep the object > reachable. It only tests `LdapSearchEnumeration` (not `LdapNamingEnumeration` > or `LdapBindingEnumeration`, though all are subclasses of > `AbstractLdapNamingEnumeration`). > > Thanks. Brent Christian has updated the pull request incrementally with one additional commit since the last revision: Rename inner class to EnumCtx ; use homeCtx() in AbstractLdapNamingEnumeration for consistencty ; new instance vars can be final - Changes: - all: https://git.openjdk.java.net/jdk/pull/8311/files - new: https://git.openjdk.java.net/jdk/pull/8311/files/3c957b51..1df8515b Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=8311=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk=8311=00-01 Stats: 30 lines in 1 file changed: 1 ins; 0 del; 29 mod Patch: https://git.openjdk.java.net/jdk/pull/8311.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8311/head:pull/8311 PR: https://git.openjdk.java.net/jdk/pull/8311
Re: RFR: 8283660: Convert com/sun/jndi/ldap/AbstractLdapNamingEnumeration.java finalizer to Cleaner
On Wed, 20 Apr 2022 13:48:21 GMT, Daniel Fuchs wrote:
>> Please review this change to replace the finalizer in
>> `AbstractLdapNamingEnumeration` with a Cleaner.
>>
>> The pieces of state required for cleanup (`LdapCtx homeCtx`, `LdapResult
>> res`, and `LdapClient enumClnt`) are moved to a static inner class . From
>> there, the change is fairly mechanical.
>>
>> Details of note:
>> 1. Some operations need to change the state values (the update() method is
>> probably the most interesting).
>> 2. Subclasses need to access `homeCtx`; I added a `homeCtx()` method to read
>> `homeCtx` from the superclass's `state`.
>>
>> The test case is based on a copy of
>> `com/sun/jndi/ldap/blits/AddTests/AddNewEntry.java`. A more minimal test
>> case might be possible, but this was done for expediency.
>>
>> The test only confirms that the new Cleaner use does not keep the object
>> reachable. It only tests `LdapSearchEnumeration` (not
>> `LdapNamingEnumeration` or `LdapBindingEnumeration`, though all are
>> subclasses of `AbstractLdapNamingEnumeration`).
>>
>> Thanks.
>
> src/java.naming/share/classes/com/sun/jndi/ldap/AbstractLdapNamingEnumeration.java
> line 73:
>
>> 71: public void run() {
>> 72: if (enumClnt != null) {
>> 73: enumClnt.clearSearchReply(res, homeCtx.reqCtls);
>
> It's a bit strange to see that there is no guard here to verify that `homeCtx
> != null`, when line 76 implies that it might. My reading is that `homeCtxt`
> is not supposed to be `null` when `enumClnt` is not `null`. That could be
> explained in a comment, or alternatively asserted just before line 73
> (`assert homeCtx != null;`)
Yes, it is strange -- that code came from the finalizer. I will add an assert.
> src/java.naming/share/classes/com/sun/jndi/ldap/AbstractLdapNamingEnumeration.java
> line 83:
>
>> 81: }
>> 82:
>> 83: private CleaningAction state;
>
> I wonder if state should be final?
Makes sense to me. `cleanable` can be final, too.
-
PR: https://git.openjdk.java.net/jdk/pull/8311
Re: RFR: 8285366: Fix typos in serviceability
On Thu, 21 Apr 2022 11:22:48 GMT, Magnus Ihse Bursie wrote: > I ran `codespell` on modules owned by the serviceability team > (`java.instrument java.management.rmi java.management jdk.attach > jdk.hotspot.agent jdk.internal.jvmstat jdk.jcmd jdk.jconsole jdk.jdi > jdk.jdwp.agent jdk.jstatd jdk.management.agent jdk.management`), and accepted > those changes where it indeed discovered real typos. > > > I will update copyright years using a script before pushing (otherwise like > every second change would be a copyright update, making reviewing much > harder). > > The long term goal here is to make tooling support for running `codespell`. > The trouble with automating this is of course all false positives. But before > even trying to solve that issue, all true positives must be fixed. Hence this > PR. Hi Magnus, Great job, thank you for doing this! It looks good to me. These findings are pretty interesting. Some are funny and some seems to be typical. Good examples of typical typos are: - double letter instead of single - single or even triple letter instead of double Thanks, Serguei - Marked as reviewed by sspitsyn (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/8334
Re: RFR: 8285477: Add a PRECISION public static field to j.l.Float and j.l.Double [v3]
On Fri, 22 Apr 2022 17:36:26 GMT, Raffaello Giulietti wrote: >> Add useful constants specified in IEEE 754. > > Raffaello Giulietti has updated the pull request incrementally with one > additional commit since the last revision: > > 8285477: Add a PRECISION public static field to j.l.Float and j.l.Double Marked as reviewed by darcy (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/8362
Re: RFR: 8283620: System.out does not use the encoding/charset specified in the Javadoc [v2]
> Promoting the internal system properties for `System.out` and `System.err` so > that users can override the encoding used for those streams to `UTF-8`, > aligning to the `Charset.defaultCharset()`. A CSR has also been drafted. Naoto Sato has updated the pull request incrementally with one additional commit since the last revision: Modified the spec for the new system properties. - Changes: - all: https://git.openjdk.java.net/jdk/pull/8270/files - new: https://git.openjdk.java.net/jdk/pull/8270/files/9ef42917..b5bcb870 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=8270=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk=8270=00-01 Stats: 6 lines in 1 file changed: 0 ins; 2 del; 4 mod Patch: https://git.openjdk.java.net/jdk/pull/8270.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8270/head:pull/8270 PR: https://git.openjdk.java.net/jdk/pull/8270
Re: RFR: 8284642: Unexpected behavior of -XX:MaxDirectMemorySize=0 [v2]
> Please review this small fix for JDK-8284642. The fix was tested by running > Mach5 tiers 1-2 on Linux, Mac OS, and Windows, and Mach5 tiers 3-5 on Linux > x64. Additionally, the modified test and the test in the bug report were run > locally. > > Thanks, Harold Harold Seigel has updated the pull request incrementally with one additional commit since the last revision: restore source, fix man page - Changes: - all: https://git.openjdk.java.net/jdk/pull/8222/files - new: https://git.openjdk.java.net/jdk/pull/8222/files/c178c602..5fdb220a Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=8222=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk=8222=00-01 Stats: 9 lines in 3 files changed: 0 ins; 0 del; 9 mod Patch: https://git.openjdk.java.net/jdk/pull/8222.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8222/head:pull/8222 PR: https://git.openjdk.java.net/jdk/pull/8222
Re: RFR: 8284642: Unexpected behavior of -XX:MaxDirectMemorySize=0
On Wed, 13 Apr 2022 12:24:46 GMT, Harold Seigel wrote: > Please review this small fix for JDK-8284642. The fix was tested by running > Mach5 tiers 1-2 on Linux, Mac OS, and Windows, and Mach5 tiers 3-5 on Linux > x64. Additionally, the modified test and the test in the bug report were run > locally. > > Thanks, Harold Please review the updated man page change. Thanks, Harold - PR: https://git.openjdk.java.net/jdk/pull/8222
Re: RFR: 8283620: System.out does not use the encoding/charset specified in the Javadoc [v2]
On Fri, 22 Apr 2022 09:31:19 GMT, Alan Bateman wrote:
>> Naoto Sato has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Modified the spec for the new system properties.
>
> src/java.base/share/classes/java/lang/System.java line 780:
>
>> 778: * The property may be set on the command line to the value
>> 779: * {@code UTF-8}. Setting the property to a value other than
>> {@code UTF-8}
>> 780: * leads to unspecified behavior.
>
> I think the proposal to introduce two standard properties is good and is
> consistent with the recently introduced native.encoding. I'm not 100% sure
> that the sentence "The property may be set on the command line ..." is
> appropriate for the spec of standard properties. We got away with that for
> file.encoding in implNote but that isn't spec. I think we may have to replace
> this with something that says that the Java runtime can be started with the
> system property set to "UTF-8", starting it with the property set to another
> value clears to undefined behavior.
Thanks, Alan. Modified them as suggested.
-
PR: https://git.openjdk.java.net/jdk/pull/8270
Re: RFR: 8285477: Add a PRECISION public static field to j.l.Float and j.l.Double [v2]
On Fri, 22 Apr 2022 16:34:46 GMT, Joe Darcy wrote:
>> Raffaello Giulietti has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> 8285477: Add a PRECISION public static field to j.l.Float and j.l.Double
>
> src/java.base/share/classes/java/lang/Double.java line 231:
>
>> 229: * @since 1.6
>> 230: */
>> 231: public static final int MAX_EXPONENT = (1 << (SIZE - PRECISION -
>> 1)) - 1;
>
> Please include the expected value as a comment; e.g.
>
> (1 << (SIZE - PRECISION - 1)) - 1; // 1023
Ready
> src/java.base/share/classes/java/lang/Float.java line 128:
>
>> 126: * The number of bits in the significand of a {@code float} value.
>> 127: * This is the parameter N in section {@jls 4.2.3} of
>> 128: * The Java Language Specification.
>
> Please use "cite" rather than "em" tags for references to the JLS.
Ready
-
PR: https://git.openjdk.java.net/jdk/pull/8362
Re: RFR: 8285477: Add a PRECISION public static field to j.l.Float and j.l.Double [v3]
> Add useful constants specified in IEEE 754. Raffaello Giulietti has updated the pull request incrementally with one additional commit since the last revision: 8285477: Add a PRECISION public static field to j.l.Float and j.l.Double - Changes: - all: https://git.openjdk.java.net/jdk/pull/8362/files - new: https://git.openjdk.java.net/jdk/pull/8362/files/80a6fe2e..b4c1b61b Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=8362=02 - incr: https://webrevs.openjdk.java.net/?repo=jdk=8362=01-02 Stats: 6 lines in 2 files changed: 0 ins; 0 del; 6 mod Patch: https://git.openjdk.java.net/jdk/pull/8362.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8362/head:pull/8362 PR: https://git.openjdk.java.net/jdk/pull/8362
Re: RFR: 8285452: Support new API to replace a file content using FileUtils.java [v3]
On Fri, 22 Apr 2022 16:06:22 GMT, Daniel Fuchs wrote:
>> Sibabrata Sahoo has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> Update FileUtils.java
>
> test/lib/jdk/test/lib/util/FileUtils.java line 402:
>
>> 400: if (!removed.equals(froms)) {
>> 401: throw new IOException("Removed not the same");
>> 402: }
>
> That's a bit strange. I would suggest to return the removed lines instead, or
> to pass a `Consumer` (or even better, a `Predicate` ?) that
> will accept the removed lines. You could continue to remove if the predicate
> returns true and throw if it returns false. It would also enable you to tell
> exactly which line failed the check.
I was just thinking about providing the removed lines and the added lines at
the same time into the method (just like what a patch file looks like). The
exception here can probably be enhanced to compare the content of `removed`
with `from`. Two blocks of code (call and callback) would be needed if a
consumer or predicate is used, and I don't feel it's worth doing. Here I've
already trimmed the lines to make sure whitespaces do not matter.
-
PR: https://git.openjdk.java.net/jdk/pull/8360
Re: RFR: 8285477: Add a PRECISION public static field to j.l.Float and j.l.Double [v2]
On Fri, 22 Apr 2022 14:38:06 GMT, Raffaello Giulietti
wrote:
>> Add useful constants specified in IEEE 754.
>
> Raffaello Giulietti has updated the pull request incrementally with one
> additional commit since the last revision:
>
> 8285477: Add a PRECISION public static field to j.l.Float and j.l.Double
src/java.base/share/classes/java/lang/Double.java line 231:
> 229: * @since 1.6
> 230: */
> 231: public static final int MAX_EXPONENT = (1 << (SIZE - PRECISION - 1))
> - 1;
Please include the expected value as a comment; e.g.
(1 << (SIZE - PRECISION - 1)) - 1; // 1023
src/java.base/share/classes/java/lang/Float.java line 128:
> 126: * The number of bits in the significand of a {@code float} value.
> 127: * This is the parameter N in section {@jls 4.2.3} of
> 128: * The Java Language Specification.
Please use "cite" rather than "em" tags for references to the JLS.
-
PR: https://git.openjdk.java.net/jdk/pull/8362
Re: RFR: 8285452: Support new API to replace a file content using FileUtils.java [v3]
On Fri, 22 Apr 2022 14:35:14 GMT, Sibabrata Sahoo wrote:
>> A new API to support replacing selective lines with desired content.
>
> Sibabrata Sahoo has updated the pull request incrementally with one
> additional commit since the last revision:
>
> Update FileUtils.java
test/lib/jdk/test/lib/util/FileUtils.java line 402:
> 400: if (!removed.equals(froms)) {
> 401: throw new IOException("Removed not the same");
> 402: }
That's a bit strange. I would suggest to return the removed lines instead, or
to pass a `Consumer` (or even better, a `Predicate` ?) that
will accept the removed lines. You could continue to remove if the predicate
returns true and throw if it returns false. It would also enable you to tell
exactly which line failed the check.
-
PR: https://git.openjdk.java.net/jdk/pull/8360
RFR: 8285485: Fix typos in corelibs
I ran `codespell` on modules owned by core-libs, and accepted those changes where it indeed discovered real typos. I will update copyright years using a script before pushing (otherwise like every second change would be a copyright update, making reviewing much harder). The long term goal here is to make tooling support for running `codespell`. The trouble with automating this is of course all false positives. But before even trying to solve that issue, all true positives must be fixed. Hence this PR. - Commit messages: - Pass #2 core - Pass #1 core - Pass #2 misc - Pass #1 misc Changes: https://git.openjdk.java.net/jdk/pull/8364/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=8364=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8285485 Stats: 215 lines in 103 files changed: 0 ins; 0 del; 215 mod Patch: https://git.openjdk.java.net/jdk/pull/8364.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8364/head:pull/8364 PR: https://git.openjdk.java.net/jdk/pull/8364
Re: RFR: 8285477: Add a PRECISION public static field to j.l.Float and j.l.Double [v2]
> Add useful constants specified in IEEE 754. Raffaello Giulietti has updated the pull request incrementally with one additional commit since the last revision: 8285477: Add a PRECISION public static field to j.l.Float and j.l.Double - Changes: - all: https://git.openjdk.java.net/jdk/pull/8362/files - new: https://git.openjdk.java.net/jdk/pull/8362/files/830faf2b..80a6fe2e Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=8362=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk=8362=00-01 Stats: 2 lines in 2 files changed: 0 ins; 0 del; 2 mod Patch: https://git.openjdk.java.net/jdk/pull/8362.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8362/head:pull/8362 PR: https://git.openjdk.java.net/jdk/pull/8362
Re: RFR: 8285452: Support new API to replace a file content using FileUtils.java [v3]
On Fri, 22 Apr 2022 14:30:57 GMT, Sibabrata Sahoo wrote: >> A new API to support replacing selective lines with desired content. > > Sibabrata Sahoo has updated the pull request incrementally with one > additional commit since the last revision: > > Update FileUtils.java API doc added along with some changes in the code. - PR: https://git.openjdk.java.net/jdk/pull/8360
Re: RFR: 8285452: Support new API to replace a file content using FileUtils.java [v3]
> A new API to support replacing selective lines with desired content. Sibabrata Sahoo has updated the pull request incrementally with one additional commit since the last revision: Update FileUtils.java - Changes: - all: https://git.openjdk.java.net/jdk/pull/8360/files - new: https://git.openjdk.java.net/jdk/pull/8360/files/0ec01e0b..ef5dc31a Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=8360=02 - incr: https://webrevs.openjdk.java.net/?repo=jdk=8360=01-02 Stats: 24 lines in 1 file changed: 15 ins; 0 del; 9 mod Patch: https://git.openjdk.java.net/jdk/pull/8360.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8360/head:pull/8360 PR: https://git.openjdk.java.net/jdk/pull/8360
Re: RFR: 8285477: Add a PRECISION public static field to j.l.Float and j.l.Double
On Fri, 22 Apr 2022 14:26:07 GMT, Raffaello Giulietti wrote: > Add useful constants specified in IEEE 754. The precision of float resp. double values, as defined by IEEE 754, is not easily derivable from the constants in the java.lang.Float resp. java.lang.Double classes. These values (24 resp. 53) are used in some places in the OpenJDK code base, where they appear as literals or even as derived literals, like 23 and 52. Hence, usages of these values are harder to find than necessary. - PR: https://git.openjdk.java.net/jdk/pull/8362
RFR: 8285477: Add a PRECISION public static field to j.l.Float and j.l.Double
Add useful constants specified in IEEE 754. - Commit messages: - 8285477: Add a PRECISION public static field to j.l.Float and j.l.Double Changes: https://git.openjdk.java.net/jdk/pull/8362/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=8362=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8285477 Stats: 50 lines in 2 files changed: 32 ins; 14 del; 4 mod Patch: https://git.openjdk.java.net/jdk/pull/8362.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8362/head:pull/8362 PR: https://git.openjdk.java.net/jdk/pull/8362
Re: RFR: 8285452: Support new API to replace a file content using FileUtils.java [v2]
On Fri, 22 Apr 2022 12:36:15 GMT, Sibabrata Sahoo wrote:
>> A new API to support replacing selective lines with desired content.
>
> Sibabrata Sahoo has updated the pull request incrementally with one
> additional commit since the last revision:
>
> Update FileUtils.java
test/lib/jdk/test/lib/util/FileUtils.java line 381:
> 379:
> 380: public static void patch(Path path, int fromLine, int toLine, String
> to) throws IOException {
> 381: if(fromLine < 1 || toLine < 1) {
It would be good to add a proper API doc comment, especially regarding the
meaning of the parameters, and whether the line in question is
included/excluded. Also RuntimeException could be replaced with
IndexOutOfBoundsException.
-
PR: https://git.openjdk.java.net/jdk/pull/8360
Re: RFR: 8285452: Support new API to replace a file content using FileUtils.java [v2]
> A new API to support replacing selective lines with desired content. Sibabrata Sahoo has updated the pull request incrementally with one additional commit since the last revision: Update FileUtils.java - Changes: - all: https://git.openjdk.java.net/jdk/pull/8360/files - new: https://git.openjdk.java.net/jdk/pull/8360/files/39fa164a..0ec01e0b Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=8360=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk=8360=00-01 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/8360.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8360/head:pull/8360 PR: https://git.openjdk.java.net/jdk/pull/8360
RFR: 8285452: Support new API to replace a file content using FileUtils.java
A new API to support replacing selective lines with desired content. - Commit messages: - 8285452: Support new API to replace a file content using FileUtils.java - Revert "8285452: Support new API to replace a file content in FileUtils.java" - 8285452: Support new API to replace a file content in FileUtils.java Changes: https://git.openjdk.java.net/jdk/pull/8360/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=8360=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8285452 Stats: 15 lines in 1 file changed: 15 ins; 0 del; 0 mod Patch: https://git.openjdk.java.net/jdk/pull/8360.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8360/head:pull/8360 PR: https://git.openjdk.java.net/jdk/pull/8360
Re: RFR: 8285440: Typo in Collections.addAll method javadoc
On Fri, 31 Dec 2021 18:58:43 GMT, Johnny Lim wrote: > This PR fixes a typo. Thank you for contributing. The change looks fine to me. After someone with a Reviewer role approves this change, I can sponsor this for you. - Marked as reviewed by jpai (Committer). PR: https://git.openjdk.java.net/jdk/pull/6942
Re: RFR: 8283620: System.out does not use the encoding/charset specified in the Javadoc
On Fri, 15 Apr 2022 20:26:55 GMT, Naoto Sato wrote:
> Promoting the internal system properties for `System.out` and `System.err` so
> that users can override the encoding used for those streams to `UTF-8`,
> aligning to the `Charset.defaultCharset()`. A CSR has also been drafted.
src/java.base/share/classes/java/lang/System.java line 780:
> 778: * The property may be set on the command line to the value
> 779: * {@code UTF-8}. Setting the property to a value other than
> {@code UTF-8}
> 780: * leads to unspecified behavior.
I think the proposal to introduce two standard properties is good and is
consistent with the recently introduced native.encoding. I'm not 100% sure that
the sentence "The property may be set on the command line ..." is appropriate
for the spec of standard properties. We got away with that for file.encoding in
implNote but that isn't spec. I think we may have to replace this with
something that says that the Java runtime can be started with the system
property set to "UTF-8", starting it with the property set to another value
clears to undefined behavior.
-
PR: https://git.openjdk.java.net/jdk/pull/8270
Re: RFR: 8284161: Implementation of Virtual Threads (Preview) [v3]
On Sun, 17 Apr 2022 04:57:34 GMT, Jaikiran Pai wrote:
>> 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:
>>
>> - Refresh
>> - Refresh
>> - Merge with jdk-19+18
>> - Refresh
>> - Initial push
>
> src/java.base/share/classes/java/lang/StackTraceElement.java line 556:
>
>> 554: }
>> 555:
>> 556: static StackTraceElement[] of(StackTraceElement[] stackTrace) {
>
> Is it intentional that this method is returning a `StackTraceElement[]`
> instead of `void`. The current implementation just returns back the passed
> array after operating on each element. Are callers mandated to use the
> returned array for subsequent operations instead of the passed array?
It's just for convenience of use.
-
PR: https://git.openjdk.java.net/jdk/pull/8166
Re: RFR: 8285440: Typo in Collections.addAll method javadoc
On Fri, 22 Apr 2022 07:35:52 GMT, Jaikiran Pai wrote: >> @robilad Hi. Thanks for the feedback! >> >> I submitted it again. I tried to fill it as possible as I can this time. >> Please let me know if there's anything missing. > > Hello @izeye, I've created a JBS issue for this change > https://bugs.openjdk.java.net/browse/JDK-8285440. Please edit the title of > this PR to "8285440: Typo in Collections.addAll method javadoc" so that it > triggers the workflow of the official review process. @jaikiran Thanks! I updated the title as you guided. - PR: https://git.openjdk.java.net/jdk/pull/6942
Re: RFR: 8285440: Typo in Collections.addAll method javadoc
On Fri, 25 Mar 2022 00:55:00 GMT, Johnny Lim wrote: >> Hi, >> >> please (re)submit a completed OCA at OCA.opensource.oracle.com. > > @robilad Hi. Thanks for the feedback! > > I submitted it again. I tried to fill it as possible as I can this time. > Please let me know if there's anything missing. Hello @izeye, I've created a JBS issue for this change https://bugs.openjdk.java.net/browse/JDK-8285440. Please edit the title of this PR to "8285440: Typo in Collections.addAll method javadoc" so that it triggers the workflow of the official review process. - PR: https://git.openjdk.java.net/jdk/pull/6942
Re: RFR: 8285440: Typo in Collections.addAll method javadoc
On Thu, 24 Mar 2022 20:28:59 GMT, Dalibor Topic wrote: >> This PR fixes a typo. > > Hi, > > please (re)submit a completed OCA at OCA.opensource.oracle.com. @robilad Hi. Thanks for the feedback! I submitted it again. I tried to fill it as possible as I can this time. Please let me know if there's anything missing. - PR: https://git.openjdk.java.net/jdk/pull/6942
Re: RFR: 8285440: Typo in Collections.addAll method javadoc
On Fri, 31 Dec 2021 18:58:43 GMT, Johnny Lim wrote: > This PR fixes a typo. Hi, please (re)submit a completed OCA at OCA.opensource.oracle.com. - PR: https://git.openjdk.java.net/jdk/pull/6942
Re: RFR: 8285440: Typo in Collections.addAll method javadoc
On Fri, 31 Dec 2021 18:58:43 GMT, Johnny Lim wrote: > This PR fixes a typo. No need to worry, all you need to do is comment to reset the timer. There's nothing much else to do besides creating a JBS issue related to the PR (You do need one for the checks to pass, unfortunately I can't help you there since I don't have access to the bug tracker) Besides, Bridgekeeper is a bot, it can't reply to your mention :P - PR: https://git.openjdk.java.net/jdk/pull/6942
Re: RFR: 8285440: Typo in Collections.addAll method javadoc
On Fri, 31 Dec 2021 18:58:43 GMT, Johnny Lim wrote: > This PR fixes a typo. @bridgekeeper I was waiting for "oca-verify", so I'm a bit confused. Please let me know if there's anything I need to do. - PR: https://git.openjdk.java.net/jdk/pull/6942
RFR: 8285440: Typo in Collections.addAll method javadoc
This PR fixes a typo. - Commit messages: - Fix typo Changes: https://git.openjdk.java.net/jdk/pull/6942/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=6942=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8285440 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/6942.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/6942/head:pull/6942 PR: https://git.openjdk.java.net/jdk/pull/6942
Re: RFR: 8283667: [vectorapi] Vectorization for masked load with IOOBE with predicate feature [v2]
On Wed, 20 Apr 2022 02:46:09 GMT, Xiaohong Gong wrote:
>> src/jdk.incubator.vector/share/classes/jdk/incubator/vector/ByteVector.java
>> line 2861:
>>
>>> 2859: ByteSpecies vsp = (ByteSpecies) species;
>>> 2860: if (offset >= 0 && offset <= (a.length -
>>> species.vectorByteSize())) {
>>> 2861: return vsp.dummyVector().fromByteArray0(a, offset, m, /*
>>> usePred */ false).maybeSwap(bo);
>>
>> Instead of usePred a term like inRange or offetInRage or offsetInVectorRange
>> would be easier to follow.
>
> Thanks for the review. I will change it later.
The name is updated to `offsetInRange`. Thanks!
-
PR: https://git.openjdk.java.net/jdk/pull/8035
Re: RFR: 8283667: [vectorapi] Vectorization for masked load with IOOBE with predicate feature [v2]
> Currently the vector load with mask when the given index happens out of the
> array boundary is implemented with pure java scalar code to avoid the IOOBE
> (IndexOutOfBoundaryException). This is necessary for architectures that do
> not support the predicate feature. Because the masked load is implemented
> with a full vector load and a vector blend applied on it. And a full vector
> load will definitely cause the IOOBE which is not valid. However, for
> architectures that support the predicate feature like SVE/AVX-512/RVV, it can
> be vectorized with the predicated load instruction as long as the indexes of
> the masked lanes are within the bounds of the array. For these architectures,
> loading with unmasked lanes does not raise exception.
>
> This patch adds the vectorization support for the masked load with IOOBE
> part. Please see the original java implementation (FIXME: optimize):
>
>
> @ForceInline
> public static
> ByteVector fromArray(VectorSpecies species,
>byte[] a, int offset,
>VectorMask m) {
> ByteSpecies vsp = (ByteSpecies) species;
> if (offset >= 0 && offset <= (a.length - species.length())) {
> return vsp.dummyVector().fromArray0(a, offset, m);
> }
>
> // FIXME: optimize
> checkMaskFromIndexSize(offset, vsp, m, 1, a.length);
> return vsp.vOp(m, i -> a[offset + i]);
> }
>
> Since it can only be vectorized with the predicate load, the hotspot must
> check whether the current backend supports it and falls back to the java
> scalar version if not. This is different from the normal masked vector load
> that the compiler will generate a full vector load and a vector blend if the
> predicate load is not supported. So to let the compiler make the expected
> action, an additional flag (i.e. `usePred`) is added to the existing
> "loadMasked" intrinsic, with the value "true" for the IOOBE part while
> "false" for the normal load. And the compiler will fail to intrinsify if the
> flag is "true" and the predicate load is not supported by the backend, which
> means that normal java path will be executed.
>
> Also adds the same vectorization support for masked:
> - fromByteArray/fromByteBuffer
> - fromBooleanArray
> - fromCharArray
>
> The performance for the new added benchmarks improve about `1.88x ~ 30.26x`
> on the x86 AVX-512 system:
>
> Benchmark before After Units
> LoadMaskedIOOBEBenchmark.byteLoadArrayMaskIOOBE 737.542 1387.069 ops/ms
> LoadMaskedIOOBEBenchmark.doubleLoadArrayMaskIOOBE 118.366 330.776 ops/ms
> LoadMaskedIOOBEBenchmark.floatLoadArrayMaskIOOBE 233.832 6125.026 ops/ms
> LoadMaskedIOOBEBenchmark.intLoadArrayMaskIOOBE233.816 7075.923 ops/ms
> LoadMaskedIOOBEBenchmark.longLoadArrayMaskIOOBE 119.771 330.587 ops/ms
> LoadMaskedIOOBEBenchmark.shortLoadArrayMaskIOOBE 431.961 939.301 ops/ms
>
> Similar performance gain can also be observed on 512-bit SVE system.
Xiaohong Gong has updated the pull request incrementally with one additional
commit since the last revision:
Rename the "usePred" to "offsetInRange"
-
Changes:
- all: https://git.openjdk.java.net/jdk/pull/8035/files
- new: https://git.openjdk.java.net/jdk/pull/8035/files/8f9e8a3c..9b2d2f19
Webrevs:
- full: https://webrevs.openjdk.java.net/?repo=jdk=8035=01
- incr: https://webrevs.openjdk.java.net/?repo=jdk=8035=00-01
Stats: 393 lines in 41 files changed: 0 ins; 0 del; 393 mod
Patch: https://git.openjdk.java.net/jdk/pull/8035.diff
Fetch: git fetch https://git.openjdk.java.net/jdk pull/8035/head:pull/8035
PR: https://git.openjdk.java.net/jdk/pull/8035
