perf: Take WM_COPYDATA off the UI thread and out of the WndProc - #32
Merged
Merged
Conversation
Games send the manager WM_COPYDATA with a bare blocking SendMessageW, so a game's thread stalls for exactly as long as our WndProc takes, and the whole fleet queues on one pump. Two things made that expensive. The window was created on the main thread, which then ran Application.Run -- so every game was queueing behind WinForms painting, the titlebar drag loop and WebView2, on top of each other. It now lives on its own pump thread that does nothing else. That also gives headless mode a real pump: it had none, and worked only because Main is [STAThread] and a managed blocking wait on an STA thread happens to dispatch inter-thread sent messages. And the WndProc itself decoded UTF-8, scanned the whole payload for "heartBeat" and ran a full JsonSerializer.Deserialize of an envelope whose one argument is the entire escaped characterState snapshot -- all O(payload), all with N games waiting. It now stamps the heartbeat or copies the bytes into a pooled buffer and returns; the consumer decodes and parses. The copy is the only part that has to happen there, since the COPYDATASTRUCT buffer dies with the call. Together that is what made the per-second capture hitch scale with instance count (ResurrectedTrader/d2bsng#11). Moving the pump off the main thread means its death is no longer the process's death, so an unexpected exit now clears Handle and stops the application: a dead pump behind a live HWND blocks every game forever on its next send, heartbeat included, so the watchdog would restart each game into the same wedge while the manager kept serving the UI and looked healthy. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CJNhrx7ZamSEaeREus4aKS
It is already the default, but the previous commit's whole premise rests on it: true would run the waiting consumer's continuation inline on TryWrite, putting the parse and the SQLite capture ingest back on the pump thread with the sending game blocked in SendMessageW for all of it. Stated explicitly so that flipping it reads as the decision it would be. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CJNhrx7ZamSEaeREus4aKS
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Manager-side half of ResurrectedTrader/d2bsng#11: with ~7 instances running, every game hitches about once a second, in lockstep, and the stall scales with instance count.
The report locates the cost in the per-second character-state capture. But the manager owns the other end of that send, and it was doing two things that turn a 1 Hz capture into a fleet-wide stall.
SendIPCuses a bare blockingSendMessageWwith no timeout, so a game's thread is stopped for exactly as long as our WndProc takes — and every other game is queued behind it.What was wrong
The window shared a pump with the UI.
CreateMessageOnlyWindow()ran on the main thread (Program.cs:128), which then went on toApplication.Run(form). A window belongs to the thread that created it, so WM_COPYDATA was dispatched by the same thread that hosts WebView2, paints WinForms, and runs the custom titlebar's modal drag loop. Seven games sending once a second were interleaving with all of that.And the WndProc did O(payload) work inline. Before returning it allocated and copied the payload, decoded UTF-8, scanned the whole string for
"heartBeat", and ran a fullJsonSerializer.Deserializeof an envelope whose single argument is the entire escaped characterState snapshot — a few hundred KB with PlugY's expanded stash. Every step of that ran with the sender blocked and the rest of the fleet waiting.What changed
A dedicated pump thread. The window is created on its own thread running a bare
GetMessageW/DispatchMessageWloop and nothing else.CreateMessageOnlyWindowblocks until the handle exists, soProgram.Main's ordering guarantee is unchanged —EngineHostedServiceand handoff rehydration still read a validHandle.This also gives headless mode a real pump. It previously had none at all:
app.Run()blocks the main thread and there is noGetMessageloop anywhere in the tree, so headless worked only becauseMainis[STAThread]and a managed blocking wait on an STA thread happens to dispatch inter-thread sent messages.The WndProc is now a memcpy. It stamps the heartbeat (mode
0xBBBB, unchanged, and now checked before the copy) or rents a buffer, copies, and enqueues. The copy is the one part that cannot move — theCOPYDATASTRUCTbuffer is only valid for the duration of the call. Decode, theheartBeatnormalisation and the JSON parse moved toD2BSMessageHandler's consumer loop viaMessageWindow.Parse. Buffers are pooled and returned in afinally, because a characterState snapshot is hundreds of KB arriving once a second per game.A dead pump now takes the process with it. This is new risk created by the move: the pump used to be the process's pump, so its death was the process's death. Now an unexpected exit clears
Handleand callsStopApplication(). Left alive, a dead pump behind a live HWND blocks every game forever on its next send — kolbot's heartbeat thread included, so the watchdog would kill and restart each game into the same wedge, on a loop, while the manager kept serving the UI and looked perfectly healthy. Exiting destroys the window, and a send to a dead HWND fails immediately instead of hanging.Not addressed here
string, so roughly double — but if the consumer stalls (a hung Discord webhook, a slow capture ingest) the backlog now grows at wire speed. A bounded channel would mean silently dropping console output and mule saves, so it wants its own decision.MessageWindow.Dispose()never runs in GUI mode.RunWithGuiusesStartAsync/StopAsyncand never disposes the service provider, so no singleton is disposed. Harmless today only because the pump thread is a background thread and the process is exiting anyway. Pre-existing, but the new teardown path is only exercised headless.Parsestill decodes to a string and deserializes from it, which for a 300 KB snapshot is roughly 1.5 MB allocated per message, most of it large-object. That is unchanged from before, but it is now on the consumer — and a gen2 pause suspends the pump too, so whatever share of "all seven hitch in lockstep" was GC-driven will persist. Deserializing straight from the UTF-8 bytes would drop the two largest allocations.data.Contains("heartBeat")still false-positives on any payload containing that substring and replaces the message with a synthetic heartbeat. Pre-existing; now that parsing is off the hot path, checkingFunctionafter deserialising would be both cheaper and correct.Testing
dotnet build -p:RunFormat=true -p:RunInspect=true -p:SkipUIBuild=true— clean, 0 inspect findings.dotnet test— 86/86 pass.🤖 Generated with Claude Code