Skip to content

Commit fb0b206

Browse files
markplesmarkples
authored
Fix GCStress timeouts in JIT/jit64 (#85040)
This includes several changes that seem to help with the timeouts. It might be overkill but seems like a good direction as this has been broken for a while. - Change the test wrapper logic to only put one test in a TestExecutor so that the callstacks are much simpler. - Factor the test wrapper logic into some helpers to simplify the main method. I also tried to make the "Full" and "XHarness" code generation very similar but didn't try to factor/unify them. - Mark several tests as RequiresProcessIsolation so that their gcstress is kept separate from the rest of the tests. Disables a large test under gcstress. - Add gcstress striping to some merged groups. Should fix #85590
1 parent 528ed0d commit fb0b206

File tree

18 files changed

+145
-49
lines changed

18 files changed

+145
-49
lines changed

‎src/tests/Common/XUnitWrapperGenerator/XUnitWrapperGenerator.cs‎

Lines changed: 88 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -166,30 +166,43 @@ private static string GenerateFullTestRunner(ImmutableArray<ITestInfo> testInfos
166166
CodeBuilder builder = new();
167167
AppendAliasMap(builder, aliasMap);
168168

169-
builder.AppendLine("System.Collections.Generic.HashSet<string> testExclusionList = XUnitWrapperLibrary.TestFilter.LoadTestExclusionList();");
169+
builder.AppendLine("XUnitWrapperLibrary.TestFilter filter;");
170+
builder.AppendLine("XUnitWrapperLibrary.TestSummary summary;");
171+
builder.AppendLine("System.Diagnostics.Stopwatch stopwatch;");
172+
builder.AppendLine("XUnitWrapperLibrary.TestOutputRecorder outputRecorder;");
170173
builder.AppendLine();
171174

172-
builder.AppendLine($@"if (System.IO.File.Exists(""{assemblyName}.tempLog.xml""))");
175+
builder.AppendLine("void Initialize()");
173176
using (builder.NewBracesScope())
174177
{
175-
builder.AppendLine($@"System.IO.File.Delete(""{assemblyName}.tempLog.xml"");");
176-
}
177-
builder.AppendLine($@"if (System.IO.File.Exists(""{assemblyName}.testStats.csv""))");
178-
using (builder.NewBracesScope())
179-
{
180-
builder.AppendLine($@"System.IO.File.Delete(""{assemblyName}.testStats.csv"");");
178+
builder.AppendLine("System.Collections.Generic.HashSet<string> testExclusionList = XUnitWrapperLibrary.TestFilter.LoadTestExclusionList();");
179+
builder.AppendLine();
180+
181+
builder.AppendLine($@"if (System.IO.File.Exists(""{assemblyName}.tempLog.xml""))");
182+
using (builder.NewBracesScope())
183+
{
184+
builder.AppendLine($@"System.IO.File.Delete(""{assemblyName}.tempLog.xml"");");
185+
}
186+
builder.AppendLine($@"if (System.IO.File.Exists(""{assemblyName}.testStats.csv""))");
187+
using (builder.NewBracesScope())
188+
{
189+
builder.AppendLine($@"System.IO.File.Delete(""{assemblyName}.testStats.csv"");");
190+
}
191+
builder.AppendLine();
192+
193+
builder.AppendLine("filter = new (args, testExclusionList);");
194+
builder.AppendLine("summary = new();");
195+
builder.AppendLine("stopwatch = System.Diagnostics.Stopwatch.StartNew();");
196+
builder.AppendLine("outputRecorder = new(System.Console.Out);");
197+
builder.AppendLine("System.Console.SetOut(outputRecorder);");
181198
}
182199
builder.AppendLine();
183200

184-
builder.AppendLine("XUnitWrapperLibrary.TestFilter filter = new (args, testExclusionList);");
185-
builder.AppendLine("XUnitWrapperLibrary.TestSummary summary = new();");
186-
builder.AppendLine("System.Diagnostics.Stopwatch stopwatch = System.Diagnostics.Stopwatch.StartNew();");
187-
builder.AppendLine("XUnitWrapperLibrary.TestOutputRecorder outputRecorder = new(System.Console.Out);");
188-
builder.AppendLine("System.Console.SetOut(outputRecorder);");
189-
builder.AppendLine();
201+
builder.AppendLine("Initialize();");
190202

203+
// Open the stream writer for the temp log.
191204
builder.AppendLine($@"using (System.IO.StreamWriter tempLogSw = System.IO.File.AppendText(""{assemblyName}.tempLog.xml""))");
192-
builder.AppendLine($@"using (System.IO.StreamWriter statsCsvSw = System.IO.File.AppendText(""{assemblyName}.testStats.csv"")){{");
205+
builder.AppendLine($@"using (System.IO.StreamWriter statsCsvSw = System.IO.File.AppendText(""{assemblyName}.testStats.csv""))");
193206
CodeBuilder testExecutorBuilder = new();
194207
int totalTestsEmitted = 0;
195208

@@ -209,9 +222,14 @@ private static string GenerateFullTestRunner(ImmutableArray<ITestInfo> testInfos
209222

210223
if (testInfos.Length > 0)
211224
{
212-
// Break tests into groups of 50 so that we don't create an unreasonably large main method
213-
// Excessively large methods are known to take a long time to compile, and use excessive stack
214-
// leading to test failures.
225+
// This code breaks the tests into groups called by helper methods.
226+
//
227+
// Reasonably large methods are known to take a long time to compile, and use excessive stack
228+
// leading to test failures. Groups of 50 were sufficient to avoid this problem.
229+
//
230+
// However, large methods also appear to causes problems when the tests are run in gcstress
231+
// modes. Groups of 1 appear to help with this. It hasn't been directly measured but
232+
// experimentally has improved gcstress testing.
215233
foreach (ITestInfo test in testInfos)
216234
{
217235
if (testsLeftInCurrentTestExecutor == 0)
@@ -224,12 +242,14 @@ private static string GenerateFullTestRunner(ImmutableArray<ITestInfo> testInfos
224242
}
225243

226244
currentTestExecutor++;
227-
testExecutorBuilder.AppendLine($"void TestExecutor{currentTestExecutor}(System.IO.StreamWriter tempLogSw, System.IO.StreamWriter statsCsvSw)");
245+
testExecutorBuilder.AppendLine($"void TestExecutor{currentTestExecutor}("
246+
+ "System.IO.StreamWriter tempLogSw, "
247+
+ "System.IO.StreamWriter statsCsvSw)");
228248
testExecutorBuilder.AppendLine("{");
229249
testExecutorBuilder.PushIndent();
230250

231251
builder.AppendLine($"TestExecutor{currentTestExecutor}(tempLogSw, statsCsvSw);");
232-
testsLeftInCurrentTestExecutor = 50; // Break test executors into groups of 50, which empirically seems to work well
252+
testsLeftInCurrentTestExecutor = 1; // Break test executors into groups of 1, which empirically seems to work well
233253
}
234254
else
235255
{
@@ -246,21 +266,27 @@ private static string GenerateFullTestRunner(ImmutableArray<ITestInfo> testInfos
246266
testExecutorBuilder.AppendLine();
247267
}
248268

249-
testExecutorBuilder.AppendLine("}");
250-
builder.AppendLine("tempLogSw.WriteLine(\"</assembly>\");");
269+
builder.AppendLine("summary.WriteFooterToTempLog(tempLogSw);");
251270
}
252271
builder.AppendLine();
253272

254-
builder.AppendLine($@"string testResults = summary.GetTestResultOutput(""{assemblyName}"");");
255-
builder.AppendLine($@"string workitemUploadRoot = System.Environment.GetEnvironmentVariable(""HELIX_WORKITEM_UPLOAD_ROOT"");");
256-
builder.AppendLine($@"if (workitemUploadRoot != null)");
273+
builder.AppendLine("void Finish()");
257274
using (builder.NewBracesScope())
258275
{
259-
builder.AppendLine($@"System.IO.File.WriteAllText(System.IO.Path.Combine(workitemUploadRoot, ""{assemblyName}.testResults.xml.txt""), testResults);");
276+
builder.AppendLine($@"string testResults = summary.GetTestResultOutput(""{assemblyName}"");");
277+
builder.AppendLine($@"string workitemUploadRoot = System.Environment.GetEnvironmentVariable(""HELIX_WORKITEM_UPLOAD_ROOT"");");
278+
builder.AppendLine($@"if (workitemUploadRoot != null)");
279+
using (builder.NewBracesScope())
280+
{
281+
builder.AppendLine($@"System.IO.File.WriteAllText(System.IO.Path.Combine(workitemUploadRoot, ""{assemblyName}.testResults.xml.txt""), testResults);");
282+
}
283+
builder.AppendLine();
284+
285+
builder.AppendLine($@"System.IO.File.WriteAllText(""{assemblyName}.testResults.xml"", testResults);");
260286
}
261-
builder.AppendLine();
262287

263-
builder.AppendLine($@"System.IO.File.WriteAllText(""{assemblyName}.testResults.xml"", testResults);");
288+
builder.AppendLine();
289+
builder.AppendLine("Finish();");
264290
builder.AppendLine("return 100;");
265291
builder.AppendLine();
266292

@@ -275,12 +301,15 @@ private static string GenerateXHarnessTestRunner(ImmutableArray<ITestInfo> testI
275301
CodeBuilder builder = new();
276302
AppendAliasMap(builder, aliasMap);
277303

278-
builder.AppendLine("System.Collections.Generic.HashSet<string> testExclusionList = XUnitWrapperLibrary.TestFilter.LoadTestExclusionList();");
304+
builder.AppendLine("XUnitWrapperLibrary.TestSummary summary;");
305+
builder.AppendLine("System.Diagnostics.Stopwatch stopwatch;");
306+
builder.AppendLine("XUnitWrapperLibrary.TestOutputRecorder outputRecorder;");
279307
builder.AppendLine();
280308

281309
builder.AppendLine("try");
282310
using (builder.NewBracesScope())
283311
{
312+
builder.AppendLine("System.Collections.Generic.HashSet<string> testExclusionList = XUnitWrapperLibrary.TestFilter.LoadTestExclusionList();");
284313
builder.AppendLine($@"return await XHarnessRunnerLibrary.RunnerEntryPoint.RunTests(RunTests, ""{assemblyName}"", args.Length != 0 ? args[0] : null, testExclusionList);");
285314
}
286315
builder.AppendLine("catch(System.Exception ex)");
@@ -291,15 +320,9 @@ private static string GenerateXHarnessTestRunner(ImmutableArray<ITestInfo> testI
291320
}
292321
builder.AppendLine();
293322

294-
builder.AppendLine("static XUnitWrapperLibrary.TestSummary RunTests(XUnitWrapperLibrary.TestFilter filter)");
323+
builder.AppendLine("void Initialize()");
295324
using (builder.NewBracesScope())
296325
{
297-
builder.AppendLine("XUnitWrapperLibrary.TestSummary summary = new();");
298-
builder.AppendLine("System.Diagnostics.Stopwatch stopwatch = new();");
299-
builder.AppendLine("XUnitWrapperLibrary.TestOutputRecorder outputRecorder = new(System.Console.Out);");
300-
builder.AppendLine("System.Console.SetOut(outputRecorder);");
301-
builder.AppendLine();
302-
303326
builder.AppendLine($@"if (System.IO.File.Exists(""{assemblyName}.tempLog.xml""))");
304327
using (builder.NewBracesScope())
305328
{
@@ -312,24 +335,43 @@ private static string GenerateXHarnessTestRunner(ImmutableArray<ITestInfo> testI
312335
}
313336
builder.AppendLine();
314337

315-
ITestReporterWrapper reporter = new WrapperLibraryTestSummaryReporting("summary", "filter", "outputRecorder");
338+
builder.AppendLine("summary = new();");
339+
builder.AppendLine("stopwatch = new();");
340+
builder.AppendLine("outputRecorder = new(System.Console.Out);");
341+
builder.AppendLine("System.Console.SetOut(outputRecorder);");
342+
}
343+
builder.AppendLine();
316344

317-
CodeBuilder testExecutorBuilder = new();
318-
int testsLeftInCurrentTestExecutor = 0;
319-
int currentTestExecutor = 0;
345+
builder.AppendLine("XUnitWrapperLibrary.TestSummary RunTests(XUnitWrapperLibrary.TestFilter filter)");
346+
using (builder.NewBracesScope())
347+
{
348+
builder.AppendLine("Initialize();");
320349

321350
// Open the stream writer for the temp log.
322351
builder.AppendLine($@"using (System.IO.StreamWriter tempLogSw = System.IO.File.AppendText(""{assemblyName}.templog.xml""))");
323352
builder.AppendLine($@"using (System.IO.StreamWriter statsCsvSw = System.IO.File.AppendText(""{assemblyName}.testStats.csv""))");
353+
CodeBuilder testExecutorBuilder = new();
354+
324355
using (builder.NewBracesScope())
325356
{
326357
builder.AppendLine($"statsCsvSw.WriteLine(\"{testInfos.Length},0,0,0\");");
327358

359+
ITestReporterWrapper reporter =
360+
new WrapperLibraryTestSummaryReporting("summary", "filter", "outputRecorder");
361+
362+
int testsLeftInCurrentTestExecutor = 0;
363+
int currentTestExecutor = 0;
364+
328365
if (testInfos.Length > 0)
329366
{
330-
// Break tests into groups of 50 so that we don't create an unreasonably large main method
331-
// Excessively large methods are known to take a long time to compile, and use excessive stack
332-
// leading to test failures.
367+
// This code breaks the tests into groups called by helper methods.
368+
//
369+
// Reasonably large methods are known to take a long time to compile, and use excessive stack
370+
// leading to test failures. Groups of 50 were sufficient to avoid this problem.
371+
//
372+
// However, large methods also appear to causes problems when the tests are run in gcstress
373+
// modes. Groups of 1 appear to help with this. It hasn't been directly measured but
374+
// experimentally has improved gcstress testing.
333375
foreach (ITestInfo test in testInfos)
334376
{
335377
if (testsLeftInCurrentTestExecutor == 0)
@@ -342,18 +384,15 @@ private static string GenerateXHarnessTestRunner(ImmutableArray<ITestInfo> testI
342384
}
343385

344386
currentTestExecutor++;
345-
testExecutorBuilder.AppendLine($"static void TestExecutor{currentTestExecutor}("
346-
+ "XUnitWrapperLibrary.TestSummary summary, "
387+
testExecutorBuilder.AppendLine($"void TestExecutor{currentTestExecutor}("
347388
+ "XUnitWrapperLibrary.TestFilter filter, "
348-
+ "XUnitWrapperLibrary.TestOutputRecorder outputRecorder, "
349-
+ "System.Diagnostics.Stopwatch stopwatch, "
350389
+ "System.IO.StreamWriter tempLogSw, "
351390
+ "System.IO.StreamWriter statsCsvSw)");
352391
testExecutorBuilder.AppendLine("{");
353392
testExecutorBuilder.PushIndent();
354393

355-
builder.AppendLine($"TestExecutor{currentTestExecutor}(summary, filter, outputRecorder, stopwatch, tempLogSw, statsCsvSw);");
356-
testsLeftInCurrentTestExecutor = 50; // Break test executors into groups of 50, which empirically seems to work well
394+
builder.AppendLine($"TestExecutor{currentTestExecutor}(filter, tempLogSw, statsCsvSw);");
395+
testsLeftInCurrentTestExecutor = 1; // Break test executors into groups of 1, which empirically seems to work well
357396
}
358397
else
359398
{

‎src/tests/Common/XUnitWrapperLibrary/TestSummary.cs‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -115,6 +115,11 @@ public void WriteHeaderToTempLog(string assemblyName, StreamWriter tempLogSw)
115115
+ $" run-date-time=\"{_testRunStart.ToString("yyyy-MM-dd HH:mm:ss")}\">");
116116
}
117117

118+
public void WriteFooterToTempLog(StreamWriter tempLogSw)
119+
{
120+
tempLogSw.WriteLine("</assembly>");
121+
}
122+
118123
public void ReportPassedTest(string name,
119124
string containingTypeName,
120125
string methodName,

‎src/tests/JIT/jit64/opt/cg/cgstress/CgStress1_do.csproj‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,8 @@
11
<Project Sdk="Microsoft.NET.Sdk">
22
<PropertyGroup>
3+
<!-- Only needed to run GCStress out-of-proc -->
4+
<RequiresProcessIsolation>true</RequiresProcessIsolation>
5+
36
<DebugType>Full</DebugType>
47
<Optimize>True</Optimize>
58
</PropertyGroup>

‎src/tests/JIT/jit64/opt/cg/cgstress/CgStress1_ro.csproj‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,8 @@
11
<Project Sdk="Microsoft.NET.Sdk">
22
<PropertyGroup>
3+
<!-- Only needed to run GCStress out-of-proc -->
4+
<RequiresProcessIsolation>true</RequiresProcessIsolation>
5+
36
<DebugType>None</DebugType>
47
<Optimize>True</Optimize>
58
</PropertyGroup>

‎src/tests/JIT/jit64/opt/cg/cgstress/CgStress2_d.csproj‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,8 @@
11
<Project Sdk="Microsoft.NET.Sdk">
22
<PropertyGroup>
3+
<!-- Only needed to run GCStress out-of-proc -->
4+
<RequiresProcessIsolation>true</RequiresProcessIsolation>
5+
36
<DebugType>Full</DebugType>
47
<Optimize>False</Optimize>
58
</PropertyGroup>

‎src/tests/JIT/jit64/opt/cg/cgstress/CgStress2_do.csproj‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,8 @@
11
<Project Sdk="Microsoft.NET.Sdk">
22
<PropertyGroup>
3+
<!-- Only needed to run GCStress out-of-proc -->
4+
<RequiresProcessIsolation>true</RequiresProcessIsolation>
5+
36
<DebugType>Full</DebugType>
47
<Optimize>True</Optimize>
58
</PropertyGroup>

‎src/tests/JIT/jit64/opt/cg/cgstress/CgStress2_r.csproj‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,8 @@
11
<Project Sdk="Microsoft.NET.Sdk">
22
<PropertyGroup>
3+
<!-- Only needed to run GCStress out-of-proc -->
4+
<RequiresProcessIsolation>true</RequiresProcessIsolation>
5+
36
<DebugType>None</DebugType>
47
<Optimize>False</Optimize>
58
</PropertyGroup>

‎src/tests/JIT/jit64/opt/cg/cgstress/CgStress2_ro.csproj‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,8 @@
11
<Project Sdk="Microsoft.NET.Sdk">
22
<PropertyGroup>
3+
<!-- Only needed to run GCStress out-of-proc -->
4+
<RequiresProcessIsolation>true</RequiresProcessIsolation>
5+
36
<DebugType>None</DebugType>
47
<Optimize>True</Optimize>
58
</PropertyGroup>

‎src/tests/JIT/jit64/opt/cg/cgstress/CgStress3_d.csproj‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,8 @@
11
<Project Sdk="Microsoft.NET.Sdk">
22
<PropertyGroup>
3+
<!-- Only needed to run GCStress out-of-proc -->
4+
<RequiresProcessIsolation>true</RequiresProcessIsolation>
5+
36
<DebugType>Full</DebugType>
47
<Optimize>False</Optimize>
58
</PropertyGroup>

‎src/tests/JIT/jit64/opt/cg/cgstress/CgStress3_do.csproj‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,8 @@
11
<Project Sdk="Microsoft.NET.Sdk">
22
<PropertyGroup>
3+
<!-- Only needed to run GCStress out-of-proc -->
4+
<RequiresProcessIsolation>true</RequiresProcessIsolation>
5+
36
<DebugType>Full</DebugType>
47
<Optimize>True</Optimize>
58
</PropertyGroup>

0 commit comments

Comments
 (0)