Repository navigation
Cut the ILSpy.Tests wall clock: cheap fixtures, dead waits, duplicate tests, and process sharding - #4269
Open
christophwille wants to merge 8 commits into
Open
Cut the ILSpy.Tests wall clock: cheap fixtures, dead waits, duplicate tests, and process sharding#4269christophwille wants to merge 8 commits into
christophwille wants to merge 8 commits into
Conversation
Most of the slow UI tests only needed "a document", "an active tab" or "a type with a few members", yet got it by decompiling System.String, System.Object or System.Linq.Enumerable, or by landing on the CoreLib assembly row. Each such decompile rebuilds a framework type system and costs one to ten seconds; the sample type in the test assembly and the emitted fixture assemblies give the same shape for a fraction of that. FieldAccessAnalyzerTests also built a fresh AnalyzerContext per call over the whole test assembly; one per fixture is enough. Tests whose assertion depends on the framework type (XML docs, type forwarders, derived-type scans, a long document, resources, references) keep it. #4262 Single-process wall clock on the same machine: 369 s -> 271 s. Assisted-by: Claude:claude-fable-5-1:Claude Code
Nine tests slept past a "NavigationHistory collapses selections within 0.5 s" window before selecting a second node. The history only collapses a rapid re-selection of the same entry (NavigationHistory.Record), and every one of these tests selects a different node, so the waits bought nothing. The affected fixtures pass three consecutive runs without them. #4262 Single-process wall clock on the same machine: 271 s -> 262 s. Assisted-by: Claude:claude-fable-5-1:Claude Code
Each deleted test drove the same production path as a surviving test in the same file and asserted a subset of what that test asserts, so a regression it could catch already fails the survivor. Where the deleted test held one assertion of its own (the TypeTreeNode type check, the MenuCategory metadata, the freeze flipping IsPreview without a new tab, the Shift+Down range precondition), that assertion moved into the survivor. The Ctrl+R test on the assembly tree pane duplicated AnalyzerTreeKeyboardTests, which reaches the same window key binding. #4262 Single-process wall clock on the same machine: 262 s -> 251 s, 16 tests fewer. Assisted-by: Claude:claude-fable-5-1:Claude Code
Each merged pair booted the app, built the same state and drove the same action, differing only in which property they then asserted; one test now carries all of those assertions, so the shared setup runs once. The package tests in AssemblyTreeModelTests also built the same zip (two emitted assemblies) per test, which is immutable input and now built once per fixture. #4262 Single-process wall clock on the same machine: 251 s -> 256 s, inside the 20 s run-to-run noise; the sum of test durations went 249 s -> 253 s with 15 tests fewer, so this step is about not re-arranging the same state rather than about seconds. Assisted-by: Claude:claude-fable-5-1:Claude Code
ILSpy.Tests is serial by design: AvaloniaTestIsolation(PerAssembly) gives one headless session whose dispatcher every [AvaloniaTest] blocks on, and the app composition is process-global, so NUnit workers would only queue. The only parallelism available is several test hosts with disjoint filters, which this script arranges: a greedy split of a measured per-group weight table, with the last shard taking the complement of everything the others name so a new fixture can never fall out of every shard. The namespace-only split is not enough because about a hundred fixtures in sub-folders still declare the root namespace, which is why root fixtures appear in the table and why the complement is needed. The leftover-session sweep in TestApp deleted every sibling directory under %TEMP%\ILSpy.Tests at startup, including the live session directories of another host, so it now only removes directories old enough to belong to an earlier run. #4262 Measured on the same machine, Debug, after the test edits (256 s in one process): 4 shards 91 s (2.8x), 2 shards 153 s (1.7x), 1 shard through the script 259 s. Shard totals sum to every non-[Explicit] test. Assisted-by: Claude:claude-fable-5-1:Claude Code
The "Execute UI tests" step is the second-longest in the Desktop job and runs one serial test host on a 3-4 core runner. Two concurrent shards through runuitests.ps1 use a second core; the TRX upload and report steps already glob test-results/*.trx, so the per-shard files flow into them unchanged. The Windows job keeps its solution-wide run: it already overlaps ILSpy.Tests with the other test hosts and is bounded by the decompiler suite. #4262 Assisted-by: Claude:claude-fable-5-1:Claude Code
Sharing one context across the fixture's ten Analyze calls was expected to save the per-call scan of the test assembly, but four consecutive full-suite runs measured the fixture at 8.4-8.7 s against 4.9 s with a fresh context per call, every test about 0.4 s slower. The per-call shape is what the production analyzer pane uses as well, so this keeps the measured faster variant rather than the predicted one. #4262 Assisted-by: Claude:claude-fable-5-1:Claude Code
The Release compiler folds Twice's local into a single return expression, so "return result" only appears in the decompiled text of a Debug build of the test assembly; CI runs the Release build and failed both DecompilerViewTests cases on Linux and macOS. "text.Length" is in the body either way and still proves the body, not just the signature, was rendered. Assisted-by: Claude:claude-fable-5-1:Claude Code
Member
Author
|
CI numbers, now that the run is green (9f41e77 fixed the two sample-body assertions that only held for a Debug build of the test assembly; CI tests the Release build). "Execute UI tests" step and whole Desktop job, master run 39292241346 (today's HEAD before this branch) vs this PR's run 39317397450:
The step time combines the test edits (fewer and cheaper tests) with the two concurrent shards; the per-shard TRX files land in the existing Assisted-by: Claude:claude-fable-5-1:Claude Code |
This branch has not been deployed
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
1 participant
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.
Implements levers A-D from #4262 ("Totals and recommended order"), plus the two sharding items: a local script for 4 shards and 2 shards on the Linux/macOS CI jobs. The harness knobs are out of scope (measured as noise in the issue). One commit per lever, so each step's effect is visible in the history.
Before implementing, the A-D sites were rediscovered against current
master(the issue was written at 6d5b5fe): every listed site still existed, and the tests added since (OptionsTabTests, FrozenTabRefreshTests, RunInNewTabTests, the new ReferenceClickTests cases, the DecompileInNewView middle-click tests that decompile the whole CoreLib assembly node) were folded into A.Measurements
Same machine as the issue (24 logical CPUs, Windows), Debug,
ILSpy.Tests.exe --report-trxrun alone frombin/Debug/net11.0, one run per step. Run-to-run noise is about 20 s (5 %), so the "sum of durations of the touched fixtures" column (from the TRX files, before vs after that commit) is the precise per-step number; the wall clock is the headline.Task.Delay(600)D is inside the noise band on the wall clock; it removes 15 boots at ~0.3 s each, which is what the fixture column shows. The 4 tests that never run are the
[Explicit]/ignored ones, same as before.Largest single-test wins from A (seconds, baseline -> after A):
AnalyzerResultHighlightTests15.7 -> 2.1,ReferenceClickTests18.0 -> 2.7,PreviewTabPromotionTests13.4 -> 6.4,DecompilerViewTests12.6 -> 5.5,BookmarkNavigationViewTests10.0 -> 5.1,DecompileInNewViewTests9.9 -> 4.0,ILLanguageTooltipTests7.1 -> 1.7,TabFocusNavigationTests5.8 -> 0.8,BookmarkAnchoringTests5.0 -> 0.4,SetActiveDockableTests4.9 -> 0.4.One prediction from the issue did not hold: sharing one
AnalyzerContextacrossFieldAccessAnalyzerTestsmade that fixture slower in all four runs (4.9 s -> 8.4-8.7 s), so 7e941ad goes back to the per-call context the fixture had before.Sharding (after D, same machine)
runuitests.ps1runs the built suite as N concurrentdotnet test --no-buildhosts with disjoint--filters from a measured weight table; the last shard is the complement of everything the others name, so a fixture added later always runs somewhere. Every shard writestest-results/ui-shard<N>.trx+.log.Shard totals sum to 1329 in every configuration, i.e. every non-
[Explicit]test. Concurrent hosts needed one harness fix:TestApp.SweepLeftoversdeleted every sibling session directory under%TEMP%\ILSpy.Testsat startup, including another live host's; it now only sweeps directories older than 30 minutes.CI
The Linux and macOS "Execute UI tests" step now runs
./runuitests.ps1 -Shards 2 -Configuration Release; the upload and report steps already globtest-results/*.trx. Two shards, not more: the heavier shard peaks at about 3 GB and the runners have 3-4 cores. The Windows job is unchanged (it overlaps the UI tests with the other test hosts already and is bounded by the decompiler suite). The step time on this PR's run is the number to compare against the 269 s the issue recorded.AGENTS.mdnotes that sharding is the local optimization for this suite and how to run one fixture.Assisted-by: Claude:claude-fable-5-1:Claude Code
🤖 Generated with Claude Code