Skip to content

Increase decompiler test-suite CPU utilization via NUnit parallelism - #3940

Draft
christophwille wants to merge 2 commits into
masterfrom
decompiler-tests-parallelism
Draft

Increase decompiler test-suite CPU utilization via NUnit parallelism#3940
christophwille wants to merge 2 commits into
masterfrom
decompiler-tests-parallelism

Conversation

@christophwille

Copy link
Copy Markdown
Member

A full ICSharpCode.Decompiler.Tests run kept a 24-logical-CPU machine at only ~46% average CPU. This PR makes the suite use the machine it runs on: fixtures run in parallel by default, the NUnit worker pool is oversubscribed to 2x logical CPUs, and the multi-minute tests are scheduled first. A companion doc describes the Windows Defender exclusions that would remove the remaining scan overhead (deliberately not automated).

Measured causes of the idle CPU

  1. ~30 fixture files had no Parallelizable attribute (TypeSystem, Output, Util, Metadata, ProjectDecompiler, ...) - NUnit runs unattributed fixtures one at a time on its non-parallel queue. Only the 12 matrix runners declared Parallelizable(ParallelScope.All).
  2. Worker count = ProcessorCount, but matrix tests spend most of their time blocked on child processes (csc/vbc/ilasm/msbuild/nunit-agent/TestRunner). Blocked workers leave cores idle.
  3. The critical path is a handful of 3-5 minute roundtrip tests (Random_TestCase_1, ExplicitConversions*, NRefactory_CSharp, ...) that started mid-run and straggled at the end.
  4. Windows Defender (MsMpEng.exe) ran at 1-4 cores continuously, scanning every compiled fixture and spawned process (out of scope for code changes; see doc/WindowsDefenderExclusions.md).

Options considered

For raising the worker count:

Option Verdict
Local .runsettings with NumberOfTestWorkers Rejected - a fixed number in a file, and an extra flag every run
Hard-coded [assembly: LevelOfParallelism(48)] Rejected - tuned to one machine; wrong everywhere else (e.g. 4-core CI runners)
Build-time computed attribute, always 2x logical CPUs Chosen - machine-independent policy, no number checked in

For the Defender overhead: apply exclusions vs. document them vs. skip. Machine-level AV configuration does not belong in the repo, so this PR only documents the exclusions (doc/WindowsDefenderExclusions.md).

How the oversubscription works

[assembly: LevelOfParallelism] only accepts a compile-time constant, so the csproj generates it during build:

<AssemblyAttribute Include="NUnit.Framework.LevelOfParallelism">
  <_Parameter1>$([MSBuild]::Multiply($([System.Environment]::ProcessorCount), 2))</_Parameter1>
  <_Parameter1_TypeName>System.Int32</_Parameter1_TypeName>
</AssemblyAttribute>
  • The AssemblyAttribute item is emitted into the SDK-generated AssemblyInfo.cs (obj/.../ICSharpCode.Decompiler.Tests.AssemblyInfo.cs); _Parameter1_TypeName makes MSBuild emit the value as an int rather than a string.
  • The value is 2x the logical CPU count of the machine building the tests - which is the machine running them, both locally and in CI. On the 24-thread dev box this generates LevelOfParallelism(48); on a 4-core CI runner it generates 8.
  • 2x is deliberate oversubscription: NUnit workers are dedicated threads, and most matrix tests block on child compiler/runner processes, so twice as many in-flight tests keeps cores busy without thrashing. The NUnit adapter honors the attribute unless a runsettings value overrides it (none does).
  • [assembly: Parallelizable(ParallelScope.Fixtures)] (in Properties/AssemblyInfo.cs) makes the previously-serial fixtures run concurrently with each other while tests within such a fixture stay sequential. Fixtures sharing process-global state can opt out with [NonParallelizable]; none currently needs to (DecompilerEventSourceTests was audited - its assertions already filter by payload marker).
  • [Order(1)]/[Order(2)] on RoundtripAssembly and CorrectnessTestRunner enqueue the multi-minute tests first.

Before / after (24 logical CPUs, ILSpy-tests checked out)

Baseline This PR
In-flight tests (from TRX start/end stamps) 24, flat 47-48, flat
Longest test start ~t+150s t=0
Wall time 8m16s (3966 tests) 8m11s-8m37s (4170 tests, two runs)
Average total CPU 45.8% 46.2%
Failures 0 0 (two consecutive green runs)

The test-count difference is unrelated new tests picked up by the rebuild. Concurrency verifiably doubled and scheduling is now optimal (every giant starts at t=0), yet wall time and CPU stayed flat - which proves the worker pool is no longer the constraint:

  • The suite is now bounded by its single longest test. Random_TestCase_1 runs 464s wall-to-wall (up from 314s mid-run at baseline - it slows under the doubled contention); total wall time is essentially that one test. More workers cannot help; they only add contention against the critical path.
  • The remaining idle CPU is Defender scan latency on thousands of process spawns and file writes (48 in-flight tests averaged ~0.23 cores each).

