Repository navigation
[OMNIML-6178] Keep the cpu_dist_workers pool in one worker of the xdist unit run - #2953
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. |
|
Important Draft PR not reviewedDraft PRs are not automatically reviewed by default.
To automatically review draft PRs, update your CodeRabbit configuration: reviews:
auto_review:
drafts: true
Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2953 +/- ##
=======================================
Coverage 79.60% 79.60%
=======================================
Files 660 660
Lines 74667 74667
=======================================
Hits 59435 59435
Misses 15232 15232
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:
|
The multi-version torch_28 unit job is stopped by its runner ("The runner has received a shutdown
signal") on 94 of 489 attempts between 10-20 and 10-23, none with a failed test and no other job
affected. The ONNX conv/IsInf tests were in flight in 93 of the 94 stops, and the stop rate rises
with the number of xdist workers that hold a cpu_dist_workers pool when those tests start: 5 of 27
attempts with 2 pools, 50 of 270 with 3, 39 of 114 with 4. The pool (two gloo processes of about
0.7 GB) is session-scoped, so under `--dist loadfile` every worker that ran one of the files using
it keeps its own until the end.
`--dist loadgroup` schedules by the xdist_group marker and treats an unmarked test as a group of
its own. A tryfirst hook in tests/unit/conftest.py marks every file as a group (what loadfile
does), except that all files with a cpu_dist_workers test share one group, so one worker holds the
pool. unit_tests.yml passes `--dist loadgroup` in both places.
Testing: full tests/unit run, CI-like (Python 3.12, torch 2.14.1 CPU, xdist 3.8.0, -n 4, coverage
on, OMP_NUM_THREADS=1), peak of the summed RSS of the process tree sampled every 0.1 s, two runs
each, same tests passing in both modes. main 3a34d65 (5,353 passed): loadfile 14,024 / 14,126
MiB, loadgroup 10,832 / 10,294. hfep stack top a3370d2 with this change (5,830 passed):
loadfile 15,633 / 15,823, loadgroup 11,089 / 10,914. The pool files ran on one worker (243 tests
on main, 743 on the hfep tree) and no file was split. RSS counts shared pages, so the absolute
numbers overstate the real use. Not run on a torch 2.8 environment or a hosted runner.
Signed-off-by: Shengliang Xu <shengliangx@nvidia.com>
a7e9743 to
3d3cbea
Compare
|
Closing this draft.
The branch stays ( |
|
What does this PR do?
Type of change: CI improvement (memory of the unit lane); not a bug fix, and no speed gain was measurable on hosted runners
Puts all unit test files that use the
cpu_dist_workerspool into one xdist group (--dist loadgroup), so that one worker instead of up to four holds the pool. The fixture says its workers "start once" and are shared by every multi-rank test; under xdist (#2845) each worker started its own pool.Change.
--dist loadgroupschedules by thexdist_groupmarker and treats an unmarked test as a group of its own. Atryfirsthook intests/unit/conftest.pymarks every file as its own group (which is whatloadfiledoes), except that all files with acpu_dist_workerstest share one group, so one worker holds the pool.unit_tests.ymlpasses--dist loadgroupin both places.What it improves.
tests/unit(Python 3.12, torch 2.14.1 CPU,-n 4, coverage on): the peak of the summed RSS falls from 14,024-14,126 MiB to 10,294-10,832 MiB on main and from 15,633-15,823 MiB to 10,914-11,089 MiB on the hfep stack's top (two runs each, the same tests pass). As physical memory (PSS, shared pages divided among the processes that map them; wholetests/unitat c00ac43,-n 4, no coverage, torch 2.10 CUDA build): 12,984 MiB with 20 processes and 29 s above 10 GB become 9,836 MiB with 12 processes, never above 10 GB.loadfile) against 201 / 197 s (this PR) on main on a loaded 96-CPU host, 158 s against 144 s in another lab run; fewer pool startups, each importing torch and transformers in two processes.What it does not show: a speed gain on hosted runners.
linuxjob, pytest's own time (the summary line), all on runner image ubuntu-24.04 20261012.334.1: this PR 245 s median over 6 runs (190-291 s), other trees since #2950 242 s over 12 runs, the hfep/epdp stack trees 296 s over 49 runs (they have 9% more tests and three times as many pool tests). TheRun unit testsstep is shorter (312 s against 382 s for other trees), but it includes the environment install, which this change does not touch. Over the 12 unit jobs the per-job step times are 0.89 of the other trees' median (9 faster), and one run varies by 20% on its own, so a gain below about 10% is not visible with 6 runs.Context: the runner stops, which this does not fix. The multi-version
torch_28unit job was stopped mid-run by its runner ("The runner has received a shutdown signal") on 94 of 489 attempts between 10-20 and 10-23; no other unit job was. The ONNX conv/IsInf tests were in flight in 93 of the 94. #2947 shrank their input and merged on 10-23 at 00:47 UTC; since #2950 let CI run again (05:23 UTC), all 32 finishedtorch_28attempts passed (23 on the hfep/epdp stack, where 32% were stopped before on the same runner image; 4 on other branches; 5 of this PR's own). The pools raised the risk without being the trigger:So this PR is optional memory headroom (the margin of a 16 GB runner, about 14.5 GB usable, is thin through the whole run: PSS keeps climbing because the pools accumulate), and is a draft. Not run: a torch 2.8 environment.
Testing
Full
tests/unit, CI-like, peak of the summed RSS of the whole process tree sampled every 0.1 s, in MiB, two runs per cell; every run passes the same tests with and without the change.--dist loadfile--dist loadgroup(this PR)The committed tree itself (main c00ac43 + this commit; 5,355 passed, 31 skipped, 12 xfailed), one run: 10,232 MiB. The pool files ran on one worker (243 tests on main, 743 on the hfep tree) and no file was split across workers; the group takes 53 s of test time on main (68 s on the hfep tree), so it is not on the critical path. RSS counts shared pages, so the absolute numbers overstate the real use; the comparison is what matters.
On the hosted runners the unit workflow of this PR ran 5 times (attempt 1 and 4 re-runs of the same commit) plus one run of the commit before the rebase; all passed.
No test was added: the hook only sets scheduling markers, and the whole unit suite runs under it.
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
Jira: [OMNIML-6178]. Only files that use
cpu_dist_workersare grouped: another multi-process fixture or memory-heavy file added later is not covered.