Repository navigation
ci(gfx1250): drop the host-wide lock on the large hipBLASLt tests - #14982
tony-davis wants to merge 2 commits into
Conversation
9d569ad to
fc587c3
Compare
There was a problem hiding this comment.
🟡 Changes recommended
The workflow publishes unsanitized timing data, can under-report peak memory after measurement resets, and aborts when the timing file is missing even if all tests pass.
3 open issues
What changed in this PR
Removes host-wide serialization of gfx1250 hipBLASLt large-memory tests, relying on the container memory cap and client memory-management fixes from the prerequisite PRs.
Changes:
- Removes the shared lock and mount while retaining the four-shard phase followed by separate large-test groups.
- Makes memory skips fail the large-test phase and adds per-process memory reporting.
- Updates the lane documentation to describe execution order, failure behavior, and summaries.
| File | Description |
|---|---|
projects/hipblaslt/TESTING.md |
Documents large-test execution, memory-skip failures, and reporting. |
.github/workflows/gfx1250_hipblaslt_tensilelite_test.yml |
Removes host locking and adds strict memory-skip handling and memory summaries. |
Review effort: Balanced
✅ All Checks Passed — Ready for Review
📖 Need help? See the Policy FAQ for details on every check and how to fix failures. |
fc587c3 to
24fa690
Compare
dc74cfc to
9f7bcf1
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## users/tony-davis/AIHPBLAS-5352-memory-preflight #14982 +/- ##
====================================================================================
- Coverage 77.65% 70.05% -7.59%
====================================================================================
Files 2905 67 -2838
Lines 490744 22641 -468103
Branches 74315 3541 -70774
====================================================================================
- Hits 381040 15861 -365179
+ Misses 84683 5279 -79404
+ Partials 25021 1501 -23520
🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Thanks @tony-davis! Both #14610 asks are in (HIPBLASLT_TEST_FAIL_ON_MEMORY_SKIP=1 on the large phase, [ PEAK RSS ]). The large phase still runs after the shards, so dropping the lock can't raise a container's peak; what's left is host-wide.
First lock-free run 41717824185: red from the already tracked intermittent AmaxD case.
No lock wait, 0 kills, no [ MEMORY ]; [ PEAK RSS ] 15.3–38.8 GiB per shard, 2.2–49.9 per large group; TensileLite 216 passed / 0 failed.
All 4 runners were busy, but the large phase is ~8 of a ~50-min job: ours overlapped ~4 min with #14123's (41730286382, with the lock) while the other two GPUs were in TensileLite. So 2 at once, not yet 4.
Sources, all on the gfx1250 host: free -g and df -h /dev/shm (11-03); the job image under docker run --user 0:0 --memory 3g (11-04); our 10-28 probe of develop 2a5d491 before the stack (4 shards under /usr/bin/time -v, host memory sampled every 10 s).
Non-blocking:
-
Risk line: 4 × 110 GiB + ~36 GiB idle = 476 of 499 GiB, ~23 GiB margin. The caps hold for pinned buffers too: they're anonymous shared memory charged to the container (probe: 333.6 GiB shmem, more than
/dev/shmholds; 8.6 MB after each run), and/dev/shmhas 1.9 MB after the 11-02 kills. -
Test plan, if you want the 4-overlap run: same-branch dispatches share a concurrency group (
event_name-ref,cancel-in-progress, #L114-L115) and cancel each other (this PR's 41706411592/41706411814/41717823133), and staggered runs rarely line up anyway. To force it: push the head to 4 refs and, with the runners idle, dispatch all 4 withrun_tensilelite=falseandhipblaslt_gtest_filter='*fast_check_size_t_index_overflow*:*stress_matmul_size_t_index_overflow*:*size_t_output_overflow*:*offset_above_2_32*:*batch_offset_large*'. The shards run nothing, so the 4 large phases start after their 3–5 min builds and overlap for most of their ~8 min. -
To measure the shard race (fix 6 in AIHPBLAS-5352): the job container mounts
/sys/fs/cgroupread-only, somemory.peakcan't be reset and at job end includes the build's page cache. Sample the non-reclaimable part instead:( while sleep 5; do awk '/^(anon|shmem) /{s+=$2} END{print s}' /sys/fs/cgroup/memory.stat; done > "$OUT/cgroup_mem.txt" ) & SAMPLER=$! # ... shards and large phase ... kill $SAMPLER; echo "container anon+shmem peak: $(sort -n "$OUT/cgroup_mem.txt" | tail -1)"; grep -E '^(max|oom_kill) ' /sys/fs/cgroup/memory.eventsPer-process peaks can't show it: 134 GiB summed vs ~69 GiB together.
-
Update PR description:
- since 9f7bcf1 the peak comes from
[ PEAK RSS ], not/usr/bin/time -v(Technical Details, Test Result) - PR also changes
memory_estimate_check.hpp,hipblaslt_gtest_main.cppandmatmul_gtest.cpp. - Update hardware run link: 41717824185
- since 9f7bcf1 the peak comes from
9f7bcf1 to
c271af7
Compare
|
Thanks Archana, for the approval and for all the digging! Your gfx1250 numbers have been a huge help. I updated the description with your points: On the 4-way overlap run and the cgroup sampler, I think I'm going to stop here, and I hope that's OK. My view is that this stack doesn't guarantee the host never runs short. It just makes things a lot better: the client holds much less pinned memory, gives it back sooner, and skips with the amounts instead of getting killed. That's the part I can fix from the client side. If a big test campaign showed it wasn't enough, I don't think I'd do anything different here. I'd put the lock back (a one-file revert) and pick up fix 6, and the caps keep any kill inside one job in the meantime. So I'd rather let the lane's normal runs tell us. Your sampler recipe is great, though, so I'll put it on AIHPBLAS-5352 under fix 6 for whoever picks that up. |
newling
left a comment
There was a problem hiding this comment.
Small PR with the correct shape, title and description look straightforward. Approving (but haven't been through in detail, I think Archana has)
c271af7 to
d7d32a9
Compare
|
🎉 All checks passed! This PR is ready for review. |
d7d32a9 to
53d3ab8
Compare
… them (#14608) JIRA ID : AIHPBLAS-5352 ## Motivation `hipblaslt-test`, the hipBLASLt test client, keeps the host buffers that each test frees in a pool, so the next test can reuse them instead of allocating again. These buffers are pinned (allocated with `hipHostMalloc`, so the operating system can neither page them out nor reclaim them), and the pool kept every one until the process exited. On the gfx1250 hardware test host, which has about 499 GiB of memory, the 93 cases with 2^31- and 2^32-element sizes enabled in #13504 left about 123 GiB pinned in one process. With 4 CI runners each splitting the suite into 4 parallel test processes, the out-of-memory killer took the runners down on 10-23 and 10-27. Separately, a failed host allocation returns a null buffer, and the test then writes through it. I don't think this stack guarantees the gfx1250 host never runs short. What it does is make the client hold much less pinned memory, give it back sooner, and skip a case with the amounts instead of getting killed, which is the part of the problem the client can fix. If pressure comes back, the 110 GiB container caps from #14569 keep a kill inside one job, and putting the lock back (#14982 removes it) is a one-file revert. So I'm not planning a long soak test before merging. The pool comes from ROCm/hipBLASLt#1202, which reused allocations to speed up `hipblaslt-bench` across about 500,000 small matmuls. Reuse still matters for large buffers: on the test host below, `hipHostMalloc` takes about 0.18 s per GiB and `hipHostFree` about 0.06 s per GiB. ## Technical Details - A pooled host buffer of 1 GiB or more (`kHostPoolRetainLimit` in `d_vector.hpp`) now stays cached only while consecutive tests reuse it. The pool records the test count when a buffer comes back, and `HostPoolReleaseListener`, a new hook that Google Test calls after every test, frees the large buffers that the test just ended did not use. A run of same-shape cases, such as the four transpose combinations (NN, NT, TN, TT) of one size, still shares one allocation. - When a test asks for a large buffer that no cached buffer fits, the pool first frees the large buffers that earlier tests left, then pins new memory. This keeps the peak near one test's footprint when the shape changes. - A host buffer no longer gets the pool's 20% extra capacity when the extra would take it to 1 GiB or more (`host_pool_capacity()`), so a buffer under the limit stays under it. The existing cutoff for skipping the extra is a quarter of device memory, so on gfx1250 nearly every host buffer got it. - `HipHostBuffer::memcheck()` reports a failed pinned allocation, and `testing_matmul` checks every host buffer after allocating, including the four temporary buffers a swizzled A or B is built in. A failure now skips the test with the requested size ("could not allocate N GiB of pinned host memory"). - A pooled host buffer under 1 GiB is released once 100 consecutive tests have not reused it (`kHostPoolSmallIdleTests`). The pool reuses a buffer only for requests between half its size and its size, so without this a run of shrinking requests kept every earlier buffer pinned. - In `hipblaslt-bench`, a failed host allocation now exits with a failure, as a failed device allocation already did, instead of reporting success. - Device memory behaves as before. `hipblaslt-bench` does not install the listener, so it never frees large buffers early; it does see the 20% change for host buffers of 1 GiB or more. The hipBLASLt library itself is unchanged. This is the first of four stacked PRs for AIHPBLAS-5352. #14609 makes the client's host-memory query respect container memory limits, #14610 runs the memory preflight for every matmul case, and #14982 drops the host-wide lock in the gfx1250 workflow. ## Test Plan - `HostMemory_pre_checkin` (new, `clients/tests/src/host_memory_gtest.cpp`, 6 tests; the `_pre_checkin` suffix puts it in the standard tier the PR checks run). The tests check that a large buffer survives one test boundary and is freed after the next, that a same-shape test reuses it (what the first test wrote is still there), that a large miss frees two buffers earlier tests left, that a 1 GiB request gets exactly 1 GiB, that a small buffer is released after 100 tests that do not reuse it (and kept when one does), and that a failed allocation leaves a null buffer whose size is reported. A 0.9 GiB request gets exactly 0.9 GiB (the old rule gave it 1.08 GiB, over the limit). The tests skip, naming the buffer, on a host that cannot pin that much, and the reuse test skips on a Windows host where the pool frees its idle buffers before every request. A temporary variant without the small-buffer aging failed the new test. - To confirm the tests can fail, I built two temporary variants of the pool. One undid four behaviors at once (no release at the end of a test, no release on a miss, the 20% extra restored, `memcheck()` always succeeding), and it failed 4 of the 5 tests. The other freed large buffers whether or not a test used them, and it failed 3. Every test failed under at least one variant. - The 82 large-memory cases that apply to gfx90a (the five name patterns from #14545), and the `smoke` and `quick` tiers, on develop and on this branch, for wall time and peak resident memory. - Handle leaks: #14714 and #14700 on develop give the matrix layouts, matmul descriptors and HIP events owners, so an early return no longer leaks them. To check that with this stack's new early returns, a preloaded library counted creates and destroys of all three, on the full stack, over 1,892 matmul cases, once normally and once under a 1 MiB host limit so that most cases skip at the preflight. - Adjacent: the `FastCheck_pre_checkin` and `FastCheckDevice_pre_checkin` suites, including `memory_shortfall_reclaims_idle_buffers`, which uses the pool's existing `ReleaseCached()`. `hipblaslt-bench` was built, because it compiles the non-gtest form of the new check, but not run. ## Test Result One MI210 (gfx90a) on a shared host with 2.2 TB of memory. This branch's clients were built against the 10-25 TheRock nightly hipBLASLt gfx90a device library. Each pair of runs went back to back, but the host was busy (load average near 300), so the times carry some noise. | Measurement | develop | this PR | | --- | --- | --- | | 82 large-memory cases, wall time | 316.9 s | 313.8 s | | 82 large-memory cases, peak resident memory | 81.9 GiB | 49.2 GiB | | 82 large-memory cases, mean resident memory | 66.9 GiB | 16.6 GiB | | Resident memory after the last large case | 74 GiB | 2 GiB | | `smoke` + `quick` (9,171 tests), two runs | 66.6 s, 62.0 s | 63.6 s, 64.1 s | - Large-memory cases: 82 of 82 passed on both. `smoke` + `quick`: 9,169 passed and 2 skipped (`GroupedMatmul.smoke_Fp16NN` and `smoke_Bf16NN`, which skip on this GPU) on both. - `HostMemory_pre_checkin`, `FastCheck_pre_checkin` and `FastCheckDevice_pre_checkin`: 64 of 64 passed. - A first version that freed every large buffer at the end of each test took 449.6 s for the large cases against 318.2 s on develop, which is why the pool keeps a buffer while the next test reuses it. - Handle leaks: 0 layouts, descriptors or events left, both normally (13,580 created) and with 409 cases skipped at the preflight (11,235 created). - With the small-buffer aging, `smoke` + `quick` passed 9,225 tests in 59.3 s with peak resident memory 2.18 GiB. - gfx1250: this PR alone passed the hardware workflow's hipBLASLt tests in [40934683157](https://github.com/ROCm/rocm-libraries/actions/runs/40934683157), and Archana dispatched the full stack in [41582794047](https://github.com/ROCm/rocm-libraries/actions/runs/41582794047), which passed hipBLASLt twice with no kills and no memory skips. Her run on the rebased stack, [41717825380](https://github.com/ROCm/rocm-libraries/actions/runs/41717825380), was clean too. All three kept the host-wide lock; #14982 covers the run without it. - Automated GitHub checks are pending. ## Submission Checklist - [x] Look over the contributing guidelines at https://github.com/ROCm/TheRock/blob/main/GOVERNANCE.md#pull-requests. ## Risk level Medium (3/5). Every `hipblaslt-test` case passes through the new listener and the host pool, so a mistake here would affect the whole suite. The hipBLASLt library and device behavior are unchanged; only the test and bench clients change. The bench sees the 20% change near 1 GiB, and now exits with a failure when a host allocation fails. ## Related - Work tracking: [AIHPBLAS-5352](https://amd-hub.atlassian.net/browse/AIHPBLAS-5352), fixes 1 (pool retention) and 4 (null host buffer) of its list. - Cause: #13504. CI workaround this work is meant to replace: #14545, merged on 10-28. - Where the pool came from: ROCm/hipBLASLt#1202. Windows already clears the pool on smaller-memory hosts for the same reason: #4793, #6121. ## Device / Architecture Coverage Host-side test-client code only, with no kernel, library or kernel-selection change, so it does not depend on the GPU architecture and the standard PR checks are enough. Measured on gfx90a. ## Flags / Guardrails None. The 1 GiB threshold is the constant `kHostPoolRetainLimit`. [AIHPBLAS-5352]: https://amd-hub.atlassian.net/browse/AIHPBLAS-5352?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ --------- Co-authored-by: Cursor <cursoragent@cursor.com>
The gfx1250 workflow ran the large-memory hipBLASLt tests behind a flock shared by every runner on the host, so jobs took turns. With the client now checking each case's memory and freeing large host buffers between tests (AIHPBLAS-5352), and each job's container capped (#14569), drop the lock and its /ci-locks mount. The large tests still run after the shards, one group per process. The large phase now sets HIPBLASLT_TEST_FAIL_ON_MEMORY_SKIP=1, so a large case that skips for memory fails the job, and every test process runs under /usr/bin/time -v. The job summary lists each process's peak resident memory and every test that stopped for memory. JIRA ID : AIHPBLAS-5352 Co-authored-by: Cursor <cursoragent@cursor.com>
The summary read peak memory from GNU time, which had three problems. time -v wrote unscrubbed CPU and elapsed times into the uploaded .time files. A missing .time file stopped the step under bash -e before the summary and the exit code. And MemoryEstimateCheck's write to /proc/self/clear_refs also resets the peak time reports: a process that touched 1 GiB and then reset it reported 2 MiB. hipblaslt-test now saves the peak before each reset and prints the run's peak under [ PEAK RSS ] at the end. The summary reads that line, and the workflow no longer installs or runs time. A process that dies before the end shows no peak. Reported by Copilot. JIRA ID : AIHPBLAS-5352 Co-authored-by: Cursor <cursoragent@cursor.com>
53d3ab8 to
b662bba
Compare

JIRA ID : AIHPBLAS-5352
Motivation
The gfx1250 hardware workflow runs the hipBLASLt client tests on a host shared by 4 runners. Since #14545, the 93 large-memory cases (2^31- and 2^32-element sizes) run after the 4 test shards, one group per process, and behind a lock file shared by every runner on the host (
flockon/ci-locks), so only one job at a time runs them. That lock was a stopgap: without it, several jobs' large cases together ran the host out of memory, because the test client kept their pinned host buffers until the process exited. The three PRs below this one fix that in the client, and #14569 gives each job's container its own 110 GiB memory cap, so the lock can go.I don't think this is a guarantee that the host never runs short, and I'm not planning a long test campaign to prove it. The client now holds much less pinned memory and skips a case with the amounts instead of getting killed, and the caps keep any kill inside one job. That's better than a lock that serializes every job's large phase. If kills come back, putting the lock back is a revert of this one file.
Technical Details
/scratch/ci-locksmount is removed. Jobs on the host run their large phases at the same time.HIPBLASLT_TEST_FAIL_ON_MEMORY_SKIP=1. These cases are the reason the job runs on this host, so one that skips for lack of memory now fails the job instead of passing quietly. The shards keep the default, where a memory skip is a skip.hipblaslt-testnow prints its own peak resident memory at the end of a run, as[ PEAK RSS ] N.NN GiB of host memory(Linux). The job summary reads that line into a table of every process's peak, which counts pinned host buffers, and the number of its tests that stopped for memory, followed by the[ MEMORY ]lines themselves. A process that dies before the end shows no peak.time, because the estimate check (HIPBLASLT_TEST_CHECK_MEMORY_ESTIMATE, from fix(hipblaslt): run the memory preflight for every matmul case and list memory skips #14610) resets the kernel's peak, which also resets whattimeandgetrusagereport: a process that touched 1 GiB and then reset it reported 2 MiB.MemoryEstimateCheck(memory_estimate_check.hpp) now saves the peak before each reset, andhipblaslt_gtest_main.cppprints the larger of that and the final peak. This also dropstimefrom the job's packages and its unscrubbed timing files.TESTING.md's row for this lane describes the large phase and the summary.This is the fourth PR in the AIHPBLAS-5352 stack and depends on #14610, #14609 and #14608. It changes the workflow,
TESTING.md, and for the peak,memory_estimate_check.hpp,hipblaslt_gtest_main.cppandmatmul_gtest.cpp. It relies on the client behavior in the PRs below it, so it merges after them.Test Plan
actionlinton the workflow: the same 3 findings as develop and none new (a custom runner label, and two deliberate glob matches in the existingin_filter).pre-commit runon both files.hipblaslt-test: once with every case passing, and once where a large case skips for memory. The summary block alone underbash -e, against one log with a[ PEAK RSS ]line and one without.MatmulMemory_pre_checkin.peak_resident_memory_survives_the_estimate_checkpins 1 GiB, frees it, runs the estimate check, and expects the kernel's peak to drop while the reported one keeps the 1 GiB. A variant that does not save the peak fails it.Test Result
[ PEAK RSS ] 12.34 GiBshows 12.34; a log without the line gets an empty cell, and the step goes on to its exit code.[ PEAK RSS ] 3.19 GiB of host memory.[ PEAK RSS ]15.3 to 38.8 GiB per shard and 2.2 to 49.9 GiB per large group; TensileLite 216 passed. Its large phase overlapped about 4 minutes with another job's, so 2 at once, not 4. Before this PR, her dispatch of the stack with the lock (41582794047) passed hipBLASLt twice with no kills and no memory skips.Submission Checklist
Risk level
Medium (3/5). It removes the only coordination between jobs on the gfx1250 host. What bounds memory now: each container's 110 GiB cap from #14569, which keeps an out-of-memory kill inside one job, and the client's preflight and buffer release from this stack. Four containers at 110 GiB plus about 36 GiB the host uses when idle come to 476 of about 499 GiB, about 23 GiB to spare (Archana's numbers). The caps hold for pinned buffers too: they are shared memory charged to the container. A large case that runs short of memory now fails the job, so a problem shows up as a red run, not a quiet skip. The lane is not a required check, and putting the lock back is a revert of this file. The hipBLASLt library is unchanged; the test client only gains the peak report.
Related
Change class
build/ci,ci-gate-change(a memory skip in the large phase now fails this lane). No new workflow, runner, trigger or shared CI state; the checkout is unchanged.