Performance of NetworkInterface methods on Windows

Wed Feb 15 19:59:27 UTC 2023

I've pushed a branch that rewrites the native code as per the last email. You can find it here: https://github.com/rdicroce/jdk/tree/windows-ni-rewrite

In the process of doing this work, I discovered a few things that are broken in the existing code:

1) The calls to GetAdaptersAddresses do not include the GAA_FLAG_INCLUDE_ALL_INTERFACES flag. That is a problem because filters (and maybe other things, I didn't look too thoroughly and the docs aren't clear) are not included in the result unless that flag is set. But when enumerating interfaces, the XP code begins by calling the non-XP code, which DOES enumerate all those filters. Therefore, if you call isUp/getMTU/supportsMulticast/etc on any of those filters and the XP code is in use, you get bogus results because the XP code can't find the interface.

2) The NoMulticast flag in GetAdaptersAddresses doesn't seem to be set under any conditions, at least not on Windows 11. When I disabled multicast on a Win 11 VM, attempting to join a multicast group failed, as it should. But GetAdaptersAddresses still didn't set the NoMulticast flag.

#1 is pretty bad and easily fixed, so it might be worth backporting a narrow fix for JDK 11/17.

Now, on to my proposed changes. Most of the new code is straightforward, except for the new implementation of supportsMulticast. I left a giant comment explaining all the things I tried that didn't work there. On my machine, the new code produces the same results as the existing code, except for the following:

A) isP2P sometimes returns different results. But the new code leans on the OS to determine the access type, instead of looking at the interface type and trying to guess from there. So the new code should be more accurate. Currently, the code only returns true if the access type is NET_IF_ACCESS_POINT_TO_POINT. I'm not sure if it should also return true for NET_IF_ACCESS_POINT_TO_MULTI_POINT. Thoughts?

B) The interface name is different, as mentioned in the previous email. For example, on my machine, "wlan0" is now "wireless_32769". Since nobody had any opinions about backwards compatibility, I didn't add any. IMO this change should just be mentioned in the release notes.

C) On my machine, most methods are now several orders of magnitude faster. One notable exception is supportsMulticast, which is several times slower. But as explained in the comments, I don't see a better solution, other than just saying "forget it" and always returning true because it's highly unlikely that multicast is disabled.

So please take a look at the changes and let me know what you think.

Thanks,

Rich DiCroce
Senior Advanced Solutions Architect

Scientific Games

HAVE FUN. DO GOOD. PLAY HEALTHY.
 May be privileged. May be confidential. Please delete if not the addressee.

-----Original Message-----
From: net-dev <net-dev-retn at openjdk.org> On Behalf Of DiCroce, Richard
Sent: Wednesday, February 8, 2023 11:27 AM
To: net-dev at openjdk.org
Subject: RE: Performance of NetworkInterface methods on Windows

WARNING: This is an external email. Do not click links or open attachments unless you recognize the sender and know the content is safe.

Okay, so based on the various replies I've gotten, it sounds like we want a patch that:

1) Completely rewrites the native code using newer APIs, and gets rid of the dual implementations that exist today (per Daniel Jelinski)
2) Gets rid of the homebrew interface naming scheme in favor of what the Windows API returns (per Alan Bateman)

Do we care about backwards compatibility for NetworkInterface#getByIndex() and getByName()? Based on my experiments, I don't think the indexes will actually change, but I can't 100% guarantee that. The names will definitely change though, per #2 above. How do we want to handle that? Do we just put something in the release notes saying there's a change that's not backwards-compatible? Or do we want to do a fallback to the existing naming scheme if the Windows API doesn't recognize a name? Or something else entirely, like a system property to toggle the behavior?

Rich DiCroce
Senior Advanced Solutions Architect

Scientific Games

HAVE FUN. DO GOOD. PLAY HEALTHY.
 May be privileged. May be confidential. Please delete if not the addressee.

-----Original Message-----
From: Daniel Fuchs <daniel.fuchs at oracle.com>
Sent: Wednesday, February 8, 2023 7:44 AM
To: DiCroce, Richard <Rich.DiCroce at scientificgames.com>; net-dev at openjdk.org
Subject: Re: Performance of NetworkInterface methods on Windows

WARNING: This is an external email. Do not click links or open attachments unless you recognize the sender and know the content is safe.

Hi Richard,

On 07/02/2023 21:04, DiCroce, Richard wrote:
> Hi Daniel,
> I'm not proposing to add any caching, or change anything about the Java side of NetworkInterface, for exactly the reasons you state. This first patch is purely a change to the native code to make it so that getAll only calls the Windows API once instead of N + 1 times.

Oh, OK - good to hear. That sounds interesting then, sorry for assuming!
:-)

> createNetworkInterface actually already had parameters for passing the previously fetched list of interfaces. getAll just doesn't currently do that.
>
> This would probably be easier to discuss if you could actually see the changes I'm proposing. Should I attach a patch to an email, or is there a better way of doing that?

Jaikiran replied with a good suggestion for that. But please take into account Daniel Jelinski's comments first.

best regards,

-- daniel

>
> Thanks,
>
> Rich DiCroce