Repository navigation
ci(tensilelite): run the hw-common tests with pytest-xdist worksteal - #15251
Alex-Vasile wants to merge 1 commit into
Conversation
github-actions
Bot
added
project: hipblaslt
project: hipsparselt
ci:hipsparselt-fast
labels
✅ All Checks Passed — Ready for Review
📖 Need help? See the Policy FAQ for details on every check and how to fix failures. |
|
🎉 All checks passed! This PR is ready for review. |
fd6736a to
2abe42c
Compare
Alex-Vasile
removed
NoCI
Adds --dist=worksteal to the hw-common category of test_categories.yaml (TheRock's tensilelite-common job) so idle xdist workers take the queued expensive configs of busy ones. Measured on the CI-like build stage; scheduling only.
2abe42c to
59bade4
Compare
tony-davis
left a comment
tony-davis
left a comment
There was a problem hiding this comment.
Hi Alex! I'm really happy to see this stack. A real speedup for the common tests is something I think a lot of folks will feel right away, and the descriptions make the layers easy to follow.
This one looks good to me. I checked that TheRock's pytest_runner.py appends pytest_args as-is next to --numprocesses, and that develop already requires pytest-xdist>=3.2 (#14798), so --dist=worksteal should reach the hw-common job the way you describe. The stack is one commit behind develop with no file overlap, so I don't see a rebase concern.
A few small things, none blocking:
- The CI evidence is still pending. The Linux Multi-Arch stages hadn't reached the
tensilelite-commonjob when I looked, and that's the only place this change does anything. Once it runs on gfx942 and gfx950, I'd love to see the wall time next to a recent develop run in the Test Result. The 475 s number is one run of the build-only stage on a different tree, so I bet the combined build-and-run number will be the one people quote later. - Two comments explain the same thing. The block at the top of
test_categories.yamlalready explainsworkstealforquickandstandard. I think extending that block to sayhw-commonuses it too (for the expensive-config reason) would keep the explanation in one place. Small thing. - The
therock-pr-botred is stale. It ran at 18:38, saw a pre-commit run that was cancelled by the 18:43 push, and hasn't re-evaluated.pre-commitpasses on the current head. A bot re-run should clear theNot ready to Reviewlabel on the PRs above this one.
For the description, I'd suggest leading the Test Result with what was measured and where, then the number. Something like:
Measured on the original unsplit branch, not this one, on a gfx942 (MI300X) host bound to 25 cores. Only the build stage was timed (
--build-only --gpu-targets gfx942,-n 4, cache cleared).workstealtook 475 s in one run. The defaultloadscheduler took 566 s and 562 s in two runs, so about 16% less wall time. Idle worker time fell from about 516 to 123 worker-seconds. All runs had 145 passes and 1 known failure (comm/gfx950/fused_a2a). That tree also deselected 146 configs that opt out of the target architecture, which this branch does not.The run stage (
--use-cache) did not change, because a per-GPU lock serializes client runs. Thehw-commonjob builds and runs in one pass, and that combined mode was not measured. Automated GitHub checks are pending.
Thanks for doing this!
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #15251 +/- ##
===========================================
- Coverage 78.10% 78.10% -0.00%
===========================================
Files 3007 3007
Lines 522272 522272
Branches 81514 81514
===========================================
- Hits 407876 407874 -2
Misses 85449 85449
- Partials 28947 28949 +2
*This pull request uses carry forward flags. Click here to find out more. 🚀 New features to boost your workflow:
|
JIRA ID : AIHPBLAS-5255
Motivation
TensileLite's common tests generate, build and run real GEMM (matrix multiply) kernels for a set of YAML test configs. TheRock CI runs them on GPU runners as the
hw-commontest category, with pytest-xdist, the pytest plugin that runs tests in parallel worker processes, and 4 workers. xdist's defaultloadscheduler hands tests out in chunks. The most expensive configs (dtv,cgemm,zgemm,bufferLoadStore0and others) sit next to each other in collection order, so they land on one worker while the other workers go idle.Technical Details
The
hw-commoncategory intest_categories.yamlgains--dist=workstealin itspytest_args, with a comment that gives the reason. Withworksteal, an idle worker takes queued tests from a busy one.parallel_workersstays 4. TheRock'spytest_runner.pyreads this file; the change affects only that job.This is CI configuration only. It changes which worker runs which test, not which tests run or what they check. No product code, test code or device behavior changes. The other categories, including the FFM (emulation) categories, keep their current settings.
Test Plan
From
projects/hipblaslt/tensilelite:pre-commit run --files test_categories.yamlchecks the YAML against the repository hooks.hw-commonjob on this PR exercises the new scheduler. Compare its pass/fail set and wall time with a recentdeveloprun.Test Result
Time saved: 87 to 91 s of wall time (15.5% to 16.1%) in the CI-like build stage of the common tests (
--build-only --gpu-targets gfx942,-n 4, cache cleared first), withworkstealat 475 s (1 run) against the defaultloadscheduler at 566 and 562 s (2 runs). Both were measured on the original unsplit branch with the same tree, on a gfx942 (MI300X) host with the process bound to 25 cores andOMP_NUM_THREADS=25. That tree also deselected configs that opt out of the target architecture (146 tests), which this branch does not include. The run stage (--use-cache) saved nothing.load(default)load --maxschedchunk=1load --maxschedchunk=2load --maxschedchunk=4workstealIdle worker time was 123 worker-seconds with
workstealagainst about 516 withload. Every run had the same outcome: 145 passed and 1 known failure (comm/gfx950/fused_a2a). The run stage took 747 and 736 s against 751 and 746 s, because a per-GPU lock serializes client runs. Thehw-commonjob builds and runs in one pass, and that combined mode was not measured.Not yet run on this branch. Automated GitHub checks are pending.
Related
--dist=worksteal; this PR applies the same scheduler tohw-common.pytest-xdist>=3.2, the first version withworksteal.Device / Architecture Coverage
CI configuration only; no kernel, client or GPU behavior changes. Passing PR checks are sufficient, provided they include the TheRock
hw-commonjob that reads this setting.Submission Checklist
Risk level
Low (2/5): changes only how the
hw-commonjob distributes tests across its 4 workers; test selection and verdicts are unchanged.workstealneeds pytest-xdist 3.2 or later, whichdevelopalready requires.