RFR(s): Improving performance of Windows socket connect on the loopback adapter

[Bernd Eckenfels]

Hello,

I am unsure about the signatures, s is of type SOCKET, why not keep this — I think I missed why this would be a JNICALL convention.

There are two inconsistencies:

The header file and implementation uses (int) argument, the call casts to (jint).

And I also think it's not a big problem to not check so_rv, but it should be done the same in both places?

Gruss
Bernd
--
http://bernd.eckenfels.net
________________________________
Von: net-dev <net-dev-retn at openjdk.java.net> im Auftrag von Alan Bateman <Alan.Bateman at oracle.com>
Gesendet: Saturday, July 18, 2020 6:56:29 PM
An: Nikola Grcevski <Nikola.Grcevski at microsoft.com>; net-dev at openjdk.java.net <net-dev at openjdk.java.net>
Betreff: Re: RFR(s): Improving performance of Windows socket connect on the loopback adapter

On 17/07/2020 02:10, Nikola Grcevski wrote:
> Thanks for reviewing the patch Alan.
>
> I applied your suggestions in this new webrev here:
>
> http://cr.openjdk.java.net/~adityam/nikola/fast_connect_loopback_1/
>
> I renamed the macro to IN6_IS_ADDR_MAPPED_IP4_LOOPBACK, following how IN6_IS_ADDR_ANY
> is named in the same file. I hope the new name works, but if there's something more suitable I
> don't mind changing it again.
>
The standard macro is IN6_IS_ADDR_V4MAPPED so it could be more
consistent to use that as the prefix and _LOOPBACK. It's only a minor
point of course.

There's a small inconsistency in connect0 for the case that getsockopt
fails. The check at L247 tests if the type is SOCK_STREAM whereas the
check at L261 also checks if so_rv is 0. Since you initialize the type
to 0 then both approaches are fine, just pick one. My preference is to
drop comment lines L242-243, leave the comment that the default on
Windows is 2s and can be shorted when it's the loopback of course.

Otherwise looks good to me although I am a bit surprised that the
implementation of ECN would impact connections through the loopback like
this. I assume that everything that could potentially connect through
the loopback would want to reduce the timeout so that it can report an
error quickly.

-Alan


-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.java.net/pipermail/net-dev/attachments/20200718/120d7b25/attachment.htm>