fix(runtime): make DFlash construction transactional - #736
Conversation
a1d0eaa to
53bba32
Compare
|
Holding #736 after independent review of the beta-composed candidate22f7e2ba6. Two real leaf paths still escape the claimed construction transaction: (1) hfq_tensor_f32 plus F16-lift/F32 matrix arms use Gpu::upload_f32, which allocates then returns on memcpy_htod failure without freeing the new owner before it reaches gt!/wt! staging; (2) AWQ scale attachment uses load_awq_scale -> direct-HIP upload_raw(...).ok(), leaking on copy failure and silently converting failure to missing scale. Successful direct allocations also bypass the pool, so pool_stats-only retry tests do not prove the AWQ invariant. Please make these uploads pool-aware/free-on-copy-failure and propagate sidecar errors, with a focused after-allocation copy-failure regression. Move the new changelog line to Unreleased as well. The reviewer specifically found the scratch tests current before-retry baselines valid for their final-allocation seams; do NOT move them based on our preliminary hypothesis. No #736 code has landed. |
|
Maintainer-integrated into beta at 24fc65c, preserving the original change with cherry-pick provenance and adding the review-required corrections. This closes the original PR as ported, not as an unmodified merge. Fixes: F32/raw leaf uploads return their pool allocation on copy failure; DFlash AWQ sidecars use pooled allocation and propagate errors while releasing the trunk; changelog entry moved to Unreleased. Leaf regression oracle warms two simultaneously live slots before testing cumulative allocation counts (the original empty-pool baseline was invalid). Parent-run proof on gfx1201 / Radeon AI PRO R9700 / HIP 7.15:
Trunk SHA256: 9f91556f7e0431a077d03756a7102d0154108757289e6e5fe9a2d204c0c9eeb7 |
Problem
DflashWeights::loadand the base/windowedDflashScratchconstructors transfer GPU owners through multi-step construction without RAII. Ordinary late errors could strand completed layers, staged weights, optional scratch, or window-extension allocations.Change
GenericDflashStaging/ Qwen35or_free!ownership remain unchangedVerification
Final head
53bba32a5on upstream/betafdb750d6d:cargo test -p hipfire-runtime --lib construction_tests -- --ignored --test-threads=1 --nocapture: 4 passed on gfx1151cargo test -p hipfire-runtime --lib: 609 passed, 4 ignoredcargo clippy -p hipfire-runtime --lib --no-deps: completed with repository warnings onlycargo buildandcargo build --release: passedHardware/lifecycle:
/home/bjoern/.hipfire/models/qwen3.5-27b.mq4, md5e1c9480a3fa54ad0a0f25ca18105510a/home/bjoern/.hipfire/models/qwen35-27b-dflash-mq4.hfq, md57b6df2a4ee1c8d933f0a52e187d1860bhipfiremd5e101abe74e886fd845dd42215e1b59a8;daemonmd51e1cba2b78dc7796a276c020da56f059serve_harness.pybattery: 5/5 coherent inspected outputs, draft loaded, every request loggeddrafter=dflashdrafter=dflash, prefix checkpoint reuse observedScope
This is G4.2 only. It does not replace outer DFlash staging, change successful generation semantics, or claim malformed-HFQ panic recovery. G4 milestone acceptance remains blocked on the remaining tracker tasks and the final composed-head matrix.