Caching local network info and using an event handler to invalidate as needed (improves menu slow down issue in FE3H)

[JumpmanSr]

Hello, this is my first PR to this project so hopefully I did what's needed to maintain a level of coding quality. I will spend a majority of this post explaining why I did what I did and why it resolves #2533 (At least partially)

First let's talk about FPS improvements:
Mainline: ~15-16 FPS

W/ this PR: (Hits 30 FPS easily)

Now lets talk changes:
There is only one function change, it is a rewrite of the GetCurrentIpAddress(ServiceCtx context) this is done to mitigate the time that the GetLocalInterface() takes because it has a nested loop and in the current mainline, the first return is immediately discarded so I figured a faster, more targeted method would help mitigate the issues presented in #2533 which it did. I use a method which involves using System.Net and the Dns.GetHostAddresses() function. Usually the length of this array is 2 (I added some code just incase to always grab the last index as that seems to always be IPV4), the first being your local IPV6 address (if applicable) and the second index being your IPV4 address. From my experience across multiple machines with different network configurations this method seems to work just fine because it targets local dns and the returned ip address is mapped to ipv4; possibly creating an artifact for people with network configurations I might not have access to, to test would be helpful.

Why this method?
The main reason is simple, any other fixes would've been too directed at one game in-which the issue isn't even emulation but a software issue, would most likely be considered "hacks", and because the issue is more of a game/software issue; only a speed improvement would be reasonable (in my opinion) to implement.
Also there was a slow-down associated with making the hacks toggleable options.

Closing Thoughts
I'm curious on any feedback any of the main contributors have on this and am open to any constructive criticism. Thank you for your time :)

[github-actions]

Download the artifacts for this pull request:

• ryujinx-headless-sdl2-Release-1.0.0+57ef779-linux_x64
• ryujinx-headless-sdl2-Release-1.0.0+57ef779-osx_x64
• ryujinx-headless-sdl2-Release-1.0.0+57ef779-win_x64
• ryujinx-Release-1.0.0+57ef779-linux_x64
• ryujinx-Release-1.0.0+57ef779-osx_x64
• ryujinx-Release-1.0.0+57ef779-win_x64

Only for Developers

• ryujinx-Debug-1.0.0+57ef779-linux_x64
• ryujinx-Debug-1.0.0+57ef779-osx_x64
• ryujinx-Debug-1.0.0+57ef779-win_x64
• ryujinx-headless-sdl2-Debug-1.0.0+57ef779-linux_x64
• ryujinx-headless-sdl2-Debug-1.0.0+57ef779-osx_x64
• ryujinx-headless-sdl2-Debug-1.0.0+57ef779-win_x64

[jduncanator]

How does this work when you have multiple network interfaces? For example, using the code in this PR on my Work machine, I get returned a Local IP of 192.168.1.31 which is the IP Address of my virtual Hyper-V interface, not the interface of my primary interface (for me, the index of my actual IP address is the second last one). This address doesn't have internet access, and so would clearly be the wrong thing to return to a game in an instance where it needed public internet access. I'm not sure if this will impact anything at the moment (LDN maybe?) but it certainly will going forward.

I mentioned this a couple of times in the original issue, but the slow performance of calling GetLocalInterface is likely just a side affect of another issue, not the issue in and of itself. I find it hard to believe this game calls GetCurrentIpAddress multiple times each frame intentionally, and so the actual cause of that should be investigated and resolved, rather than patching around the issue by making the call faster.

If the game does indeed call GetCurrentIpAddress multiple times each frame intentionally and this is unavoidable, then the proper solution would likely be caching the result of GetLocalInterface, and having a method to invalidate that cache when the interface configuration changes, rather than trying to determine the local IP address by doing a DNS resolution (which depending on configuration, could be even slower if the local hostname needs to be resolved against an external DNS server). You can use the NetworkAddressChanged event to listen for changes to network configuration, which could them simply set the cached interface information to null, triggering a call to GetLocalInterface the next time through the service call.

I think going down the path of caching the interface configuration is the best solution here, as I can imagine in the future we may wish to offer a Network Interface setting that lets users bind the emulator to a specific network interface (for LDN etc), and using this approach loses us control over the interface of which IP address we get returned.

[JumpmanSr]

How does this work when you have multiple network interfaces? For example, using the code in this PR on my Work machine, I get returned a Local IP of 192.168.1.31 which is the IP Address of my virtual Hyper-V interface, not the interface of my primary interface (for me, the index of my actual IP address is the second last one). This address doesn't have internet access, and so would clearly be the wrong thing to return to a game in an instance where it needed public internet access. I'm not sure if this will impact anything at the moment (LDN maybe?) but it certainly will going forward.

