fix(clr): Remove premature staging buffer flush in batch copy operations - #9743
Closed
kardarra-amd wants to merge 1 commit into
Closed
fix(clr): Remove premature staging buffer flush in batch copy operations#9743kardarra-amd wants to merge 1 commit into
kardarra-amd wants to merge 1 commit into
Conversation
## Summary Fixes a ~15% performance regression in Laghos cube_922_hex benchmark on 4-GPU MI300A systems introduced in commit fb484f3. The regression was caused by premature staging buffer flushes in `WriteBufferBatch` and `ReadBufferBatch` that created artificial synchronization points. ### Root cause Commit fb484f3 ("Add batched version of writeBuffer and readBuffer") introduced staging buffer size checks that would flush accumulated batch operations before they exceeded `StagingXferSize`. While intended to prevent resource exhaustion, these checks created unnecessary sync points: - Each flush forces all queued copies to complete before continuing - In MPI-heavy workloads with `--no-gpu-aware-mpi`, this creates a GPU→Host→MPI→Host→GPU data transfer pattern that gets serialized - The CG (H1) kernel time increased from 1.287s to 1.551s (+20.5%) The staging pool already handles rotation naturally, and `getBuffer()` will handle resource exhaustion if needed. The premature flush checks were overly conservative. ### Fix Remove the premature flush checks in both `WriteBufferBatch` and `ReadBufferBatch`. Operations now accumulate and flush only at the end of the full batch, matching the behavior of the original non-batch `writeBuffer()` and `readBuffer()` paths. ## Test Results **Before fix (commit fb484f3, TheRock nightly 7.15.0-20260707):** - 4-GPU Laghos FOM: 3821.02 Mdofs*timestep/sec - CG (H1) time: 1.551s **After fix (with library swap validation):** - 4-GPU Laghos FOM: 4554.13 Mdofs*timestep/sec (+19.2%) - CG (H1) time: 1.287s (restored to pre-regression level) **Test command:** ``` mpirun -np 4 ./laghos -pa -p 1 -tf 0.6 -m data/cube_922_hex.mesh \ --ode-solver 7 --max-steps 4 --cg-tol 0 --cg-max-steps 50 \ -ok 3 -ot 2 -rs 3 -rp 1 -d hip -f -pt 221 --no-gpu-aware-mpi ``` **Impact:** - 1-GPU and 2-GPU configurations: unaffected (no MPI communication overhead) - 4-GPU configuration: regression scales with MPI rank count - Fix verified on MI300A gfx942 hardware ## Related Issues Regression reported in TheRock nightly performance tracking between 7.15.0-20260706 (good) and 7.15.0-20260707 (bad).
❌ PR Check — Action Required
📖 Need help? See the Policy FAQ for details on every check and how to fix failures. |
|
🚫 Please fix the failed policies before requesting reviews. The following policy checks failed:
The |
kardarra-amd
deleted the
users/kardarra/fix-laghos-batch-copy-regression
branch
August 6, 2026 19:18
kardarra-amd
restored the
users/kardarra/fix-laghos-batch-copy-regression
branch
August 6, 2026 20:31
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.