Use async IO to avoid blocking main thread

[Macil]

This mod would call the WebSocketBehavior.Broadcast method on the main game thread, which did synchronous blocking IO and could stall the game for an arbitrary amount of time, especially if there were multiple connections to the websocket. I've measured the broadcast call sometimes taking 1-15 ms. This PR changes it so that asynchronous IO is used to write to the websocket, so that the game's main thread doesn't ever get stalled.

This PR also fixes a thread-safety issue: when websockets first connected, the OnOpen methods would unsafely read from objects that were written to from the main game thread. The code now uses UnityMainThreadTaskScheduler.Factory.StartNew(() => ...).Result to evaluate the expression reading the game info on the main game thread.

This PR fixes an issue where connections to /BSDataPuller/LiveData were always immediately closed, causing the client to have to constantly reopen the connection. There had been an unnecessary call to CloseSession left in. (I wonder if that call was present to work around a bug caused by the thread-safety issue I've fixed now.)

This PR also improves the code so that it doesn't need to connect to its own websockets on startup. (It seems like the code did this to work around the fact that it didn't null-check the event property.2)

This PR is very similar to my PR https://github.com/opl-/beatsaber-http-status/pull/60, which fixes the same set of issues that existed coincidentally in the HTTP Status mod.

[ReadieFur]

Hi there, I read through the commit changes and the link you posted for the HTTP status mod. I had fixed the null checking in another project I was working on but never thought to update this one. Anyway the changes seem good, I won't release it straight away and I'll probably tweak a few things here and there with it (naming etc) and I'll update the contributors.md file to include your proposed changes.

[ReadieFur]

@Macil I see that in the server.cs you get the JsonData on a new thread UnityMainThreadTaskScheduler.Factory.StartNew(() => new MapData.JsonData()).Result;. Would it not be best to do the same for the Send function? And what about the JSON serilization, that still appears to be running on the main thread, would that benifit from being done on another thread?

[Macil]

The QueuedSend function is written to be thread-safe so that it can be called from any thread. I've added a comment to it to mark this. (It does its reads and writes, of this.readyToWrite and calls to SendAsync, in a thread-safe manner. If this weren't true, then yes you would want to make sure that QueuedSend was always called from the same thread such as the main thread.)

For thread-safety purposes, it doesn't matter where the JSON serialization happens since it only operates on the data we pass in. For performance purposes, we could block the main game thread less by moving the JSON serialization out of it, but that could complicate the code for possibly little benefit as the JSON serialization is very cheap compared to the synchronous IO calls that used to exist here. (Also there are some small trade-offs between how much we block the main thread, the latency of us getting messages into the websocket, and the total work we do between all threads. I don't think there's an obviously best option unless the JSON serialization was measured to take a non-negligible amount of time.)

[ReadieFur]

Ah I see, thanks for the description. One more thing, the peformance change here is probably very minor but youve changed the websocketserver to send to each client individually by adding to the event for every client that joins, would broadcasting the data be slightly better, it would just mean that the event isnt dispatched as much. Again extremely minor.

[Macil]

The Broadcast method basically just did a for-loop over calling Send to each open connection. (There was a very minor benefit that it would do utf-8 encoding of the message once, but that part is even more negligible than the JSON serialization.) The difference between Send(Async) and Broadcast(Async) is negligible, except that if BroadcastAsync were used in this PR, then if there were multiple websocket connections, then all of the connections would be held back to the speed of the slowest connection, because each BroadcastAsync call would wait until sends to all connections are complete before sending the next message to any sockets.

I just found a thread-safety issue in this (technically one that existed already). The MapData.Modifiers and MapData.PracticeModeModifiers values are mutated from the main thread but read on a websocket thread in MapDataServer.OnOpen. I'll push a fix for this in a minute.

[Macil]

I fixed that thread-safety issue now. I found it when I was reinvestigating the path of how JSON serialization happened after you mentioned it; I hadn't realized at first that the JsonData object was mutable. Now each instance is kept immutable and unchanging.

I feel pretty confident about this PR now. I've run it through a couple test runs with overlays running.