Repository navigation
Conversation
The init_cuda teardown's per-test gc.collect() (added by NVIDIA#2484) gets superlinearly slower as a pytest session proceeds: pytest-repeat's --count=N expands the collected item list N-fold up front, and the executed-item bookkeeping (TestReport objects, and since the JUnit logger was added, its XML nodes) accumulates for the life of the session. gc.collect() walks that whole live heap on every call, so nightly's 5 repeats measured at ~10x one repeat, not ~5x (see the "cuda.core test time" investigation referenced from NVIDIA#3041 and NVIDIA#3045). NVIDIA#3041 addressed the first term with one gc.freeze() after collection. This change addresses the session-length growth directly: ci/tools/ pytest_batched.py collects the full, already---count-expanded item list once via `pytest --collect-only`, then runs it across several pytest subprocesses of at most --batch-size items each. Each subprocess starts with an empty heap, so neither accumulation term grows past one batch, regardless of how large --count or the suite gets. The per-batch JUnit XML files are merged back into a single file at the original --junitxml path, so the script is a drop-in replacement for the plain pytest invocation it replaces in ci/tools/run-tests: same CLI, same final junit-core.xml (verified byte-for-byte equal `tests`/`failures`/`testcase` counts against an unbatched run of the same suite), same aggregate exit code. --runner-prefix forwards SANITIZER_CMD to each batch. Scoped to the "Run cuda.core tests" step only (ci/tools/run-tests), since that is the step the investigation identified as the bottleneck; bindings/pathfinder/cython invocations are untouched. ci/tools/tests/test_pytest_batched.py covers the pure helpers (ID parsing, chunking, path stripping, JUnit merging) and five end-to-end subprocess scenarios, including the two non-obvious correctness requirements this script depends on: - `--collect-only -q`'s flat ID format needs a verbosity this script controls exactly; a caller's `-v` (CI always passes one) would otherwise partially cancel the `-q` pytest adds internally. - PYTEST_ADDOPTS (e.g. --count=N) must stay active for the batch runs, not just collection: a repeated item's node ID only exists once pytest-repeat's hook re-parametrizes the module, which requires --count to still be in effect, even though pytest still only runs the items named explicitly on the command line. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
Contributor
Contributor
Author
|
Closing: #3053 removed the superlinear repeat scaling this targeted. Real nightly data (linux-64/py3.12/l4, "Run cuda.core tests"):
Ratio now under 5x means no superlinear residual left to recover. A synthetic benchmark confirms batching is net-negative once gc.collect() frequency drops this low (subprocess startup overhead dominates). No further action needed here. |
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
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.
Description
Related to the "cuda.core test time" investigation discussed in #3041 and #3045 (no dedicated issue filed yet).
The
init_cudateardown's per-testgc.collect()(added by #2484) gets superlinearly slower as a pytest session proceeds. Two things accumulate for the life of a single session and are walked by everygc.collect():pytest-repeat's--count=N(used on nightly runs) expands the collected item list N-fold up front.TestReportobjects, and (since--junitxmlentered the CI command) the JUnit XML node per item — grows as the run proceeds.Measured effect: nightly's 5 repeats cost ~10x one repeat, not ~5x. #3041 addressed term 1 with a single
gc.freeze()right after collection. This PR addresses term 2 (and keeps term 1 bounded too) by capping how much of the suite any one pytest process ever sees.ci/tools/pytest_batched.pycollects the full, already---count-expanded item list once viapytest --collect-only, then runs it across several pytest subprocesses of at most--batch-sizeitems each (default 1000, overridable via--batch-sizeor$PYTEST_BATCH_SIZE). Each subprocess starts with an empty heap, so neither accumulation term grows past one batch, regardless of how large--countor the suite gets.The per-batch JUnit XML files are merged back into a single file at the original
--junitxmlpath, so the script is a drop-in replacement for the plainpytestinvocation it replaces inci/tools/run-tests: same CLI, same finaljunit-core.xml(verified byte-for-byte equaltests/failures/testcasecounts against an unbatched run ofci/tools/testsitself), same aggregate exit code.--runner-prefixforwardsSANITIZER_CMDto each batch's pytest invocation.Scoped to the "Run cuda.core tests" step only (
ci/tools/run-tests), since that is the step the investigation identified as the bottleneck (cuda.bindings stayed flat). Thebindings/pathfinder/cythoninvocations are untouched.Two non-obvious correctness requirements this script depends on, each covered by a dedicated regression test:
pytest --collect-only -q's flat, one-ID-per-line format needs a verbosity this script controls exactly. pytest sums-v/-qoccurrences rather than letting the last one win, so a caller-supplied-v(CI always passes one) would otherwise partially cancel the-qthis script adds internally, silently switching to the indented tree format and collecting zero node IDs.PYTEST_ADDOPTS(e.g.--count=N) must stay active for the batch runs, not just for collection. A repeated item's node ID (test_foo[1-3]) only exists oncepytest-repeat'spytest_generate_testshook re-parametrizes the module, which requires--countto still be in effect — even though pytest still only runs the items named explicitly on the command line, so this does not re-execute anything beyond what each batch names.Verification (CPU-only, no GPU needed for this script itself):
ci/tools/tests/test_pytest_batched.py: 18 unit tests for the pure helpers (node-ID parsing, batching, path stripping,--junitxmlextraction, JUnit merging) and 7 end-to-end subprocess tests (multi-batch execution, failure propagation, empty-collection handling,--countinteraction, the-v/-qregression).python -m pytest -v --noconftest ci/tools/tests(the suite CI runs) passes: 136 passed, 36 subtests passed — unchanged from before this PR.pytest_batched.pyagainstci/tools/testsitself with the exact flag setci/tools/run-testsuses (-rsxXfE -v --randomly-dont-reorganize --exclude-warning-annotations --junitxml ...) and confirmed the mergedjunit-*.xml'stests/failures/testcasecounts exactly match an unbatched run of the same suite (both reporttests="172",testcasecount 136 — the gap is pytest's own pre-existingpytest-subtestscounting quirk, reproduced identically, not something this PR introduces).This has not been verified against the real
cuda_coresuite under CUDA/GPU CI; that needs a CI run on this PR.Checklist