From my experience multiple network interfaces are fine, I've even tried insstalling tunneling networks like hamachi on top just to see and for now it's been fine. I think the actual mistake I made was hard-coding the index to grab, I didn't consider that possibility which was my fault but I added some code that will use a loop to go through the IPAddresses and get the first IPV4 address assigned to your local host. This still has a major performance improvement over the GetLocalInterface() method. One small improvement I also noticed is according to Dns.GetHostAddresses that by passing a empty string you can get the local host information only; saves calling the Dns.GetHostName() function. (I might need to add a loopback check here but from my testing it seemed like it was okay to not check that for now) If you would be kind enough to try and see if these resolve the issue for you, I'd greatly appreciate it because I don't have a Hyper-V setup.

I mentioned this a couple of times in the original issue, but the slow performance of calling GetLocalInterface is likely just a side affect of another issue, not the issue in and of itself. I find it hard to believe this game calls GetCurrentIpAddress multiple times each frame intentionally, and so the actual cause of that should be investigated and resolved, rather than patching around the issue by making the call faster.

I know you spoke about that a bit and from all the data I've been able to collect, I can't find any reason outside of the game just directly making this call that it would happen. I've tried disabling the nifm service (not good), returning no internet access upstream of nfim (did nothing), and I've even looked at the binary with a friend who is a cheat developer and he says he can see the game frequently makes the call but doesn't understand why. That being said, I think from profiling and the nested loop in GetLocalInterface() the performance of it is just a bit too slow as it can impact frame times for simply returning an IPAddress.

If the game does indeed call GetCurrentIpAddress multiple times each frame intentionally and this is unavoidable, then the proper solution would likely be caching the result of GetLocalInterface, and having a method to invalidate that cache when the interface configuration changes, rather than trying to determine the local IP address by doing a DNS resolution (which depending on configuration, could be even slower if the local hostname needs to be resolved against an external DNS server). You can use the NetworkAddressChanged event to listen for changes to network configuration, which could them simply set the cached interface information to null, triggering a call to GetLocalInterface the next time through the service call.
I think going down the path of caching the interface configuration is the best solution here, as I can imagine in the future we may wish to offer a Network Interface setting that lets users bind the emulator to a specific network interface (for LDN etc), and using this approach loses us control over the interface of which IP address we get returned.

