Server ping optimizations are somewhat problematic

[Anuken]

So I saw that this repo appeared to have a fix for the issue of every single server ping occurring on a different thread ("no longer creating a billion threads").

When trying to implement this in Mindustry, the roadblock I ran into was that, while the actual pinging of servers can be done on one thread via NIO, the DNS resolution cannot be done asynchronously without creating a new thread or using external libraries.

I checked your implementation, and it didn't seem like this issue was addressed: You construct a InetSocketAddress in the main thread, which attempts to resolve the hostname, which blocks... and sure enough, with this mod, opening the join dialog makes the game freeze for a solid 2-3 seconds. Refreshing immediately after that was instant, but after a couple seconds, the Java DNS cache gets invalidated again (?), leading to another (shorter) freeze when the list is refreshed.

In contrast, the vanilla Mindustry implementation lags badly for about a second on my devices as the (massive) thread pool is built up, but runs smoothly enough with subsequent refreshes. This is probably awful as far as memory is concerned, but still.... at least it's noticeably faster for the user.

Do you have any thoughts on this? Is it something you are planning to address in the future?

[Redstonneur1256]

I originally made this implementation because the Mindustry's default one was actually causing my game to lag more than this custom one, I very rarely got a freeze with it and previous tests that I made were showing that allocating the labels was causing the majority of the lag (apparently due to OpenGL calls) so I tough the issue wasn't coming from the mod and didn't really bother about it.

You pointed that it is not possible to resolve the domains asynchronously, however this is already done with the srv resolution and the code could be reused for the A records effectively making it single-threaded and fixing the issues.

I am currently working on a fix and will commit it as soon as the issue is fully fixed.

Edit: This optimization was actually introduced in the v6 or Mindustry where the pool allowing the ping of the servers was actually limited to the maximum between 6 and the number of CPU cores which would actually slow down the pings by quite a lot, it has been updated to support the new SRV records which lead to me accidentally placing the address resolution in the main thread.

[Anuken]

I very rarely got a freeze with it

This is very odd, as some individual domains took more than a second to resolve on my machine.

You pointed that it is not possible to resolve the domains asynchronously, however this is already done with the srv resolution and the code could be reused for the A records effectively making it single-threaded and fixing the issues.

Well, if you consider "doing it all yourself", yes, but the ArcDns list of nameservers seemed kinda sketchy to me, and I did not want to rely on it (or untested custom DNS implementations). It's worth a try, but I am almost certain that certain users will experience issues with it.

Unrelated: More often than not, the mod would fail to load with the following error, which made testing difficult:

[Redstonneur1256]

I have most of the server list hidden except for a few servers but even when revealing the list the lag was comparable to the default one without the mod. For now the custom DNS implementation worked fine, at least for me, I don't think much players are using the mod to this day. The crash is also very odd considering I have the exact same implementation than Arc's settings class with the only change being read(byte[]) being replaced by readFully(byte[]).

[Anuken]

Maybe it's a Linux thing, but I can reproduce it fairly consistently.

There was also a bug that involved the mod invoking UI#showException before UI was initialized, leading to a crash 100% of the time, that I had to manually fix before testing the mod. It occurred on both Linux and Windows.

[Redstonneur1256]

The only place calling UI#showException is in the LauncherInitializer class meaning that for it to be reached the mod failed to launch at least once, can you provide more details about the crash of the mod if possible ?