Main window: stop timers and undo polling on close, start them on Loaded (UI suite 2m20s → ~55s) - #14593
Merged
Merged
Conversation
…rt them on Loaded Hunting the thread growth seen in the UI test suite (ThreadPool at ~200 threads, process at ~900 by the end of a run) with per-test stack dumps: - 318 threads sat in UiTickPump.Loop: MainViewModel started its 50 ms position pump and 16 ms cursor pump in the constructor and nothing stopped them for a view model whose window closed without OnClosing, or that never had a window. The 400 ms slow timer (a full GetFastHash pass) kept running on the shared UI thread for every one of them too. - 149 thread-pool threads were parked in Dispatcher.Invoke inside UndoRedoManager.CheckForChanges: the 333 ms change-detection poll of every closed editor window kept firing, and its blocking Invoke never returned. That is the starvation behind the OverlappingTick flake and the 15 s mpv test. In the app the same leak applies to File > New window: a second editor window closed again left its pumps and its undo poll running for the process lifetime. StartBackgroundWork runs from OnLoaded (the timers have nothing to drive before the window is up), StopBackgroundWork runs from CleanUp and from the host window's Closed event, and the delayed StartChangeDetection in OnLoaded is skipped when the window is already gone. Test-side speed-ups on top: the multiple-replace preview timer is exposed as PreviewIntervalMs so the tests wait 25 ms instead of 250 ms per settle, and the fix-common-errors "Analysing..." delays are internal statics the unfixable-errors tests zero. UI suite: 2 m 20 s -> ~55 s locally, thread pool stays at 9 threads. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
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.
What
Follow-up to #14591: hunting the thread growth in the UI test suite with per-test
dotnet-stackdumps.Findings
UiTickPump.LoopMainViewModelstarted its 50 ms position pump and 16 ms cursor pump in the constructor; a view model whose window closed withoutOnClosing(every test host, which detaches the save prompt) or that never had a window kept them forever. The 400 ms slow timer (a fullGetFastHashpass) ran on the shared UI thread for every one of them too.Dispatcher.InvokeinUndoRedoManager.CheckForChangesOverlappingTickflake and the 15 s mpv test in #14591.The same leak exists in the app for File > New window: closing the second editor window left its pumps and undo poll running until exit.
Change
StartBackgroundWork()runs fromOnLoadedinstead of the constructor (the timers have nothing to drive before the window is up).StopBackgroundWork()(timers +StopChangeDetection) runs fromCleanUpand from the host window'sClosedevent, so it also covers a close that bypassesOnClosing.StartChangeDetectioninOnLoadedis skipped when the window is already gone.MultipleReplaceViewModel.PreviewIntervalMs(internal) lets the tests wait 25 ms per preview settle instead of 250 ms; the fix-common-errors "Analysing…" delays are internal statics the unfixable-errors tests zero.Numbers (local, full
UITests)SubtitleGridScrollPerformanceTestsTwo full runs clean (4895 passed, 1 skipped).
🤖 Generated with Claude Code