Shrink large multidimensional array tests to avoid OOM killer#131332
Conversation
The Copy/Clear_LargeMultiDimensionalArray tests allocated an 8 GB short[2, 2_000_000_000] array but only skipped below 4 GB available, so ~8 GB machines passed the guard and were OOM-killed. Use a byte[2, 1_073_741_825] array (~2 GB) that still exceeds int.MaxValue elements, exercising the same native-sized index paths. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
|
Azure Pipelines: Successfully started running 3 pipeline(s). 13 pipeline(s) were filtered out due to trigger conditions. There may be pipelines that require an authorized user to comment /azp run to run. |
There was a problem hiding this comment.
Pull request overview
Adjusts System.Runtime.Tests OuterLoop coverage for large multi-dimensional arrays to avoid Helix OOM-killer terminations while still exercising the >int.MaxValue flattened-length code paths in Array.Copy/Array.Clear.
Changes:
- Reduce large MD array allocations by switching from
short[2, 2_000_000_000](~8 GB) tobyte[2, 1_073_741_825](~2 GB) while keeping flattened element count >int.MaxValue. - Recompute index literals used to validate
Array.Copy/Array.Clearbehavior at the boundary. - Clean up skip-guard formatting and fix a stray
$in a skip message.
Show a summary per file
| File | Description |
|---|---|
| src/libraries/System.Runtime/tests/System.Runtime.Tests/System/ArrayTests.cs | Shrinks large MD array test allocations, updates boundary index math, and cleans up skip messages to avoid OOM-killer failures. |
Copilot's findings
Comments suppressed due to low confidence (1)
src/libraries/System.Runtime/tests/System.Runtime.Tests/System/ArrayTests.cs:4955
- Same maintainability concern as Copy_LargeMultiDimensionalArray: the boundary values are correct but easy to accidentally desynchronize (dim1 length vs last-column index). Consider factoring them into named constants derived from int.MaxValue and reusing them throughout the method.
if (memoryInfo.TotalAvailableMemoryBytes < 4_000_000_000)
{
// The array below is ~2 GB; require headroom so the OOM killer
// doesn't terminate the process before the test completes.
throw new SkipTestException($"Prone to OOM killer. {memoryInfo.TotalAvailableMemoryBytes} is available.");
- Files reviewed: 1/1 changed files
- Comments generated: 1
Run the ~2 GB allocations in a child process so an OOM kill fails only the test instead of the whole run, and hoist the boundary length into a Dim1Length constant so the derived indices stay in sync. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
adamsitnik
left a comment
There was a problem hiding this comment.
LGTM, it's clearly an improvement. Please consider using RemoteExecutor as suggested by @vcsjones (it's supported on every platform except of WASM, tvOS, iOS, android and mac catalyst)
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Copilot's findings
Comments suppressed due to low confidence (1)
src/libraries/System.Runtime/tests/System.Runtime.Tests/System/ArrayTests.cs:4972
- Same as the
Copy_LargeMultiDimensionalArraycase: the large allocation now happens insideRemoteExecutor.Invokewithout translatingOutOfMemoryException(or a Linux SIGKILL/137) into a skip. That changes the failure mode from a skip to a test failure on memory-constrained or fragmented environments.
// Allocate in a child process so that if the OOM killer does fire it
// fails just this test instead of taking down the whole test run.
RemoteExecutor.Invoke(static () =>
{
byte[,] a = new byte[2, Dim1Length];
- Files reviewed: 1/1 changed files
- Comments generated: 1
Run the allocation with CheckExitCode disabled and map a managed OutOfMemoryException or a SIGKILL (137) from the OOM killer back to SkipTestException, so memory pressure skips instead of failing the test. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
|
@adamsitnik could this get another sign-off now that the remote executor is used? |
adamsitnik
left a comment
There was a problem hiding this comment.
LGTM!
@tannergooding could you please kick off the Outerloop CI leg to make sure these tests are passing (running them locally is also acceptable)? RemoteExecutor is sometimes giving me hard time when refactoring the code and using lambdas (IIRC it captures the compiler generated method name and asks its runner to load given dll, type and run this particular method)
The
Copy_LargeMultiDimensionalArrayandClear_LargeMultiDimensionalArrayOuterLoop tests each allocatedshort[2, 2_000_000_000]== 8 GB, but only skipped whenTotalAvailableMemoryBytes < 4_000_000_000. On the ~8 GB Debian.10/Debian.11 Helix queues the guard passes and the 8 GB allocation then trips the OOM killer (exit code 137), which is what #58616 is tracking.The tests exist to exercise the native-sized (
nuint/nint) index paths inArray.Copy/Array.Clear/element access, which only requires the flattened element count to exceedInt32.MaxValue. Element size is irrelevant to that boundary since the public API indices are allint. So switch tobyte[2, 1_073_741_825](~2 GB, total2_147_483_650elements) and recompute the row/col literals. The allocation now sits at half the unchanged 4 GB guard, so its headroom assumption actually holds, without narrowing where the tests run.Also removes a stale
~1GBcomment and a stray$in one skip message.Fixes #58616
Validated locally on Windows x64:
System.Runtime.Testscompiles clean, and both tests run (not skipped) withouterloop=true--Total: 2, Failed: 0, Skipped: 0.Note
This PR description was drafted by Copilot.