Repository navigation
[OMNIML-6178] [2/11] Use the sysmon coverage core in CI - #2846
shengliangxu wants to merge 1 commit into
Conversation
|
Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually. Contributors can view more details about this message here. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📝 Walkthrough
Merge Risk: ⚪ Minimal · up to
|
| Check name | Status | Explanation |
|---|---|---|
| Description Check |  |
Check skipped - CodeRabbit’s high-level summary is enabled. |
| Title check | |
The title clearly and concisely describes the main change: enabling the sysmon coverage core in CI workflows. |
| Docstring Coverage | |
No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 0… |
| Linked Issues check | |
Check skipped because no linked issues were found for this pull request. |
| Out of Scope Changes check | |
Check skipped because no linked issues were found for this pull request. |
| Security Anti-Patterns | |
PASS. The pull request changes only three GitHub Actions workflow files and adds COVERAGE_CORE=sysmon environment settings. It does not change modelopt or examples Python files, dependency manif… |
Full details: Docstring Coverage
-
Explanation
No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 0 files. (3 skipped: 3 unsupported.)
Full details: Security Anti-Patterns
-
Explanation
PASS. The pull request changes only three GitHub Actions workflow files and adds
COVERAGE_CORE=sysmonenvironment settings. It does not changemodeloptorexamplesPython files, dependency manifests, or security-sensitive loading or execution code. Therefore, none of the listed security anti-patterns is introduced.
✨ Finishing Touches: 💡 1
-
🛠️ Fix failing CI checks: 💡
-
- Commit to this branch
- Create a new PR
🧪 Generate unit tests (beta)
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Comment @coderabbitai help to get the list of available commands.
shengliangxu
changed the title
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## shengliangx/ci-1-unit-xdist #2846 +/- ##
===============================================================
+ Coverage 76.53% 79.10% +2.57%
===============================================================
Files 658 658
Lines 73950 73950
===============================================================
+ Hits 56594 58498 +1904
+ Misses 17356 15452 -1904
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
/ok to test 1c8d165 |
shengliangxu
changed the title
1c8d165 to
73d351b
Compare
shengliangxu
changed the title
|
/ok to test 73d351b |
shengliangxu
changed the title
Python 3.12 defaults to coverage's C tracer, which slows every line of Python that runs under it. Every lane starts coverage in each spawned worker and subprocess through COVERAGE_PROCESS_START, so the import-heavy start-ups pay for it: the 76 s Megatron worker-pool start, the 22 s hf_ptq processes of the unified-export tests, the puzzletron spawns, the vLLM engine core. COVERAGE_CORE=sysmon records the same lines through sys.monitoring at a fraction of the overhead. Measured (the first three here, the last by #2727 on its hf_ptq example lane): - serial tests/unit run on 4 CPUs: 572 s without coverage, 825 s with the C tracer, 640 s with sysmon; - `import modelopt.torch.quantization.extensions` with this repo's coverage config: 17.1 s native, 23.6 s with the C tracer, 18.3 s with sysmon, and the child process of a spawn writes its data file too, with the same 362 files and 13,821 executed lines; - one TinyLlama fp8 plus TRT-LLM deploy test: 64.8 s without coverage, 103.3 s with the C tracer, 70.5 s with sysmon, identical covered lines. Applies to the unit `linux` job (Python 3.12), gpu_tests.yml and regression_tests.yml. The example runner is left to #2727, which makes the same change there. sysmon needs Python 3.12 or newer; on an older interpreter coverage warns and keeps its default core, so a lane that turns out to run an older Python is slower, not broken. The gpu and gpu_megatron logs show 3.12. Testing: actionlint is clean on all workflows. Not yet run on a real CI runner; the first run should be compared against the previous coverage.xml. Signed-off-by: Shengliang Xu <shengliangx@nvidia.com>
73d351b to
2574994
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
This branch has not been deployed
1 participant
What does this PR do?
Type of change: CI (test runtime)
Sets
COVERAGE_CORE=sysmonon the unitlinuxjob,gpu_tests.ymlandregression_tests.yml. Python 3.12 defaults to coverage's C tracer, which slows every line of Python run under it, and every lane starts coverage in each spawned worker and subprocess throughCOVERAGE_PROCESS_START, so import-heavy start-ups pay for it (the 76 s Megatron pool start, the 22 s hf_ptq processes of the unified-export tests, the puzzletron spawns, the vLLM engine core). sysmon records the same lines throughsys.monitoring. The example runner is left to #2727, which makes the same change there. On a Python older than 3.12 coverage warns and keeps its default core, so a lane that turned out to run an older Python would be slower, not broken (the gpu and gpu_megatron logs show 3.12).Time improvement (measured in CI)
pytest time of each lane in the run of #2845 (the base of this PR) and in this PR's run, one run each:
linux(xdist)Workflow wall time: GPU tests 59.2 -> 39.6 min, Regression 21.3 -> 15.2 min.
coverage.xml-only report change was dropped from this PR. The runs above are of its earlier head, which also had it. It showed no CI effect: the pytest summary line came 1-4 s after the last test in the [OMNIML-6178] [1/11] Run the unit tests in parallel with pytest-xdist #2845 runs, which still wrote the html report, and 0-3 s in this PR's runs (median 1.1 s over the 82 lane runs of the series), so the table is the sysmon core's. It would only have saved developers' local runs (about 25 s per pytest session).Method: every PR of the series has its own CI run, so "before" is the run of the PR(s) below that lack the change and "after" the runs that carry it. Times come from the job logs (
--durations, or the gap between consecutive result lines). Where runner speed matters, the tests that no PR of the series touches scale each run, and the raw and the scaled medians are both shown. One test varies by 15-30% from run to run on its own, so smaller differences are not evidence.Usage
Nothing user-facing.
Testing
Measured on this tree (tests/unit on 4 CPUs serial: 572 s without coverage, 825 s with the C tracer, 640 s with sysmon;
import modelopt.torch.quantization.extensions: 17.1 s native, 23.6 s with the C tracer, 18.3 s with sysmon, with the same 362 files and 13,821 executed lines in the spawned child's data file; one TinyLlama fp8 plus TRT-LLM deploy test: 64.8 s without coverage, 103.3 s with the C tracer, 70.5 s with sysmon, identical covered lines). Pre-commit and actionlint pass. The CI runs of the series are the check on real runners (time section above).Before your PR is "Ready for review"
Make sure you read and follow Contributor guidelines and your commits are signed (
git commit -s -S).Make sure you read and follow the Security Best Practices (e.g. avoiding hardcoded
trust_remote_code=True,torch.load(..., weights_only=False),pickle, etc.).CONTRIBUTING.md: no new dependencyAdditional Information
Slice 2 of 11 of the GPU-CI speed-up work, split out of the reference draft #2730 (not for merge; it holds the whole series with its measurements). Merge in order; each PR stands on its own and passes CI without the later ones.
Summary by CodeRabbit