Follow-up ideas (non-Defender)

  • Split the generated monoliths in ILSpy-tests. Random_TestCase_1 and the ExplicitConversions* variants are single generated executables from Random Tests/TestCaseGenerator in the ILSpy-tests submodule. Splitting each into several smaller assemblies (or emitting the conversions matrix as N partitions) would turn one 460s pipeline into parallelizable chunks - the only way to push wall time meaningfully below ~8 minutes.
  • The per-file decompile phase is already parallel (WholeProjectDecompiler.MaxDegreeOfParallelism = ProcessorCount), so the giants' serial cost is in their single-assembly csc rebuild and execute/compare phases, which only splitting addresses.

🤖 Generated with Claude Code

A full ICSharpCode.Decompiler.Tests run kept a 24-logical-CPU machine at
only ~46% average CPU: unattributed fixtures ran one at a time on NUnit's
non-parallel queue, the default one-worker-per-CPU pool sat blocked on
child compiler/runner processes, and the multi-minute roundtrip and
correctness tests straggled at the end of the run. Fixtures now run in
parallel by default, the worker count is generated at build time as 2x
the building machine's logical CPUs (LevelOfParallelism only accepts a
constant, and a checked-in number would be wrong on every other machine),
and the two heavyweight fixtures are ordered first so the longest tests
start immediately. In-flight tests measured 47-48 instead of 24; the
suite is now bounded by its single longest test rather than by scheduling.

Assisted-by: Claude:claude-fable-5:Claude Code
While the decompiler test suite runs, Defender's scan engine was measured
using 1-4 CPU cores continuously and adds scan latency to every spawned
compiler/runner process. Machine-level AV configuration does not belong in
the repo, so document the folders worth excluding, the tradeoff, and the
commands instead of automating the change.

Assisted-by: Claude:claude-fable-5:Claude Code
@christophwille

Copy link
Copy Markdown
Member Author

Verification: do the NUnit parallelism attributes actually work under Microsoft.Testing.Platform?

The attributes are consumed by NUnit itself, not by the host platform, so MTP vs. VSTest makes no difference - but here is the full verified chain rather than an appeal to documentation.

1. The attributes are physically in the compiled assembly

Reading the PE metadata of the built ICSharpCode.Decompiler.Tests.dll (System.Reflection.Metadata, no runtime load) shows both assembly-level custom attributes with the expected arguments:

Attribute Blob Decoded
NUnit.Framework.LevelOfParallelismAttribute 01 00 30 00 00 00 int32 0x30 = 48 (2x24 logical CPUs of the build machine)
NUnit.Framework.ParallelizableAttribute 01 00 00 02 00 00 0x200 = ParallelScope.Fixtures

2. NUnit 4.6.1 honors the attribute unless the adapter passes an override

Decompiling nunit.framework.dll (with this repo's ilspycmd), NUnitTestAssemblyRunner.GetLevelOfParallelism is:

private int GetLevelOfParallelism(ITest loadedTest)
{
    if (!Settings.TryGetValue("NumberOfTestWorkers", out object value))
        return loadedTest.Properties.TryGet("LevelOfParallelism", DefaultLevelOfParallelism);
    return ConvertSetting<int>(value);
}

The assembly attribute populates the LevelOfParallelism property on the loaded test assembly; the result feeds new ParallelWorkItemDispatcher(48). The parallelism engine is entirely NUnit's in-process dispatcher - MTP (via Microsoft.Testing.Extensions.VSTestBridge) just hosts the adapter and adds nothing to this path.

3. NUnit3TestAdapter 6.2.0 does not inject NumberOfTestWorkers in our invocation

Decompiling NUnit3.TestAdapter.dll:

  • AdapterSettings: NumberOfTestWorkers = GetInnerTextAsInt(xmlNode, "NumberOfTestWorkers", -1) - defaults to -1 when no runsettings node exists.
  • NUnitTestAdapter.CreateTestPackage: the setting is only written into the test package when >= 0, so with the default -1 the framework falls through to the assembly attribute.
  • The only paths that force it to 0 (serial): debugger attached (without AllowParallelWithDebugger), DisableParallelization in runsettings, or CollectDataForEachTestSeparately / Live Unit Testing in-proc collectors. None applies to a plain --report-trx run with no --settings.

4. Empirical proof from the TRX timelines

Reconstructing concurrency from per-test startTime/endTime stamps:

  • Overall in-flight tests: 24, flat (baseline) vs. 47-48, flat (this PR) for the entire run.
  • The ~30 fixtures that previously had no Parallelizable attribute (718 tests in the comparison set): 0 cross-fixture overlapping executions in the baseline run - perfectly serial, as predicted - vs. 4596 overlapping execution pairs across 206 distinct fixture pairs with this PR.

Caveat

The attribute is a default, not a mandate: --settings with NumberOfTestWorkers, DisableParallelization, CollectDataForEachTestSeparately, or running under a debugger overrides or disables it. That matches the adapter's long-standing behavior under VSTest as well.

🤖 Generated with Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant