Re: RFR: 8258422: Cleanup unnecessary null comparison before instanceof check in java.base [v4]
On Wed, 16 Dec 2020 10:32:12 GMT, Andrey Turbanov wrote: >> 8258422: Cleanup unnecessary null comparison before instanceof check in >> java.base > > Andrey Turbanov has updated the pull request incrementally with one > additional commit since the last revision: > > 8258422: Cleanup unnecessary null comparison before instanceof check in > java.base > take advantage of "flow scoping" to eliminate casts Looks good to me. - Marked as reviewed by chegar (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/20
Re: RFR: 8258422: Cleanup unnecessary null comparison before instanceof check in java.base [v4]
On Wed, 16 Dec 2020 10:32:12 GMT, Andrey Turbanov
wrote:
>> 8258422: Cleanup unnecessary null comparison before instanceof check in
>> java.base
>
> Andrey Turbanov has updated the pull request incrementally with one
> additional commit since the last revision:
>
> 8258422: Cleanup unnecessary null comparison before instanceof check in
> java.base
> take advantage of "flow scoping" to eliminate casts
src/java.base/share/classes/jdk/internal/misc/Signal.java line 102:
> 100: */
> 101: public boolean equals(Object other) {
> 102: if (this == other) {
It might be a bit cleaner to rename Object other to "obj" to avoid having
Object other and Signal other1.
-
PR: https://git.openjdk.java.net/jdk/pull/20
Re: RFR: 8258422: Cleanup unnecessary null comparison before instanceof check in java.base [v4]
On Thu, 17 Dec 2020 10:29:50 GMT, Alan Bateman wrote:
>> Andrey Turbanov has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> 8258422: Cleanup unnecessary null comparison before instanceof check in
>> java.base
>> take advantage of "flow scoping" to eliminate casts
>
> src/java.base/share/classes/jdk/internal/misc/Signal.java line 102:
>
>> 100: */
>> 101: public boolean equals(Object other) {
>> 102: if (this == other) {
>
> It might be a bit cleaner to rename Object other to "obj" to avoid having
> Object other and Signal other1.
Actually, I'm not sure if `oth` is better name for variable than `other1`.
I would say they have the same rank :)
-
PR: https://git.openjdk.java.net/jdk/pull/20
Re: RFR: 8258422: Cleanup unnecessary null comparison before instanceof check in java.base [v4]
On Wed, 16 Dec 2020 10:32:12 GMT, Andrey Turbanov wrote: >> 8258422: Cleanup unnecessary null comparison before instanceof check in >> java.base > > Andrey Turbanov has updated the pull request incrementally with one > additional commit since the last revision: > > 8258422: Cleanup unnecessary null comparison before instanceof check in > java.base > take advantage of "flow scoping" to eliminate casts Thank you for checking `HttpURLConnection` and `Socket`. The latest version looks good to me. - Marked as reviewed by aefimov (Committer). PR: https://git.openjdk.java.net/jdk/pull/20
Re: RFR: 8258422: Cleanup unnecessary null comparison before instanceof check in java.base [v4]
On Thu, 17 Dec 2020 11:35:26 GMT, Andrey Turbanov
wrote:
>> src/java.base/share/classes/jdk/internal/misc/Signal.java line 102:
>>
>>> 100: */
>>> 101: public boolean equals(Object other) {
>>> 102: if (this == other) {
>>
>> It might be a bit cleaner to rename Object other to "obj" to avoid having
>> Object other and Signal other1.
>
> Actually, I'm not sure if `oth` is better name for variable than `other1`.
> I would say they have the same rank :)
I believe Alan is suggesting to do:
/**
* Compares the equality of two Signal objects.
*
* @param obj the object to compare with.
* @return whether two Signal objects are equal.
*/
public boolean equals(Object obj) {
if (this == obj) {
this leaves the variable name `other` free for later use inside the method.
-
PR: https://git.openjdk.java.net/jdk/pull/20
Re: RFR: 8258422: Cleanup unnecessary null comparison before instanceof check in java.base [v4]
On Thu, 17 Dec 2020 13:32:06 GMT, Daniel Fuchs wrote:
>> Actually, I'm not sure if `oth` is better name for variable than `other1`.
>> I would say they have the same rank :)
>
> I believe Alan is suggesting to do:
>
> /**
> * Compares the equality of two Signal objects.
> *
> * @param obj the object to compare with.
> * @return whether two Signal objects are equal.
> */
> public boolean equals(Object obj) {
> if (this == obj) {
>
> this leaves the variable name `other` free for later use inside the method.
> Actually, I'm not sure if `oth` is better name for variable than `other1`.
> I would say they have the same rank :)
Sorry, I should have been clearer, the comment was about equals(Object other).
If you rename "other" then it will avoid "other1"
-
PR: https://git.openjdk.java.net/jdk/pull/20
Re: [JDK-8257080] Java does not try all DNS results when opening a socket
Hi Simone, We are investigating introducing a Service Provider interface which would allow an application to replace the default built-in implementation that blocks inside the kernel. There is no plan to introduce any public asynchronous API to perform address resolution at this point. With the advent of Loom and virtual threads I'm not sure there would be much motivation for that anyway. best regards, -- daniel On 16/12/2020 19:59, Simone Bordet wrote: Hi, On Wed, Dec 16, 2020 at 5:55 PM Aleks Efimov wrote: Hi Benjamin, As Alan stated I'm working on adding an SPI [1] which will provide a possibility to alter how host names and IP addresses are resolved by JDK platform. I believe it would be possible to use this mechanism for addressing issue described in JDK-8257080. Is it hopefully going to be non-blocking?
Re: RFR: 8258422: Cleanup unnecessary null comparison before instanceof check in java.base [v5]
> 8258422: Cleanup unnecessary null comparison before instanceof check in > java.base Andrey Turbanov has updated the pull request incrementally with one additional commit since the last revision: 8258422: Cleanup unnecessary null comparison before instanceof check in java.base - Changes: - all: https://git.openjdk.java.net/jdk/pull/20/files - new: https://git.openjdk.java.net/jdk/pull/20/files/f5727ca9..314217ec Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=20&range=04 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=20&range=03-04 Stats: 4 lines in 1 file changed: 0 ins; 0 del; 4 mod Patch: https://git.openjdk.java.net/jdk/pull/20.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/20/head:pull/20 PR: https://git.openjdk.java.net/jdk/pull/20
Re: RFR: 8258422: Cleanup unnecessary null comparison before instanceof check in java.base [v5]
On Thu, 17 Dec 2020 14:01:14 GMT, Andrey Turbanov
wrote:
>> 8258422: Cleanup unnecessary null comparison before instanceof check in
>> java.base
>
> Andrey Turbanov has updated the pull request incrementally with one
> additional commit since the last revision:
>
> 8258422: Cleanup unnecessary null comparison before instanceof check in
> java.base
src/java.base/share/classes/jdk/internal/misc/Signal.java line 101:
> 99: * @return whether two Signal objects are equal.
> 100: */
> 101: public boolean equals(Object oth) {
I'd suggest to replace `oth` with `obj` and fix the `@param` clause in the
method javadoc.
-
PR: https://git.openjdk.java.net/jdk/pull/20
Re: [JDK-8257080] Java does not try all DNS results when opening a socket
Looping in a prior relevant issue in JIRA: https://bugs.openjdk.java.net/browse/JDK-8170568 - Improve address selection for network clients -Chris. > On 17 Dec 2020, at 13:45, Daniel Fuchs wrote: > > Hi Simone, > > We are investigating introducing a Service Provider interface > which would allow an application to replace the default > built-in implementation that blocks inside the kernel. > > There is no plan to introduce any public asynchronous API to perform > address resolution at this point. With the advent of Loom and > virtual threads I'm not sure there would be much motivation for > that anyway. > > best regards, > > -- daniel > > On 16/12/2020 19:59, Simone Bordet wrote: >> Hi, >> On Wed, Dec 16, 2020 at 5:55 PM Aleks Efimov >> wrote: >>> >>> Hi Benjamin, >>> >>> As Alan stated I'm working on adding an SPI [1] which will provide a >>> possibility to alter how host names and IP addresses are resolved by JDK >>> platform. >>> I believe it would be possible to use this mechanism for addressing >>> issue described in JDK-8257080. >> Is it hopefully going to be non-blocking? >
RFR: 8258582: HttpClient: the HttpClient doesn't explicitely shutdown its default executor when stopping.
Hi, Please find an almost trivial fix for: 8258582: HttpClient: the HttpClient doesn't explicitely shutdown its default executor when stopping. The HttpClient should shutdown his executor when stopping, when the executor was created by the HttpClient itself. best regards, -- daniel - Commit messages: - 8258582: HttpClient: the HttpClient doesn't explicitely shutdown its default executor when stopping. Changes: https://git.openjdk.java.net/jdk/pull/1822/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=1822&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8258582 Stats: 16 lines in 2 files changed: 16 ins; 0 del; 0 mod Patch: https://git.openjdk.java.net/jdk/pull/1822.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/1822/head:pull/1822 PR: https://git.openjdk.java.net/jdk/pull/1822
Re: RFR: JDK-8212035 : merge jdk.test.lib.util.SimpleHttpServer with jaxp.library.SimpleHttpServer [v4]
On Mon, 14 Dec 2020 19:44:59 GMT, Daniel Fuchs wrote:
>> Mahendra Chhipa has updated the pull request incrementally with two
>> additional commits since the last revision:
>>
>> - Merge branch 'JDK-8212035' of https://github.com/mahendrachhipa/jdk into
>> JDK-8212035
>> - Implemnted the review comments.
>
> test/lib/jdk/test/lib/net/SimpleHttpServer.java line 105:
>
>> 103: Path fPath;
>> 104: try {
>> 105: fPath = Paths.get(docRoot, path);
>
> Hi Mahendra,
>
> I am not comfortable with this. You cannot just concatenate a file system
> path with a URI path in the general case (think windows file system). I would
> suggest to transform docRoot into a root URI using
> `Path.of(docRoot).toURI().normalize()` - then you can probably concatenate
> the string representation of this root URI with the *raw path* extracted from
> the request URI, normalize that again (to remove potential double slashes)
> and convert that back into a Path using Path.of(URI).
> Some additional conditions might need to be asserted:
> - `t.getRequestURI().getRawPath()` should either be empty or start with
> `"/"` (and even that may be a bit lax)
> - the resulting URI you obtain (after normalization and before converting
> back to Path) should either be equal to the root URI or start with the root
> URI.
> `(rootURI.toString().equals(uri.toString()) ||
> uri.toString().startsWith(rootUri.toString() + "/") )`
>
> best regards,
>
> -- daniel
Thanks Daniel. I have implemented the review comments.
-
PR: https://git.openjdk.java.net/jdk/pull/1632
Re: RFR: JDK-8212035 : merge jdk.test.lib.util.SimpleHttpServer with jaxp.library.SimpleHttpServer [v5]
> jaxp.library.SimpleHttpServer > https://bugs.openjdk.java.net/browse/JDK-8212035 Mahendra Chhipa has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains seven commits: - Implemented the Review comments. - Implemented the review comments - Merge branch 'JDK-8212035' of https://github.com/mahendrachhipa/jdk into JDK-8212035 - Implemnted the review comments. - Implemnted the review comments. - Implemented the review comments. - JDK-8212035 merge jdk.test.lib.util.SimpleHttpServer with jaxp.library.SimpleHttpServer https://bugs.openjdk.java.net/browse/JDK-8212035 - Changes: https://git.openjdk.java.net/jdk/pull/1632/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=1632&range=04 Stats: 650 lines in 17 files changed: 226 ins; 321 del; 103 mod Patch: https://git.openjdk.java.net/jdk/pull/1632.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/1632/head:pull/1632 PR: https://git.openjdk.java.net/jdk/pull/1632
Re: RFR: 8258582: HttpClient: the HttpClient doesn't explicitly shutdown its default executor when stopping.
On Thu, 17 Dec 2020 14:51:44 GMT, Daniel Fuchs wrote: > Hi, > > Please find an almost trivial fix for: > 8258582: HttpClient: the HttpClient doesn't explicitly shutdown its default > executor when stopping. > > The HttpClient should shutdown his executor when stopping, when the executor > was created by the HttpClient itself. > > best regards, > > -- daniel Marked as reviewed by chegar (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/1822
Re: RFR: JDK-8212035 : merge jdk.test.lib.util.SimpleHttpServer with jaxp.library.SimpleHttpServer [v5]
On Thu, 17 Dec 2020 16:24:10 GMT, Mahendra Chhipa
wrote:
>> jaxp.library.SimpleHttpServer
>> https://bugs.openjdk.java.net/browse/JDK-8212035
>
> Mahendra Chhipa has updated the pull request with a new target base due to a
> merge or a rebase. The pull request now contains seven commits:
>
> - Implemented the Review comments.
> - Implemented the review comments
> - Merge branch 'JDK-8212035' of https://github.com/mahendrachhipa/jdk into
> JDK-8212035
> - Implemnted the review comments.
> - Implemnted the review comments.
> - Implemented the review comments.
> - JDK-8212035 merge jdk.test.lib.util.SimpleHttpServer with
>jaxp.library.SimpleHttpServer
>https://bugs.openjdk.java.net/browse/JDK-8212035
test/lib/jdk/test/lib/net/SimpleHttpServer.java line 104:
> 102: try {
> 103: uri = URI.create(rootUri.getRawPath() +
> path).normalize();
> 104: fPath = Path.of(uri.getRawPath());
This is wrong. It should be `fpath = Path.of(uri);`. See
https://docs.oracle.com/en/java/javase/15/docs/api/java.base/java/nio/file/Path.html#of(java.net.URI)
-
PR: https://git.openjdk.java.net/jdk/pull/1632
Re: RFR: 8258422: Cleanup unnecessary null comparison before instanceof check in java.base [v3]
> On Dec 16, 2020, at 1:47 AM, Chris Hegarty wrote:
>
> On Wed, 16 Dec 2020 09:20:09 GMT, Andrey Turbanov
> wrote:
>
>>> 8258422: Cleanup unnecessary null comparison before instanceof check in
>>> java.base
>>
>> Andrey Turbanov has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> 8258422: Cleanup unnecessary null comparison before instanceof check in
>> java.base
>> use instanceof pattern matching in UnixPath too
>
> Let's take advantage of "flow scoping" to eliminate some of these casts. A
> few examples follow.
>
> src/java.base/share/classes/java/net/InetSocketAddress.java line 414:
>
>> 412: if (!(obj instanceof InetSocketAddress))
>> 413: return false;
>> 414: return holder.equals(((InetSocketAddress) obj).holder);
>
> If we restructure this a little we can get:
>
>public final boolean equals(Object obj) {
>if (obj instanceof InetSocketAddress that)
>return holder.equals(that.holder);
>return false;
>}
>
It’s also possible to use a ternary expression as in:
return (obj instanceof InetSocketAddress that) ? holder.equals(that.holder)
: false;
Not suggesting you have to do that here. More for information purposes for
those that might not know.
Paul.
Re: RFR: 8258422: Cleanup unnecessary null comparison before instanceof check in java.base [v6]
> 8258422: Cleanup unnecessary null comparison before instanceof check in > java.base Andrey Turbanov has updated the pull request incrementally with one additional commit since the last revision: 8258422: Cleanup unnecessary null comparison before instanceof check in java.base rename 'other' -> 'oth', 'other1' -> 'other' - Changes: - all: https://git.openjdk.java.net/jdk/pull/20/files - new: https://git.openjdk.java.net/jdk/pull/20/files/314217ec..fe303a2c Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=20&range=05 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=20&range=04-05 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/20.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/20/head:pull/20 PR: https://git.openjdk.java.net/jdk/pull/20
Re: RFR: 8258422: Cleanup unnecessary null comparison before instanceof check in java.base [v7]
> 8258422: Cleanup unnecessary null comparison before instanceof check in > java.base Andrey Turbanov has updated the pull request incrementally with one additional commit since the last revision: 8258422: Cleanup unnecessary null comparison before instanceof check in java.base rename 'oth' -> 'obj' - Changes: - all: https://git.openjdk.java.net/jdk/pull/20/files - new: https://git.openjdk.java.net/jdk/pull/20/files/fe303a2c..5949f9a8 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=20&range=06 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=20&range=05-06 Stats: 4 lines in 1 file changed: 0 ins; 0 del; 4 mod Patch: https://git.openjdk.java.net/jdk/pull/20.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/20/head:pull/20 PR: https://git.openjdk.java.net/jdk/pull/20
Re: RFR: 8258582: HttpClient: the HttpClient doesn't explicitly shutdown its default executor when stopping.
On Thu, 17 Dec 2020 14:51:44 GMT, Daniel Fuchs wrote: > Hi, > > Please find an almost trivial fix for: > 8258582: HttpClient: the HttpClient doesn't explicitly shutdown its default > executor when stopping. > > The HttpClient should shutdown his executor when stopping, when the executor > was created by the HttpClient itself. > > best regards, > > -- daniel Marked as reviewed by michaelm (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/1822
Integrated: 8258582: HttpClient: the HttpClient doesn't explicitly shutdown its default executor when stopping.
On Thu, 17 Dec 2020 14:51:44 GMT, Daniel Fuchs wrote: > Hi, > > Please find an almost trivial fix for: > 8258582: HttpClient: the HttpClient doesn't explicitly shutdown its default > executor when stopping. > > The HttpClient should shutdown his executor when stopping, when the executor > was created by the HttpClient itself. > > best regards, > > -- daniel This pull request has now been integrated. Changeset: 3f77a600 Author:Daniel Fuchs URL: https://git.openjdk.java.net/jdk/commit/3f77a600 Stats: 16 lines in 2 files changed: 16 ins; 0 del; 0 mod 8258582: HttpClient: the HttpClient doesn't explicitly shutdown its default executor when stopping. Reviewed-by: chegar, michaelm - PR: https://git.openjdk.java.net/jdk/pull/1822
