Increase decompiler test-suite CPU utilization via NUnit parallelism

.github/workflows/build-ilspy.yml

@@ -99,6 +99,13 @@ jobs:
 
     - name: Execute unit tests
       id: unit-tests
+      # All test hosts of the solution run concurrently on a 4-core runner. Server GC (enabled by
+      # ICSharpCode.Decompiler.Tests for machines it has to itself) sizes its heaps for the whole
+      # box and starves the neighbours: process-module walks in ILSpy.Tests.Windows stalled for
+      # 5-15 minutes, while the decompiler suite finished no faster than under workstation GC
+      # here. The environment variable overrides the runtimeconfig setting.
+      env:
+        DOTNET_gcServer: 0
       run: >
         dotnet test --solution ilspy.sln
         --configuration ${{ matrix.configuration }}

ICSharpCode.Decompiler.Tests/CorrectnessTestRunner.cs

@@ -28,7 +28,9 @@
 
 namespace ICSharpCode.Decompiler.Tests
 {
-	[TestFixture, Parallelizable(ParallelScope.All)]
+	// Order(2) enqueues these long compile+execute tests right after the roundtrip fixture,
+	// ahead of unordered fixtures, so they do not straggle at the end of a parallel run.
+	[TestFixture, Parallelizable(ParallelScope.All), Order(2)]
 	public class CorrectnessTestRunner
 	{
 		static readonly string TestCasePath = Tester.TestCasePath + "/Correctness";
@@ -436,30 +438,43 @@ async Task RunCS([CallerMemberName] string testName = null, CompilerOptions opti
 			string testOutputFileName = TestsAssemblyOutput.GetFilePath(TestCasePath, testName, Tester.GetSuffix(options) + ".exe");
 			Helpers.CompilerResults outputFile = null, decompiledOutputFile = null;
 
+			// The mcs mutation below never touches these flags, so they can be captured here
+			// and used for the original run started before the mutation happens.
+			bool useTestRunner = (options & CompilerOptions.UseTestRunner) != 0;
+			bool force32Bit = (options & CompilerOptions.Force32Bit) != 0;
+
 			try
 			{
 				outputFile = await Tester.CompileCSharp(Path.Combine(TestCasePath, testFileName), options,
 					outputFileName: testOutputFileName).ConfigureAwait(false);
-				string decompiledCodeFile = await Tester.DecompileCSharp(outputFile.PathToAssembly, Tester.GetSettings(options)).ConfigureAwait(false);
 				if ((options & CompilerOptions.UseMcsMask) != 0)
 				{
-					// For second pass, use roslyn instead of mcs.
-					// mcs has some compiler bugs that cause it to not accept ILSpy-generated code,
-					// for example when there's unreachable code due to other compiler bugs in the first mcs run.
-					options &= ~CompilerOptions.UseMcsMask;
-					options |= CompilerOptions.UseRoslynLatest;
-					// Also, add an .exe.config so that we consistently use the .NET 4.x runtime.
+					// Add an .exe.config so that we consistently use the .NET 4.x runtime.
+					// Written before the original executable starts below, because the runtime
+					// reads it at process start.
 					File.WriteAllText(outputFile.PathToAssembly + ".config", @"<?xml version=""1.0"" encoding=""utf-8""?>
 <configuration>
 	<startup>
 		<supportedRuntime version=""v4.0"" sku="".NETFramework,Version=v4.0,Profile=Client"" />
 	</startup>
 </configuration>");
+				}
+				// The original executable is complete at this point; its run overlaps the
+				// decompile and recompile of the same assembly, which only read it.
+				var originalRun = Tester.StartRun(outputFile.PathToAssembly, useTestRunner, force32Bit);
+				string decompiledCodeFile = await Tester.DecompileCSharp(outputFile.PathToAssembly, Tester.GetSettings(options)).ConfigureAwait(false);
+				if ((options & CompilerOptions.UseMcsMask) != 0)
+				{
+					// For second pass, use roslyn instead of mcs.
+					// mcs has some compiler bugs that cause it to not accept ILSpy-generated code,
+					// for example when there's unreachable code due to other compiler bugs in the first mcs run.
+					options &= ~CompilerOptions.UseMcsMask;
+					options |= CompilerOptions.UseRoslynLatest;
 					options |= CompilerOptions.TargetNet40;
 				}
 				decompiledOutputFile = await Tester.CompileCSharp(decompiledCodeFile, options).ConfigureAwait(false);
 
-				await Tester.RunAndCompareOutput(testFileName, outputFile.PathToAssembly, decompiledOutputFile.PathToAssembly, decompiledCodeFile, (options & CompilerOptions.UseTestRunner) != 0, (options & CompilerOptions.Force32Bit) != 0);
+				await Tester.RunAndCompareOutput(testFileName, originalRun, decompiledOutputFile.PathToAssembly, decompiledCodeFile, useTestRunner, force32Bit);
 				Tester.RepeatOnIOError(() => File.Delete(decompiledCodeFile));
 			}
 			finally
@@ -484,10 +499,13 @@ async Task RunVB([CallerMemberName] string testName = null, CompilerOptions opti
 			{
 				outputFile = await Tester.CompileVB(Path.Combine(TestCasePath, testFileName), options,
 					outputFileName: testOutputFileName).ConfigureAwait(false);
+				// The original executable is complete at this point; its run overlaps the
+				// decompile and recompile of the same assembly, which only read it.
+				var originalRun = Tester.StartRun(outputFile.PathToAssembly, (options & CompilerOptions.UseTestRunner) != 0, (options & CompilerOptions.Force32Bit) != 0);
 				string decompiledCodeFile = await Tester.DecompileCSharp(outputFile.PathToAssembly, Tester.GetSettings(options)).ConfigureAwait(false);
 				decompiledOutputFile = await Tester.CompileCSharp(decompiledCodeFile, options).ConfigureAwait(false);
 
-				await Tester.RunAndCompareOutput(testFileName, outputFile.PathToAssembly, decompiledOutputFile.PathToAssembly, decompiledCodeFile, (options & CompilerOptions.UseTestRunner) != 0, (options & CompilerOptions.Force32Bit) != 0);
+				await Tester.RunAndCompareOutput(testFileName, originalRun, decompiledOutputFile.PathToAssembly, decompiledCodeFile, (options & CompilerOptions.UseTestRunner) != 0, (options & CompilerOptions.Force32Bit) != 0);
 				Tester.RepeatOnIOError(() => File.Delete(decompiledCodeFile));
 			}
 			finally
@@ -519,10 +537,13 @@ async Task RunIL(string testFileName, CompilerOptions options = CompilerOptions.
 					options |= CompilerOptions.UseRoslynLatest;
 				}
 				outputFile = await Tester.AssembleIL(Path.Combine(TestCasePath, testFileName), asmOptions).ConfigureAwait(false);
+				// The original executable is complete at this point; its run overlaps the
+				// decompile and recompile of the same assembly, which only read it.
+				var originalRun = Tester.StartRun(outputFile, (options & CompilerOptions.UseTestRunner) != 0, (options & CompilerOptions.Force32Bit) != 0);
 				string decompiledCodeFile = await Tester.DecompileCSharp(outputFile, Tester.GetSettings(options)).ConfigureAwait(false);
 				decompiledOutputFile = await Tester.CompileCSharp(decompiledCodeFile, options).ConfigureAwait(false);
 
-				await Tester.RunAndCompareOutput(testFileName, outputFile, decompiledOutputFile.PathToAssembly, decompiledCodeFile, (options & CompilerOptions.UseTestRunner) != 0, (options & CompilerOptions.Force32Bit) != 0).ConfigureAwait(false);
+				await Tester.RunAndCompareOutput(testFileName, originalRun, decompiledOutputFile.PathToAssembly, decompiledCodeFile, (options & CompilerOptions.UseTestRunner) != 0, (options & CompilerOptions.Force32Bit) != 0).ConfigureAwait(false);
 				Tester.RepeatOnIOError(() => File.Delete(decompiledCodeFile));
 			}
 			finally

ICSharpCode.Decompiler.Tests/Helpers/RoslynToolset.cs

@@ -127,6 +127,8 @@ await packageReader.CopyFilesAsync(outputPath, files,
 
 	class RoslynToolset : AbstractToolset
 	{
+		// Registrations run concurrently while Tester.Initialize awaits all Fetch calls;
+		// lookups only happen after Initialize completes, so the read paths stay lock-free.
 		readonly Dictionary<string, string> installedCompilers = new Dictionary<string, string> {
 			{ "legacy", Environment.ExpandEnvironmentVariables(@"%WINDIR%\Microsoft.NET\Framework\v4.0.30319") }
 		};
@@ -144,7 +146,10 @@ public async Task Fetch(string version, string packageName = "Microsoft.Net.Comp
 				await FetchPackage(packageName, version, sourcePath, Path.Combine(baseDir, version)).ConfigureAwait(false);
 			}
 
-			installedCompilers.Add(SanitizeVersion(version), path);
+			lock (installedCompilers)
+			{
+				installedCompilers.Add(SanitizeVersion(version), path);
+			}
 		}
 
 		// In the .NET ("netcore") build of the compiler toolset the executables live in a
@@ -218,6 +223,8 @@ public async Task Fetch()
 
 	class RefAssembliesToolset : AbstractToolset
 	{
+		// Registrations run concurrently while Tester.Initialize awaits all Fetch calls;
+		// lookups only happen after Initialize completes, so the read paths stay lock-free.
 		readonly Dictionary<string, string> installedFrameworks = new Dictionary<string, string> {
 			{ "legacy", Path.Combine(Roundtrip.RoundtripAssembly.TestDir, "dotnet", "legacy") },
 			{ "2.2.0", Path.Combine(Roundtrip.RoundtripAssembly.TestDir, "dotnet", "netcore-2.2") },
@@ -236,7 +243,10 @@ public async Task Fetch(string version, string packageName = "Microsoft.NETCore.
 				await FetchPackage(packageName, version, sourcePath, Path.Combine(baseDir, version)).ConfigureAwait(false);
 			}
 
-			installedFrameworks.Add(RoslynToolset.SanitizeVersion(version), path);
+			lock (installedFrameworks)
+			{
+				installedFrameworks.Add(RoslynToolset.SanitizeVersion(version), path);
+			}
 		}
 
 		internal string GetPath(string targetFramework)

ICSharpCode.Decompiler.Tests/Helpers/Tester.cs

@@ -143,37 +143,54 @@ static Tester()
 
 		internal static async Task Initialize()
 		{
-			await roslynToolset.Fetch("1.3.2", "Microsoft.Net.Compilers", "tools").ConfigureAwait(false);
-			if (OperatingSystem.IsWindows())
-			{
-				await roslynToolset.Fetch("2.10.0", "Microsoft.Net.Compilers", "tools").ConfigureAwait(false);
-			}
-			else
-			{
+			// All fetches download/extract into disjoint directories and the TestRunner builds
+			// do not depend on any fetched toolset, so everything runs concurrently and is
+			// awaited in one place. Individual toolset registrations are synchronized inside
+			// the toolsets (see RoslynToolset.cs).
+			var tasks = new List<Task> {
+				roslynToolset.Fetch("1.3.2", "Microsoft.Net.Compilers", "tools"),
 				// Microsoft.Net.Compilers only ships .NET Framework executables. The sibling
 				// Microsoft.NETCore.Compilers package contains the dotnet-hosted build of the
 				// same compiler version (tools/bincore/csc.dll), usable on any platform.
-				await roslynToolset.Fetch("2.10.0", "Microsoft.NETCore.Compilers", "tools/bincore").ConfigureAwait(false);
+				OperatingSystem.IsWindows()
+					? roslynToolset.Fetch("2.10.0", "Microsoft.Net.Compilers", "tools")
+					: roslynToolset.Fetch("2.10.0", "Microsoft.NETCore.Compilers", "tools/bincore"),
+				// On non-Windows hosts the net472 compiler binaries cannot be executed; use the
+				// .NET build of each toolset instead. Its tasks folder is named "netcoreapp3.1"
+				// up to Roslyn 3.x and "netcore" from Roslyn 4.x on.
+				roslynToolset.Fetch("3.11.0", sourcePath: OperatingSystem.IsWindows() ? "tasks/net472" : "tasks/netcoreapp3.1"),
+				roslynToolset.Fetch("4.14.0", sourcePath: OperatingSystem.IsWindows() ? "tasks/net472" : "tasks/netcore"),
+				roslynToolset.Fetch(roslynLatestVersion, sourcePath: OperatingSystem.IsWindows() ? "tasks/net472" : "tasks/netcore"),
+				vswhereToolset.Fetch(),
+				RefAssembliesToolset.Fetch("5.0.0", sourcePath: "ref/net5.0"),
+				RefAssembliesToolset.Fetch("9.0.0", sourcePath: "ref/net9.0"),
+				RefAssembliesToolset.Fetch(CurrentNetCoreRefAsmVersion, sourcePath: $"ref/net{CurrentNetCoreVersion}"),
+				BuildTestRunners(),
+			};
+			Task all = Task.WhenAll(tasks);
+			try
+			{
+				await all.ConfigureAwait(false);
 			}
-			// On non-Windows hosts the net472 compiler binaries cannot be executed; use the
-			// .NET build of each toolset instead. Its tasks folder is named "netcoreapp3.1"
-			// up to Roslyn 3.x and "netcore" from Roslyn 4.x on.
-			await roslynToolset.Fetch("3.11.0", sourcePath: OperatingSystem.IsWindows() ? "tasks/net472" : "tasks/netcoreapp3.1").ConfigureAwait(false);
-			await roslynToolset.Fetch("4.14.0", sourcePath: OperatingSystem.IsWindows() ? "tasks/net472" : "tasks/netcore").ConfigureAwait(false);
-			await roslynToolset.Fetch(roslynLatestVersion, sourcePath: OperatingSystem.IsWindows() ? "tasks/net472" : "tasks/netcore").ConfigureAwait(false);
-
-			await vswhereToolset.Fetch().ConfigureAwait(false);
-			await RefAssembliesToolset.Fetch("5.0.0", sourcePath: "ref/net5.0").ConfigureAwait(false);
-			await RefAssembliesToolset.Fetch("9.0.0", sourcePath: "ref/net9.0").ConfigureAwait(false);
-			await RefAssembliesToolset.Fetch(CurrentNetCoreRefAsmVersion, sourcePath: $"ref/net{CurrentNetCoreVersion}").ConfigureAwait(false);
+			catch when (all.Exception is { InnerExceptions.Count: > 1 })
+			{
+				// Surface every failed download/build, not just the first.
+				throw all.Exception;
+			}
+		}
 
+		static async Task BuildTestRunners()
+		{
 #if DEBUG
 			const string testRunnerConfig = "Debug";
 #else
 			const string testRunnerConfig = "Release";
 #endif
 			if (OperatingSystem.IsWindows())
 			{
+				// The two RID builds share the same project file and intermediate directory
+				// (obj/project.assets.json is written by each build's implicit restore), so
+				// they must not run concurrently with each other.
 				await BuildTestRunner("win-x86", testRunnerConfig).ConfigureAwait(false);
 				await BuildTestRunner("win-x64", testRunnerConfig).ConfigureAwait(false);
 			}
@@ -1090,21 +1107,34 @@ private static CSharpFormattingOptions CreateFormattingPolicyForTests()
 			return formattingPolicy;
 		}
 
-		public static async Task RunAndCompareOutput(string testFileName, string outputFile, string decompiledOutputFile, string decompiledCodeFile = null, bool useTestRunner = false, bool force32Bit = false)
+		/// <summary>
+		/// Starts executing the given assembly and returns the in-flight task, so that the run
+		/// can overlap other work (e.g. decompiling and recompiling the same assembly). The
+		/// task's fault is pre-observed: a caller that abandons the run because an earlier
+		/// pipeline stage failed first does not trigger UnobservedTaskException.
+		/// </summary>
+		public static Task<(int ExitCode, string Output, string Error)> StartRun(string assemblyFileName, bool useTestRunner = false, bool force32Bit = false)
 		{
-			string output1, output2, error1, error2;
-			int result1, result2;
+			var task = useTestRunner ? RunWithTestRunner(assemblyFileName, force32Bit) : Run(assemblyFileName);
+			task.ContinueWith(static t => _ = t.Exception, CancellationToken.None,
+				TaskContinuationOptions.OnlyOnFaulted | TaskContinuationOptions.ExecuteSynchronously, TaskScheduler.Default);
+			return task;
+		}
 
-			if (useTestRunner)
-			{
-				(result1, output1, error1) = await RunWithTestRunner(outputFile, force32Bit).ConfigureAwait(false);
-				(result2, output2, error2) = await RunWithTestRunner(decompiledOutputFile, force32Bit).ConfigureAwait(false);
-			}
-			else
-			{
-				(result1, output1, error1) = await Run(outputFile).ConfigureAwait(false);
-				(result2, output2, error2) = await Run(decompiledOutputFile).ConfigureAwait(false);
-			}
+		public static Task RunAndCompareOutput(string testFileName, string outputFile, string decompiledOutputFile, string decompiledCodeFile = null, bool useTestRunner = false, bool force32Bit = false)
+		{
+			return RunAndCompareOutput(testFileName, StartRun(outputFile, useTestRunner, force32Bit), decompiledOutputFile, decompiledCodeFile, useTestRunner, force32Bit);
+		}
+
+		public static async Task RunAndCompareOutput(string testFileName, Task<(int ExitCode, string Output, string Error)> originalRun, string decompiledOutputFile, string decompiledCodeFile = null, bool useTestRunner = false, bool force32Bit = false)
+		{
+			var decompiledRun = StartRun(decompiledOutputFile, useTestRunner, force32Bit);
+			// Plain WhenAll, no error aggregation: it observes both faults and rethrows the
+			// first one, which keeps NUnit's Ignore semantics intact when both runs raise
+			// IgnoreException (e.g. Force32Bit on a non-Windows host).
+			await Task.WhenAll(originalRun, decompiledRun).ConfigureAwait(false);
+			var (result1, output1, error1) = originalRun.Result;
+			var (result2, output2, error2) = decompiledRun.Result;
 
 			Assert.That(result1, Is.EqualTo(0), "Exit code != 0; did the test case crash?" + Environment.NewLine + error1);
 			Assert.That(result2, Is.EqualTo(0), "Exit code != 0; did the decompiled code crash?" + Environment.NewLine + error2);
@@ -1207,10 +1237,23 @@ public static async Task SignAssembly(string assemblyPath, string keyFilePath)
 			}
 		}
 
-		public static async Task<string> FindMSBuild()
+		// Lazy<Task<T>> memoizes the vswhere lookup: the answer is invariant for the process,
+		// and the parallel roundtrip tests would otherwise each spawn their own vswhere.exe.
+		// A failed lookup stays cached, which is fine because a missing MSBuild is
+		// environmental, not transient.
+		static readonly Lazy<Task<string>> msbuildPath = new(FindMSBuildUncached, LazyThreadSafetyMode.ExecutionAndPublication);
+
+		public static Task<string> FindMSBuild()
 		{
+			// The platform check stays outside the cache so that the IgnoreException is
+			// raised per test instead of being memoized as a faulted task.
 			if (!OperatingSystem.IsWindows())
 				Assert.Ignore("FindMSBuild uses vswhere.exe to locate Visual Studio's MSBuild; not available on this platform.");
+			return msbuildPath.Value;
+		}
+
+		static async Task<string> FindMSBuildUncached()
+		{
 			string path = vswhereToolset.GetVsWhere();
 
 			var result = await Cli.Wrap(path)

ICSharpCode.Decompiler.Tests/ICSharpCode.Decompiler.Tests.csproj

@@ -18,6 +18,12 @@
 
     <AllowUnsafeBlocks>True</AllowUnsafeBlocks>
 
+    <!-- The suite runs dozens of allocation-heavy decompiles concurrently (2x logical CPUs NUnit
+         workers, each with WholeProjectDecompiler's own Parallel.ForEach). Workstation GC stops the
+         whole process for every gen0/gen1 collection any worker triggers; measured on a 24-thread
+         machine that was ~300 s of pause in a ~490 s run. Server GC gives each core its own heap and
+         collects in parallel. -->
+    <ServerGarbageCollection>true</ServerGarbageCollection>
     <!-- NU1902/1903 are "Package 'X' has a known security vulnerability". In our tests, we don't care. -->
     <!-- CA1416 fires because the TFM dropped its -windows suffix: Windows-only helpers (SdkUtility,
          Tester.SignAssembly/FindMSBuild/RunWithTestRunner) and Windows-only test inputs (Console.CapsLock
@@ -58,6 +64,17 @@
     <DefineConstants>TRACE;$(DefineConstants)</DefineConstants>
   </PropertyGroup>
 
+  <ItemGroup>
+    <!-- NUnit's default worker count (one per logical CPU) underutilizes the machine because
+         most matrix tests block on child processes (csc/vbc/ilasm/msbuild/test runners).
+         LevelOfParallelism only accepts a constant, so compute 2x logical CPUs at build time;
+         the machine building the tests is the machine running them. -->
+    <AssemblyAttribute Include="NUnit.Framework.LevelOfParallelism">
+      <_Parameter1>$([MSBuild]::Multiply($([System.Environment]::ProcessorCount), 2))</_Parameter1>
+      <_Parameter1_TypeName>System.Int32</_Parameter1_TypeName>
+    </AssemblyAttribute>
+  </ItemGroup>
+
   <ItemGroup>
     <PackageReference Include="DiffLib" />
     <PackageReference Include="CliWrap" />

ICSharpCode.Decompiler.Tests/Properties/AssemblyInfo.cs

@@ -22,8 +22,15 @@
 using System.Reflection;
 using System.Runtime.InteropServices;
 
+using NUnit.Framework;
+
 #endregion
 
+// Fixtures without their own Parallelizable attribute run concurrently with other fixtures;
+// tests within such a fixture still run sequentially. Fixtures that share process-global
+// state must opt out individually with [NonParallelizable].
+[assembly: Parallelizable(ParallelScope.Fixtures)]
+
 [assembly: AssemblyTrademark("")]
 [assembly: AssemblyCulture("")]