While I agree caching is a viable alternative, unless making it an always on cache, it's implementation will have to be done in a specific way as to not impact performance; such as checking the config too frequently or having to set it up so that toggling that wouldn't require a restart (I'm speculating a bit since my experience with the configurations in ryujinx is a little limited). Regardless, imo is also not an ideal situation but if this method proves to be too problematic, I can give that a shot with that event handler.

[jduncanator]

I know you spoke about that a bit and from all the data I've been able to collect, I can't find any reason outside of the game just directly making this call that it would happen. I've tried disabling the nifm service (not good), returning no internet access upstream of nfim (did nothing), and I've even looked at the binary with a friend who is a cheat developer and he says he can see the game frequently makes the call but doesn't understand why. That being said, I think from profiling and the nested loop in GetLocalInterface() the performance of it is just a bit too slow as it can impact frame times for simply returning an IPAddress.

What I was suggesting here was that this specific code path could be triggered from a bug somewhere else in the HLE implementation in Ryujinx, not necessarily due to a specific response we're returning somewhere else in nifm.

The only way to check this would be to reverse engineer the game binary, identify the call that is happening multiple times per frame, and then look under what conditions the game is expecting to make that call. From there, you can back track through the application to determine what other calls it makes to get stuck in the state where it's making those calls multiple times per frame.

It's entirely possible that this is "intended" behaviour, and the only way to validate that would be to run the game on a hacked Switch and MITM the service calls.

I guess the point I was trying to make is that we should verify that this isn't caused by a bug somewhere else in Ryujinx first, before covering it up with a workaround like this, because if it is a bug somewhere else in Ryujinx, then all this PR serves to do is put it "out of sight, out of mind" and make it increasingly difficult to track down and resolve in the future.

While I agree caching is a viable alternative, unless making it an always on cache, it's implementation will have to be done in a specific way as to not impact performance; such as checking the config too frequently or having to set it up so that toggling that wouldn't require a restart (I'm speculating a bit since my experience with the configurations in ryujinx is a little limited). Regardless, imo is also not an ideal situation but if this method proves to be too problematic, I can give that a shot with that event handler.

You would set it up to cache always, yes, and then invalidate that cache when a networking configuration change occurs. I linked to the event you can listen to in .NET to act upon network configuration changes. This cache invalidation should be incredibly rare, and so I imagine that it will hit the cache effectively 100% of the time.

I wasn't suggesting you needed to create a setting/configuration option to let the user pick the network interface from the emulation Options, I was simply putting that forward as something that we may wish to do in the future, so it would be good to implement this in a way that would allow that. I feel like caching the interface information is the best way forward here.

Let me know if you want some specific implementation details and I can help guide you in the right direction for caching the interface configuration.

[jduncanator]

This could use String.Empty to show that passing an empty string here is deliberate, and not a mistake.

[jduncanator]

From my experience multiple network interfaces are fine, I've even tried insstalling tunneling networks like hamachi on top just to see and for now it's been fine. I think the actual mistake I made was hard-coding the index to grab, I didn't consider that possibility which was my fault but I added some code that will use a loop to go through the IPAddresses and get the first IPV4 address assigned to your local host. This still has a major performance improvement over the GetLocalInterface() method. One small improvement I also noticed is according to Dns.GetHostAddresses that by passing a empty string you can get the local host information only; saves calling the Dns.GetHostName() function. (I might need to add a loopback check here but from my testing it seemed like it was okay to not check that for now) If you would be kind enough to try and see if these resolve the issue for you, I'd greatly appreciate it because I don't have a Hyper-V setup.

So I did some more testing using your new variant, but it hits a new edge case now. Here's the result from my personal laptop for Dns.GetHostAddresses(""):

If we parse those IP addresses, we get the following:

• 172.26.112.1
• 192.168.5.84

Here Ryuijnx picks the first IPv4 address returned, and so it picks the 172.26.112.1 address. 172.26.112.1 belongs to my Hyper-V vSwitch again, which is obviously the wrong IP address.

I think the order in which the IP's come back from the underlying OS aren't sorted in any particular order, so without actually looking up the interfaces on the local machine, I'm not sure how you'd determine which is allocated to the default route interface.

[jduncanator]

Also, something to note is the proposed changes only supports IPv4-only or dual stack networking setups, if you're IPv6-only, this code will return a null IP address, as you're explicitly looking for IPv4 addresses.

Admittedly, the current implementation only looks for IPv4 supporting interfaces as well, so it's not a huge issue, but it would be good to get that fixed, as the Switch natively supports IPv6-only networks.

[JumpmanSr]

There was a lot to reply and chew threw, so I will reply to some parts and for the parts I missed, I did try to acknowledge as much as possible, the reply may have slipped through the cracks, but I did go through it all.

What I was suggesting here was that this specific code path could be triggered from a bug somewhere else in the HLE implementation in Ryujinx, not necessarily due to a specific response we're returning somewhere else in nifm.

The only way to check this would be to reverse engineer the game binary, identify the call that is happening multiple times per frame, and then look under what conditions the game is expecting to make that call. From there, you can back track through the application to determine what other calls it makes to get stuck in the state where it's making those calls multiple times per frame.

It's entirely possible that this is "intended" behaviour, and the only way to validate that would be to run the game on a hacked Switch and MITM the service calls.

I guess the point I was trying to make is that we should verify that this isn't caused by a bug somewhere else in Ryujinx first, before covering it up with a workaround like this, because if it is a bug somewhere else in Ryujinx, then all this PR serves to do is put it "out of sight, out of mind" and make it increasingly difficult to track down and resolve in the future.

I understand that, I'd say its a bit beyond my ability to MITM and even with the best effort that I could put in, I found nothing that really stuck out as to why. My cheat developer friend and I did our best with the tools at hand but we could not find the root cause.

You would set it up to cache always, yes, and then invalidate that cache when a networking configuration change occurs. I linked to the event you can listen to in .NET to act upon network configuration changes. This cache invalidation should be incredibly rare, and so I imagine that it will hit the cache effectively 100% of the time.

I wasn't suggesting you needed to create a setting/configuration option to let the user pick the network interface from the emulation Options, I was simply putting that forward as something that we may wish to do in the future, so it would be good to implement this in a way that would allow that. I feel like caching the interface information is the best way forward here.

Let me know if you want some specific implementation details and I can help guide you in the right direction for caching the interface configuration.

I went ahead and implemented a pretty lightweight caching solution using the NetworkAddressChanged event you posted and reverted the changes to GetCurrentIPAddress(), please feel free to look through my code and point out if I made any mistakes or missed any best practices. I did my best to look through the code base to maintain a bit of consistency with variable names and such but if you have any recommendations on that I am all ears. I did test it with the caching and the FPS gain is still there, IP is returned accurately, and only when network status changes (disconnect, manual or not, ie ipconfig /release or pulling ethernet) and the event handler works well, from my experience (fires once for disconnect and fires once per adapter when reconnecting, nulling the cache so GetLocalInterface() can function like it normally would to resolve the local IP; hopefully that should eliminate edge cases since it's using the original code with caching)

As for IPV6, I did notice that the current solution only returns IPV4, I think that would be a little bit outside of the scope of this fix but that is something I can look at in the future (truthfully I'd probably have to setup/emulate a IPV6 only network or vm or something for testing). Thank you for all your input!

[jduncanator]

Great, thanks for making that change! I'll take a look through it this afternoon or tomorrow, do some testing, and provide some feedback :)

[gdkchan]

lgtm, thanks.