cuda: query the device type with cudaDeviceGetAttribute, not cudaGetDeviceProperties (+8% decode) - #163
Merged
Merged
Conversation
…eviceProperties ggml_backend_cuda_device_get_type read the whole cudaDeviceProp to test one flag. The full property read costs ~660 us per call; the hybrid-attention graph builder asks for the device type of every device on every recurrent layer of every graph build (dozens of calls per graph), which an H100 nsys profile put at 409 cudaGetDeviceProperties calls / 337 ms per llama-bench run against 25 / 22 ms for upstream, about 0.3 ms per decoded token. cudaDeviceGetAttribute(cudaDevAttrIntegrated) answers the same question in ~20 ns. HIP gets the attribute alias; MUSA resolves the CUDA name already.
Collaborator
|
Reproduced on our L40S (CUDA 12.8, sm_89). PR head f7ae96b vs its base 8c0170d, separate worktrees, two interleaved passes of
Prefill unchanged within noise; decode gains consistent across both passes and well outside the 1 to 2 tok/s error bars. Smaller models gain most, as expected for a fixed per-token host cost. Review notes:
Merging. |
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.
What
ggml_backend_cuda_device_get_typeread the wholecudaDevicePropto test theintegratedflag. It now askscudaDeviceGetAttribute(cudaDevAttrIntegrated). HIP gets the attribute alias invendors/hip.h; MUSA already resolves the CUDA name.Why
The full property read costs about 660 us per call on an H100; the attribute read about 20 ns. The hybrid-attention graph builder queries the device type of every device on every recurrent layer of every graph build, so one decode graph pays it once per recurrent layer, dozens of times. An nsys profile in July put it at 409
cudaGetDevicePropertiescalls / 337 ms per llama-bench run against 25 calls / 22 ms for upstream, about 0.3 ms per decoded token, and it was the largest identified piece of the fork's host-side decode gap vs upstream.The graph-side loop is left as is: with the backend call cheap it costs ~2 us per build, and caching it in the model would have to reason about multiple loaded models.
Verification (L40S, CUDA 12.8, sm_89)
test-backend-ops test -b CUDA0 -o MUL_MAT: 1283/1283 passed,-o MUL_MAT_ID: 1019/1019 passed on the sibling build of the same tree (exit 0 both).8c0170d19:Prefill differences are inside the run-to-run spread (about 8% on this card at pp512); decode error bars are 1 to 4 tok/s.
Bit-exact by construction: no kernel changes, only the host query.