Re: RFR: 8270490: Charset.forName() taking fallback default value [v2]
On Thu, 21 Oct 2021 01:30:05 GMT, Sergey Bylokhov wrote:
>> Naoto Sato has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Moved the null sentence into @param tag.
>
> src/java.base/share/classes/java/io/Console.java line 587:
>
>> 585: try {
>> 586: cs = Charset.forName(csname, null);
>> 587: } catch (Exception ignored) { }
>
> The comment which suggests this enhancement was about eliminating such
> "exception ignored" code paths. Is it still needed here? And if it is needed
> why do we pass the null as a fallback?
Right, I think both try-catch usages will be removed.
-
PR: https://git.openjdk.java.net/jdk/pull/6045
RFR: JDK-8275688: Suppress warnings on non-serializable non-transient instance fields in DualPivotQuicksort
This should be the last core libraries cleanup of non-serializable non-transient instance fields ahead of the upcoming javac lint warnings: https://mail.openjdk.java.net/pipermail/jdk-dev/2021-October/006166.html - Commit messages: - JDK-8275688: Suppress warnings on non-serializable non-transient instance fields in DualPivotQuicksort Changes: https://git.openjdk.java.net/jdk/pull/6058/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=6058&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8275688 Stats: 3 lines in 1 file changed: 3 ins; 0 del; 0 mod Patch: https://git.openjdk.java.net/jdk/pull/6058.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/6058/head:pull/6058 PR: https://git.openjdk.java.net/jdk/pull/6058
Re: RFR: JDK-8275686: Suppress warnings on non-serializable non-transient instance fields in java.rmi
On Thu, 21 Oct 2021 01:06:33 GMT, Joe Darcy wrote: > Another serialization warnings cleanup; this time in java.rmi. Marked as reviewed by iris (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/6055
Re: RFR: JDK-8275686: Suppress warnings on non-serializable non-transient instance fields in java.rmi
On Thu, 21 Oct 2021 01:06:33 GMT, Joe Darcy wrote: > Another serialization warnings cleanup; this time in java.rmi. Marked as reviewed by bpb (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/6055
Re: RFR: 8271515: Integration of JEP 417: Vector API (Third Incubator) [v5]
On Wed, 20 Oct 2021 03:13:07 GMT, Nick Gasson wrote: > AArch64 changes look ok apart from some minor comments. Thank you @nick-arm for the review! All your comments have been addressed. - PR: https://git.openjdk.java.net/jdk/pull/5873
Re: RFR: 8270490: Charset.forName() taking fallback default value [v2]
On Wed, 20 Oct 2021 19:02:30 GMT, Naoto Sato wrote:
>> During the review of JEP 400, a proposal to provide an overloaded method to
>> `Charset.forName()` was suggested
>> [[1]](https://github.com/openjdk/jdk/pull/4733#discussion_r669693954). This
>> PR is to implement the proposal. A CSR is also drafted as
>> https://bugs.openjdk.java.net/browse/JDK-8275348
>
> Naoto Sato has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Moved the null sentence into @param tag.
src/java.base/share/classes/java/io/Console.java line 587:
> 585: try {
> 586: cs = Charset.forName(csname, null);
> 587: } catch (Exception ignored) { }
The comment which suggests this enhancement was about eliminating such
"exception ignored" code paths. Is it still needed here? And if it is needed
why do we pass the null as a fallback?
src/java.base/share/classes/java/io/Console.java line 594:
> 592: cs = Charset.forName(StaticProperty.nativeEncoding(),
> Charset.defaultCharset());
> 593: } catch (Exception ignored) {
> 594: cs = Charset.defaultCharset();
What kind of actual improvements do we get here since the catch block is still
in place?
-
PR: https://git.openjdk.java.net/jdk/pull/6045
RFR: JDK-8275686: Suppress warnings on non-serializable non-transient instance fields in java.rmi
Another serialization warnings cleanup; this time in java.rmi. - Commit messages: - JDK-8275686: Suppress warnings on non-serializable non-transient instance fields in java.rmi Changes: https://git.openjdk.java.net/jdk/pull/6055/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=6055&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8275686 Stats: 1 line in 1 file changed: 1 ins; 0 del; 0 mod Patch: https://git.openjdk.java.net/jdk/pull/6055.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/6055/head:pull/6055 PR: https://git.openjdk.java.net/jdk/pull/6055
Integrated: 8275167: x86 intrinsic for unsignedMultiplyHigh
On Wed, 13 Oct 2021 18:55:10 GMT, Vamsi Parasa wrote: > Optimize the new Math.unsignedMultiplyHigh using the x86 mul instruction. > This change show 1.87X improvement on a micro benchmark. This pull request has now been integrated. Changeset: af7c56b8 Author:vamsi-parasa Committer: Sandhya Viswanathan URL: https://git.openjdk.java.net/jdk/commit/af7c56b85bb2828a9d68f9e1c753a4adfa7ebb4f Stats: 63 lines in 11 files changed: 61 ins; 2 del; 0 mod 8275167: x86 intrinsic for unsignedMultiplyHigh Reviewed-by: kvn, sviswanathan - PR: https://git.openjdk.java.net/jdk/pull/5933
Re: RFR: 8275167: x86 intrinsic for unsignedMultiplyHigh [v2]
On Wed, 20 Oct 2021 22:14:33 GMT, Vladimir Kozlov wrote: > Tests passed. You can integrate changes. Thanks Vladimir! What are the next steps to integrate the change? - PR: https://git.openjdk.java.net/jdk/pull/5933
Re: RFR: 8275167: x86 intrinsic for unsignedMultiplyHigh [v2]
On Tue, 19 Oct 2021 20:34:55 GMT, Vamsi Parasa wrote: >> Optimize the new Math.unsignedMultiplyHigh using the x86 mul instruction. >> This change show 1.87X improvement on a micro benchmark. > > Vamsi Parasa has updated the pull request incrementally with one additional > commit since the last revision: > > refactoring to remove code duplication by using a common routine for > UMulHiLNode and MulHiLNode Tests passed. You can integrate changes. - PR: https://git.openjdk.java.net/jdk/pull/5933
RFR: 8273660: Propagate CNF exception in FieldValues.get via an IOException
The ObjectInputStream.GetField method `get(String name, Object val)` should have been throwing a ClassNotFoundException if the class was not found. Instead the implementation was returning null. A design error does not allow the `get(String name, Object val)` method to throw CNFE as it should. However, an exception must be thrown to prevent invalid data from being returned. Wrapping the CNFE in IOException allows it to be thrown and the exception handled. The call to `get(String name, Object val)` is always from within a `readObject` method so the deserialization logic can catch the IOException and unwrap it to handle the CNFE. - Commit messages: - 8273660: Propagate CNF exception in FieldValues.get via an IOException Changes: https://git.openjdk.java.net/jdk/pull/6053/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=6053&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8273660 Stats: 112 lines in 2 files changed: 109 ins; 0 del; 3 mod Patch: https://git.openjdk.java.net/jdk/pull/6053.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/6053/head:pull/6053 PR: https://git.openjdk.java.net/jdk/pull/6053
Re: RFR: 8270490: Charset.forName() taking fallback default value [v2]
On Wed, 20 Oct 2021 19:02:30 GMT, Naoto Sato wrote: >> During the review of JEP 400, a proposal to provide an overloaded method to >> `Charset.forName()` was suggested >> [[1]](https://github.com/openjdk/jdk/pull/4733#discussion_r669693954). This >> PR is to implement the proposal. A CSR is also drafted as >> https://bugs.openjdk.java.net/browse/JDK-8275348 > > Naoto Sato has updated the pull request incrementally with one additional > commit since the last revision: > > Moved the null sentence into @param tag. Looks good - Marked as reviewed by rriggs (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/6045
Re: RFR: 8270490: Charset.forName() taking fallback default value [v2]
On Wed, 20 Oct 2021 19:02:30 GMT, Naoto Sato wrote: >> During the review of JEP 400, a proposal to provide an overloaded method to >> `Charset.forName()` was suggested >> [[1]](https://github.com/openjdk/jdk/pull/4733#discussion_r669693954). This >> PR is to implement the proposal. A CSR is also drafted as >> https://bugs.openjdk.java.net/browse/JDK-8275348 > > Naoto Sato has updated the pull request incrementally with one additional > commit since the last revision: > > Moved the null sentence into @param tag. Marked as reviewed by joehw (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/6045
Re: RFR: 8270490: Charset.forName() taking fallback default value [v2]
On Wed, 20 Oct 2021 18:58:34 GMT, Alan Bateman wrote: >> Thanks, Joe. Moved that explanation into the `fallback` param, which I >> initially intended. > > The java.nio.charset package has the usual "Unless otherwise noted, passing a > null argument ..." so if fallback is allowed to be null then you will need to > add ", can null" to its description. Our comments crossed I guess? Moved the null allowance into `fallback`. - PR: https://git.openjdk.java.net/jdk/pull/6045
Re: RFR: 8275167: x86 intrinsic for unsignedMultiplyHigh [v2]
On Tue, 19 Oct 2021 20:34:55 GMT, Vamsi Parasa wrote: >> Optimize the new Math.unsignedMultiplyHigh using the x86 mul instruction. >> This change show 1.87X improvement on a micro benchmark. > > Vamsi Parasa has updated the pull request incrementally with one additional > commit since the last revision: > > refactoring to remove code duplication by using a common routine for > UMulHiLNode and MulHiLNode Looks good. And I submitted testing. - Marked as reviewed by kvn (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/5933
Re: RFR: 8270490: Charset.forName() taking fallback default value [v2]
On Wed, 20 Oct 2021 18:37:05 GMT, Joe Wang wrote:
>> Naoto Sato has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Moved the null sentence into @param tag.
>
> src/java.base/share/classes/java/nio/charset/Charset.java line 545:
>
>> 543: * @return A charset object for the named charset, or {@code
>> fallback}
>> 544: * in case the charset object for the named charset is not
>> 545: * available. May be {@code null}
>
> A minor comment: it seems to me returning the fallback charset is sufficient,
> and "May be null" may be not necessary since the fallback charset is
> provided, it's expected if it's null.
Thanks, Joe. Moved that explanation into the `fallback` param, which I
initially intended.
-
PR: https://git.openjdk.java.net/jdk/pull/6045
Re: RFR: 8270490: Charset.forName() taking fallback default value [v2]
On Wed, 20 Oct 2021 18:56:22 GMT, Naoto Sato wrote:
>> src/java.base/share/classes/java/nio/charset/Charset.java line 545:
>>
>>> 543: * @return A charset object for the named charset, or {@code
>>> fallback}
>>> 544: * in case the charset object for the named charset is not
>>> 545: * available. May be {@code null}
>>
>> A minor comment: it seems to me returning the fallback charset is
>> sufficient, and "May be null" may be not necessary since the fallback
>> charset is provided, it's expected if it's null.
>
> Thanks, Joe. Moved that explanation into the `fallback` param, which I
> initially intended.
The java.nio.charset package has the usual "Unless otherwise noted, passing a
null argument ..." so if fallback is allowed to be null then you will need to
add ", can null" to its description.
-
PR: https://git.openjdk.java.net/jdk/pull/6045
Re: RFR: 8270490: Charset.forName() taking fallback default value [v2]
> During the review of JEP 400, a proposal to provide an overloaded method to > `Charset.forName()` was suggested > [[1]](https://github.com/openjdk/jdk/pull/4733#discussion_r669693954). This > PR is to implement the proposal. A CSR is also drafted as > https://bugs.openjdk.java.net/browse/JDK-8275348 Naoto Sato has updated the pull request incrementally with one additional commit since the last revision: Moved the null sentence into @param tag. - Changes: - all: https://git.openjdk.java.net/jdk/pull/6045/files - new: https://git.openjdk.java.net/jdk/pull/6045/files/a31dbdc7..4c2142be Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=6045&range=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=6045&range=00-01 Stats: 2 lines in 1 file changed: 0 ins; 0 del; 2 mod Patch: https://git.openjdk.java.net/jdk/pull/6045.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/6045/head:pull/6045 PR: https://git.openjdk.java.net/jdk/pull/6045
Re: RFR: 8244202: Implementation of JEP 418: Internet-Address Resolution SPI [v5]
On Wed, 20 Oct 2021 11:52:38 GMT, Aleksei Efimov wrote: >> This change implements a new service provider interface for host name and >> address resolution, so that java.net.InetAddress API can make use of >> resolvers other than the platform's built-in resolver. >> >> The following API classes are added to `java.net.spi` package to facilitate >> this: >> - `InetAddressResolverProvider` - abstract class defining a service, and >> is, essentially, a factory for `InetAddressResolver` resolvers. >> - `InetAddressResolverProvider.Configuration ` - an interface describing the >> platform's built-in configuration for resolution operations that could be >> used to bootstrap a resolver construction, or to implement partial >> delegation of lookup operations. >> - `InetAddressResolver` - an interface that defines methods for the >> fundamental forward and reverse lookup operations. >> - `InetAddressResolver.LookupPolicy` - a class whose instances describe the >> characteristics of one forward lookup operation. >> >> More details in [JEP-418](https://openjdk.java.net/jeps/418). >> >> Testing: new and existing `tier1:tier3` tests > > Aleksei Efimov has updated the pull request incrementally with one additional > commit since the last revision: > > Change InetAddressResolver method names src/java.base/share/classes/java/net/InetAddress.java line 244: > 242: * @implNote > 243: * For any lookup operation that might occur before the VM is fully > booted the built-in > 244: * resolver will be used. I think we will need decide if InetAddress class description is the right place for this or whether some/most of it should move to InetAddressResolverProvider. It might be that we update the existing "Host Name Resolution" section with some basic/readable text to make the reader aware that there is a provider mechanism with a link to InetAddressResolverProvider with the details. - PR: https://git.openjdk.java.net/jdk/pull/5822
Re: RFR: 8270490: Charset.forName() taking fallback default value
On Wed, 20 Oct 2021 17:23:36 GMT, Naoto Sato wrote:
> During the review of JEP 400, a proposal to provide an overloaded method to
> `Charset.forName()` was suggested
> [[1]](https://github.com/openjdk/jdk/pull/4733#discussion_r669693954). This
> PR is to implement the proposal. A CSR is also drafted as
> https://bugs.openjdk.java.net/browse/JDK-8275348
Marked as reviewed by joehw (Reviewer).
src/java.base/share/classes/java/nio/charset/Charset.java line 545:
> 543: * @return A charset object for the named charset, or {@code
> fallback}
> 544: * in case the charset object for the named charset is not
> 545: * available. May be {@code null}
A minor comment: it seems to me returning the fallback charset is sufficient,
and "May be null" may be not necessary since the fallback charset is provided,
it's expected if it's null.
-
PR: https://git.openjdk.java.net/jdk/pull/6045
Re: RFR: 8244202: Implementation of JEP 418: Internet-Address Resolution SPI [v5]
On Wed, 20 Oct 2021 11:52:38 GMT, Aleksei Efimov wrote:
>> This change implements a new service provider interface for host name and
>> address resolution, so that java.net.InetAddress API can make use of
>> resolvers other than the platform's built-in resolver.
>>
>> The following API classes are added to `java.net.spi` package to facilitate
>> this:
>> - `InetAddressResolverProvider` - abstract class defining a service, and
>> is, essentially, a factory for `InetAddressResolver` resolvers.
>> - `InetAddressResolverProvider.Configuration ` - an interface describing the
>> platform's built-in configuration for resolution operations that could be
>> used to bootstrap a resolver construction, or to implement partial
>> delegation of lookup operations.
>> - `InetAddressResolver` - an interface that defines methods for the
>> fundamental forward and reverse lookup operations.
>> - `InetAddressResolver.LookupPolicy` - a class whose instances describe the
>> characteristics of one forward lookup operation.
>>
>> More details in [JEP-418](https://openjdk.java.net/jeps/418).
>>
>> Testing: new and existing `tier1:tier3` tests
>
> Aleksei Efimov has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Change InetAddressResolver method names
src/java.base/share/classes/java/net/InetAddress.java line 340:
> 338:
> 339: /* Used to store the system-wide resolver */
> 340: private static volatile InetAddressResolver resolver;
Can this be @Stable?
src/java.base/share/classes/java/net/spi/InetAddressResolver.java line 34:
> 32:
> 33: /**
> 34: * This interface defines operations for looking-up host names and IP
> addresses.
I think change "looking-up" to "looking up".
src/java.base/share/classes/java/net/spi/InetAddressResolver.java line 102:
> 100:
> 101: /**
> 102: * Specifies if IPv4 addresses need to be queried during lookup.
It might be better to change this to "Characteristic value signifying ..."
rather than "Specifies".
src/java.base/share/classes/java/net/spi/InetAddressResolver.java line 186:
> 184:
> 185: /**
> 186: * Returns an integer value which specifies lookup operation
> characteristics.
I think I'd change the first line here to something like "Returns a set of
characteristics of this lookup policy".
src/java.base/share/classes/java/net/spi/InetAddressResolverProvider.java line
35:
> 33: /**
> 34: * A resolver provider class is a factory for custom implementations of
> {@linkplain
> 35: * InetAddressResolver resolvers} which define operations for looking-up
> (resolving) host names
This reads "custom implementations of resolvers". It might be better to start
with "Service-provider class for InetAddress resolvers".
src/java.base/share/classes/java/net/spi/InetAddressResolverProvider.java line
52:
> 50: /**
> 51: * Initialise and return the {@link InetAddressResolver} provided by
> 52: * this provider. This method is called by {@link InetAddress} when
"the InetAddressResolver" suggests there is just one instance. That is probably
the case but I don't think you want to be restricted to that.
src/java.base/share/classes/java/net/spi/InetAddressResolverProvider.java line
88:
> 86: * {@code
> RuntimePermission("inetAddressResolverProvider")}.
> 87: * @implNote It is recommended that an {@code
> InetAddressResolverProvider} service
> 88: * implementation does not perform any heavy initialization in its
"heavy initialization" is probably not the best phrase here. Maybe it should
say that the initialisation should be as simple as possible to avoid recursive
initialisation issues.
src/java.base/share/classes/java/net/spi/InetAddressResolverProvider.java line
110:
> 108: /**
> 109: * A {@code Configuration} interface is supplied to the
> 110: * {@link InetAddressResolverProvider#get(Configuration)} method
> when installing a
I think this should be "Configuration object" rather than "interface" here.
-
PR: https://git.openjdk.java.net/jdk/pull/5822
Re: RFR: 8275167: x86 intrinsic for unsignedMultiplyHigh [v2]
On Tue, 19 Oct 2021 20:34:55 GMT, Vamsi Parasa wrote: >> Optimize the new Math.unsignedMultiplyHigh using the x86 mul instruction. >> This change show 1.87X improvement on a micro benchmark. > > Vamsi Parasa has updated the pull request incrementally with one additional > commit since the last revision: > > refactoring to remove code duplication by using a common routine for > UMulHiLNode and MulHiLNode Marked as reviewed by sviswanathan (Reviewer). The patch looks good to me. - PR: https://git.openjdk.java.net/jdk/pull/5933
Re: RFR: 8275167: x86 intrinsic for unsignedMultiplyHigh [v2]
On Fri, 15 Oct 2021 20:19:31 GMT, Vladimir Kozlov wrote: >>> How you verified correctness of results? I suggest to extend >>> `test/jdk//java/lang/Math/MultiplicationTests.java` test to cover unsigned >>> method. >> >> Tests for unsignedMultiplyHigh were already added in >> test/jdk//java/lang/Math/MultiplicationTests.java (in July 2021 by Brian >> Burkhalter). Used that test to verify the correctness of the results. > >> > How you verified correctness of results? I suggest to extend >> > `test/jdk//java/lang/Math/MultiplicationTests.java` test to cover unsigned >> > method. >> >> Tests for unsignedMultiplyHigh were already added in >> test/jdk//java/lang/Math/MultiplicationTests.java (in July 2021 by Brian >> Burkhalter). Used that test to verify the correctness of the results. > > Good. It seems I have old version of the test. > Did you run it with -Xcomp? How you verified that intrinsic is used? @vnkozlov if the patch looks ok to you, could you please run this through your testing? - PR: https://git.openjdk.java.net/jdk/pull/5933
RFR: 8270490: Charset.forName() taking fallback default value
During the review of JEP 400, a proposal to provide an overloaded method to `Charset.forName()` was suggested [[1]](https://github.com/openjdk/jdk/pull/4733#discussion_r669693954). This PR is to implement the proposal. A CSR is also drafted as https://bugs.openjdk.java.net/browse/JDK-8275348 - Commit messages: - 8270490: Charset.forName() taking fallback default value Changes: https://git.openjdk.java.net/jdk/pull/6045/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=6045&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8270490 Stats: 117 lines in 3 files changed: 115 ins; 0 del; 2 mod Patch: https://git.openjdk.java.net/jdk/pull/6045.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/6045/head:pull/6045 PR: https://git.openjdk.java.net/jdk/pull/6045
Re: RFR: 8275650: Problemlist java/io/File/createTempFile/SpecialTempFile.java for Windows 11
On Wed, 20 Oct 2021 00:04:08 GMT, Ivan Šipka wrote: > cc @ctornqvi Marked as reviewed by iignatyev (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/6025
RFR: 8275650: Problemlist java/io/File/createTempFile/SpecialTempFile.java for Windows 11
cc @ctornqvi - Commit messages: - 8274122: added to problem list Changes: https://git.openjdk.java.net/jdk/pull/6025/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=6025&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8275650 Stats: 1 line in 1 file changed: 1 ins; 0 del; 0 mod Patch: https://git.openjdk.java.net/jdk/pull/6025.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/6025/head:pull/6025 PR: https://git.openjdk.java.net/jdk/pull/6025
Re: RFR: 8272614: Unused parameters in MethodHandleNatives linking methods
On Tue, 19 Oct 2021 17:12:16 GMT, Harold Seigel wrote: > Please review this small fix for JDK-8272614 to remove the unused indexInCP > argument to linkCallSite() and linkDynamicConstant(). The fix was tested > with Mach5 tiers 1-2 on Linux, Mac OS, and Windows, and Mach5 tiers 3-6 on > Linux x64. > > Thanks, Harold Thanks David and Lois! - PR: https://git.openjdk.java.net/jdk/pull/6021
Integrated: 8272614: Unused parameters in MethodHandleNatives linking methods
On Tue, 19 Oct 2021 17:12:16 GMT, Harold Seigel wrote: > Please review this small fix for JDK-8272614 to remove the unused indexInCP > argument to linkCallSite() and linkDynamicConstant(). The fix was tested > with Mach5 tiers 1-2 on Linux, Mac OS, and Windows, and Mach5 tiers 3-6 on > Linux x64. > > Thanks, Harold This pull request has now been integrated. Changeset: bbc60611 Author:Harold Seigel URL: https://git.openjdk.java.net/jdk/commit/bbc606117fcd8b48fc8f830c50cf7eb573da1c4c Stats: 8 lines in 3 files changed: 0 ins; 3 del; 5 mod 8272614: Unused parameters in MethodHandleNatives linking methods Reviewed-by: dholmes, lfoltan - PR: https://git.openjdk.java.net/jdk/pull/6021
Re: RFR: 8272614: Unused parameters in MethodHandleNatives linking methods
On Tue, 19 Oct 2021 17:12:16 GMT, Harold Seigel wrote: > Please review this small fix for JDK-8272614 to remove the unused indexInCP > argument to linkCallSite() and linkDynamicConstant(). The fix was tested > with Mach5 tiers 1-2 on Linux, Mac OS, and Windows, and Mach5 tiers 3-6 on > Linux x64. > > Thanks, Harold LGTM! - Marked as reviewed by lfoltan (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/6021
Re: RFR: 8244202: Implementation of JEP 418: Internet-Address Resolution SPI [v5]
On Wed, 20 Oct 2021 11:52:38 GMT, Aleksei Efimov wrote: >> This change implements a new service provider interface for host name and >> address resolution, so that java.net.InetAddress API can make use of >> resolvers other than the platform's built-in resolver. >> >> The following API classes are added to `java.net.spi` package to facilitate >> this: >> - `InetAddressResolverProvider` - abstract class defining a service, and >> is, essentially, a factory for `InetAddressResolver` resolvers. >> - `InetAddressResolverProvider.Configuration ` - an interface describing the >> platform's built-in configuration for resolution operations that could be >> used to bootstrap a resolver construction, or to implement partial >> delegation of lookup operations. >> - `InetAddressResolver` - an interface that defines methods for the >> fundamental forward and reverse lookup operations. >> - `InetAddressResolver.LookupPolicy` - a class whose instances describe the >> characteristics of one forward lookup operation. >> >> More details in [JEP-418](https://openjdk.java.net/jeps/418). >> >> Testing: new and existing `tier1:tier3` tests > > Aleksei Efimov has updated the pull request incrementally with one additional > commit since the last revision: > > Change InetAddressResolver method names Marked as reviewed by dfuchs (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/5822
Re: RFR: 8271515: Integration of JEP 417: Vector API (Third Incubator) [v6]
> This PR improves the performance of vector operations that accept masks on > architectures that support masking in hardware, specifically Intel AVX512 and > ARM SVE. > > On architectures that do not support masking in hardware the same technique > as before is applied to most operations, specifically composition using blend. > > Masked loads/stores are a special form of masked operation that require > additional care to ensure out-of-bounds access throw exceptions. The range > checking has not been fully optimized and will require further work. > > No API enhancements were required and only a few additional tests were needed. Paul Sandoz has updated the pull request incrementally with two additional commits since the last revision: - Merge pull request #1 from nsjian/JDK-8271515 Address AArch64 review comments from Nick. - Address review comments from Nick. - Changes: - all: https://git.openjdk.java.net/jdk/pull/5873/files - new: https://git.openjdk.java.net/jdk/pull/5873/files/9945ce8d..2919465a Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=5873&range=05 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=5873&range=04-05 Stats: 35 lines in 5 files changed: 3 ins; 2 del; 30 mod Patch: https://git.openjdk.java.net/jdk/pull/5873.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/5873/head:pull/5873 PR: https://git.openjdk.java.net/jdk/pull/5873
Re: RFR: 8269336: Malformed jdk.serialFilter incorrectly handled [v2]
On Tue, 19 Oct 2021 14:22:26 GMT, Jaikiran Pai wrote: >> Can I please get a review for this change which addresses >> https://bugs.openjdk.java.net/browse/JDK-8269336? >> >> As noted in that issue, this change will now propagate any exception that >> occurred during parsing and creation of the filter configured through the >> `jdk.serialFilter` system property. It will also continue to log those >> errors, like it previously did. >> >> A new jtreg test has been introduced to reproduce this issue and verify the >> fix. >> >> Given that invalid values for this system property will now start throwing >> exception, will this change need a CSR? > > Jaikiran Pai has updated the pull request incrementally with one additional > commit since the last revision: > > Roger's review suggestion - rethrow the RuntimeException instead of > wrapping in ExceptionInInitializerError Thank you Roger for the review. - PR: https://git.openjdk.java.net/jdk/pull/5988
Integrated: 8269336: Malformed jdk.serialFilter incorrectly handled
On Mon, 18 Oct 2021 13:44:56 GMT, Jaikiran Pai wrote: > Can I please get a review for this change which addresses > https://bugs.openjdk.java.net/browse/JDK-8269336? > > As noted in that issue, this change will now propagate any exception that > occurred during parsing and creation of the filter configured through the > `jdk.serialFilter` system property. It will also continue to log those > errors, like it previously did. > > A new jtreg test has been introduced to reproduce this issue and verify the > fix. > > Given that invalid values for this system property will now start throwing > exception, will this change need a CSR? This pull request has now been integrated. Changeset: 35e5bb5f Author:Jaikiran Pai URL: https://git.openjdk.java.net/jdk/commit/35e5bb5f59c01a1b07893780fa73f93c2abab653 Stats: 90 lines in 2 files changed: 89 ins; 0 del; 1 mod 8269336: Malformed jdk.serialFilter incorrectly handled Reviewed-by: rriggs - PR: https://git.openjdk.java.net/jdk/pull/5988
Re: RFR: 8275063: Implementation of Foreign Function & Memory API (Second incubator) [v8]
> This PR contains the API and implementation changes for JEP-419 [1]. A more > detailed description of such changes, to avoid repetitions during the review > process, is included as a separate comment. > > [1] - https://openjdk.java.net/jeps/419 Maurizio Cimadamore has updated the pull request incrementally with one additional commit since the last revision: Fix copyright header in TestArrayCopy - Changes: - all: https://git.openjdk.java.net/jdk/pull/5907/files - new: https://git.openjdk.java.net/jdk/pull/5907/files/52189683..414952ad Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=5907&range=07 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=5907&range=06-07 Stats: 16 lines in 1 file changed: 0 ins; 0 del; 16 mod Patch: https://git.openjdk.java.net/jdk/pull/5907.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/5907/head:pull/5907 PR: https://git.openjdk.java.net/jdk/pull/5907
RFR: 8275535: Retrying a failed authentication on multiple LDAP servers can lead to users blocked
I'd like to propose a fix for JDK-8275535. This fix reverts the behavior to the state previous to JDK-8160768, where an authentication failure stops from trying other LDAP servers with the same credentials [1]. After JDK-8160768 we have 2 possible loops to stop: the one that iterates over different URLs and the one that iterates over different endpoints (after a DNS query that returns multiple values). No test regressions observed in jdk/com/sun/jndi/ldap. -- [1] - https://hg.openjdk.java.net/jdk/jdk/rev/a609d549992a#l2.137 - Commit messages: - Initial commit for JDK-8275535. Changes: https://git.openjdk.java.net/jdk/pull/6043/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=6043&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8275535 Stats: 8 lines in 1 file changed: 8 ins; 0 del; 0 mod Patch: https://git.openjdk.java.net/jdk/pull/6043.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/6043/head:pull/6043 PR: https://git.openjdk.java.net/jdk/pull/6043
Re: RFR: 8269336: Malformed jdk.serialFilter incorrectly handled [v2]
On Tue, 19 Oct 2021 14:22:26 GMT, Jaikiran Pai wrote: >> Can I please get a review for this change which addresses >> https://bugs.openjdk.java.net/browse/JDK-8269336? >> >> As noted in that issue, this change will now propagate any exception that >> occurred during parsing and creation of the filter configured through the >> `jdk.serialFilter` system property. It will also continue to log those >> errors, like it previously did. >> >> A new jtreg test has been introduced to reproduce this issue and verify the >> fix. >> >> Given that invalid values for this system property will now start throwing >> exception, will this change need a CSR? > > Jaikiran Pai has updated the pull request incrementally with one additional > commit since the last revision: > > Roger's review suggestion - rethrow the RuntimeException instead of > wrapping in ExceptionInInitializerError LGTM - Marked as reviewed by rriggs (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/5988
Re: RFR: 8268595: java/io/Serializable/serialFilter/GlobalFilterTest.java#id1 failed in timeout
On Tue, 19 Oct 2021 13:00:01 GMT, Jaikiran Pai wrote: > The `GlobalFilterTest` has to 2 `@test` tags. One of them has failed with a > timeout as noted in https://bugs.openjdk.java.net/browse/JDK-8268595. The > timeout seems to have happened even after the tests had already completed > successfully. Like I note in the JBS comments of that issue, I suspect it > might have to do with the > "-XX:StartFlightRecording:name=DeserializationEvent,dumponexit=true" usage in > this test action. > > The commit in this PR removes that second `@test` altogether because (correct > me if I'm wrong) from what I understand, this test never enables the > DeserializationEvent which means there is no JFR events being captured for > deserialization in this test, nor does the test do any JFR events related > testing. So, I think this second `@test` is virtually a no-op when it comes > to the JFR testing. There's a separate `TestDeserializationEvent` which has a > comprehensive testing of the DeserializationEvent. @chegar Please comment on this change. The purpose of the second test group is not clear. Thanks - PR: https://git.openjdk.java.net/jdk/pull/6008
Re: RFR: 8244202: Implementation of JEP 418: Internet-Address Resolution SPI [v3]
On Sun, 17 Oct 2021 21:03:56 GMT, Mark Sheppard wrote: > getByName requires a hostname lookup and getByAdress requires (eventually - I > know the docs says there’s no reverse lookup) an address reverse lookup. > Thus, a logical mapping is getByName —> lookupHostname, and getByAddr —> > lookupAddress, but the opposite will occur getByName --> lookupAddresses and > getByAddress --> lookupHostname. Therfore I proffer maps neatly to rename > methods to resolveHostname and resolveAddress such that the mapping are > getByName --> resolveHostname and getByAddress --> resolveAddress. Thank you, Mark, for highlighting the naming contradiction in the `InetAddressResolver` interface method names. It can be solved by making these names symmetrical to `InetAddress` API. In 302264810725f86a4569161754f7b052c78fd826 `InetAddressResolver` method names have been updated to stress input parameters type, which brings them in sync with `InetAddress` API: - `InetAddressResolver.lookupAddresses` is renamed to `InetAddressResolver.lookupByName` - `InetAddressResolver.lookupHostName` is renamed to `InetAddressResolver.lookupByAddress` - PR: https://git.openjdk.java.net/jdk/pull/5822
Re: RFR: 8244202: Implementation of JEP 418: Internet-Address Resolution SPI [v5]
> This change implements a new service provider interface for host name and > address resolution, so that java.net.InetAddress API can make use of > resolvers other than the platform's built-in resolver. > > The following API classes are added to `java.net.spi` package to facilitate > this: > - `InetAddressResolverProvider` - abstract class defining a service, and is, > essentially, a factory for `InetAddressResolver` resolvers. > - `InetAddressResolverProvider.Configuration ` - an interface describing the > platform's built-in configuration for resolution operations that could be > used to bootstrap a resolver construction, or to implement partial delegation > of lookup operations. > - `InetAddressResolver` - an interface that defines methods for the > fundamental forward and reverse lookup operations. > - `InetAddressResolver.LookupPolicy` - a class whose instances describe the > characteristics of one forward lookup operation. > > More details in [JEP-418](https://openjdk.java.net/jeps/418). > > Testing: new and existing `tier1:tier3` tests Aleksei Efimov has updated the pull request incrementally with one additional commit since the last revision: Change InetAddressResolver method names - Changes: - all: https://git.openjdk.java.net/jdk/pull/5822/files - new: https://git.openjdk.java.net/jdk/pull/5822/files/ac0d2184..30226481 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=5822&range=04 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=5822&range=03-04 Stats: 33 lines in 9 files changed: 0 ins; 0 del; 33 mod Patch: https://git.openjdk.java.net/jdk/pull/5822.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/5822/head:pull/5822 PR: https://git.openjdk.java.net/jdk/pull/5822
Re: RFR: 8275063: Implementation of Foreign Function & Memory API (Second incubator) [v7]
> This PR contains the API and implementation changes for JEP-419 [1]. A more > detailed description of such changes, to avoid repetitions during the review > process, is included as a separate comment. > > [1] - https://openjdk.java.net/jeps/419 Maurizio Cimadamore has updated the pull request incrementally with one additional commit since the last revision: Fix failing microbenchmarks. Contributed by @FrauBoes (thanks!) - Changes: - all: https://git.openjdk.java.net/jdk/pull/5907/files - new: https://git.openjdk.java.net/jdk/pull/5907/files/a4db81fe..52189683 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=5907&range=06 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=5907&range=05-06 Stats: 39 lines in 9 files changed: 1 ins; 10 del; 28 mod Patch: https://git.openjdk.java.net/jdk/pull/5907.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/5907/head:pull/5907 PR: https://git.openjdk.java.net/jdk/pull/5907
Re: RFR: 8275512: Upgrade required version of jtreg to 6.1 [v2]
On Wed, 20 Oct 2021 09:28:30 GMT, Leslie Zhai wrote: >> Weijun Wang has updated the pull request incrementally with one additional >> commit since the last revision: >> >> upgrade the version in GHA config >> >> only in patch2: >> unchanged: > > Hi @wangweij > > But how to be suitable for jtreg version 6-dev+0? > > > Running tests using JTREG control variable > 'VM_OPTIONS=-XX:+UseZGC;TIMEOUT_FACTOR=20;VERBOSE=summary' > Test selection 'tier1', will run: > * jtreg:test/hotspot/jtreg:tier1 > * jtreg:test/jdk:tier1 > * jtreg:test/langtools:tier1 > * jtreg:test/jaxp:tier1 > * jtreg:test/lib-test:tier1 > > Running test 'jtreg:test/hotspot/jtreg:tier1' > Error: The testsuite at /var/lib/jenkins/repos/openjdk/jdk/test/hotspot/jtreg > requires jtreg version 6.1 b1 or higher and this is jtreg version 6-dev+0. > > > Thanks, > Leslie Zhai @xiangzhai The entire point of this PR is to *not* allow jtreg 6.0. You need to upgrade your local jtreg installation to 6.1. - PR: https://git.openjdk.java.net/jdk/pull/6012
Re: RFR: 8275512: Upgrade required version of jtreg to 6.1 [v2]
On Tue, 19 Oct 2021 17:24:17 GMT, Weijun Wang wrote: >> As a follow up of JEP 411, we will soon disallow security manager by >> default. jtreg 6.1 does not set its own security manager if JDK version is >> >= 18. > > Weijun Wang has updated the pull request incrementally with one additional > commit since the last revision: > > upgrade the version in GHA config > > only in patch2: > unchanged: Hi @wangweij But how to be suitable for jtreg version 6-dev+0? Running tests using JTREG control variable 'VM_OPTIONS=-XX:+UseZGC;TIMEOUT_FACTOR=20;VERBOSE=summary' Test selection 'tier1', will run: * jtreg:test/hotspot/jtreg:tier1 * jtreg:test/jdk:tier1 * jtreg:test/langtools:tier1 * jtreg:test/jaxp:tier1 * jtreg:test/lib-test:tier1 Running test 'jtreg:test/hotspot/jtreg:tier1' Error: The testsuite at /var/lib/jenkins/repos/openjdk/jdk/test/hotspot/jtreg requires jtreg version 6.1 b1 or higher and this is jtreg version 6-dev+0. Thanks, Leslie Zhai - PR: https://git.openjdk.java.net/jdk/pull/6012
