Skip to content

CUDA: use LRU based eviction for cuda graphs #21611

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
am17an merged 5 commits into from
Apr 17, 2026
Merged

CUDA: use LRU based eviction for cuda graphs #21611

Changes from 2 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
beb4e98
CUDA: use a ring-buffer for cuda graphs
am17an Apr 8, 2026
193b81e
bump limit to 128
am17an Apr 16, 2026
bd942c4
use LRU eviction
am17an Apr 16, 2026
958fd9f
better naming
am17an Apr 16, 2026
579972b
do periodic clean-up
am17an Apr 17, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions ggml/src/ggml-cuda/common.cuh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
#include <string>
#include <unordered_map>
#include <vector>
#include <deque>

#if defined(GGML_USE_HIP)
#include "vendors/hip.h"
Expand Down Expand Up @@ -151,6 +152,7 @@ static int ggml_cuda_highest_compiled_arch(const int arch) {
#define MATRIX_ROW_PADDING 512 // last row of quant. matrices is a multiple of this to avoid out-of-bounds memory accesses

#define GGML_CUDA_MAX_STREAMS 8
#define GGML_CUDA_MAX_GRAPHS 128

@gaugarg-nv gaugarg-nv Apr 16, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think 128 might be too low.

The TP implementation breaks each decoder layer into 2-sub graphs. So, for larger models, it will easily hit this limit.

@am17an am17an Apr 16, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How many layers do large models have? Is 256 a better limit?

@gaugarg-nv gaugarg-nv Apr 16, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have seen up to 80 layers. So, yes, 256 will be a better limit.

@MaxKruse MaxKruse Apr 16, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

GLM4.7 notably has 92 layers https://huggingface.co/zai-org/GLM-4.7/blob/main/config.json

@JohannesGaessler JohannesGaessler Apr 16, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Without NCCL the number of ggml graphs is currently even higher. However, these graphs only have a single node so using CUDA graphs may not be worthwhile in the first place.

@am17an am17an Apr 16, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay implemented this. Since std::priority_queue doesn't support random access, we need to a do a little bit of book-keeping on the side and keep some stale entries in the queue, but I think it should be okay.

@ggerganov ggerganov Apr 17, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was thinking about a much simpler implementation:

  • Add a timestamp to ggml_cuda_graph
  • LRU purging is loop over cuda_graphs and remove outdated entires
  • Update timestamp each time a graph is referenced

I highly doubt the priority queue here has any advantage in terms of performance since we are dealing with very small number of entries, while it makes the logic quite complicated.

@am17an am17an Apr 17, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

-sm tensor on peak was at 800 cuda graphs with 4x 4090 running gpt-oss-120b, so I don't think we can loop over all entries? We have to keep them sorted somehow

@ggerganov ggerganov Apr 17, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't need to purge on each iteration. For example, we can purge only if X seconds have passed since the last purge, so even with 800 graphs or more in the container, it should not be a problem to loop over all of them from time to time.

@am17an am17an Apr 17, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok I simplified this, and did not see any performance degradation


[[noreturn]]
void ggml_cuda_error(const char * stmt, const char * func, const char * file, int line, const char * msg);
Expand Down Expand Up @@ -1367,11 +1369,17 @@ struct ggml_backend_cuda_context {
// Map from first_node_ptr to cuda_graph - allows multiple graphs per context
// when the computation is split across CPU/GPU (e.g., with --n-cpu-moe)
std::unordered_map<const void *, std::unique_ptr<ggml_cuda_graph>> cuda_graphs;
std::deque<const void *> graph_roots;

ggml_cuda_graph * cuda_graph(const void * first_node_ptr) {
auto it = cuda_graphs.find(first_node_ptr);
if (it == cuda_graphs.end()) {
if (graph_roots.size() >= GGML_CUDA_MAX_GRAPHS) {
cuda_graphs.erase(graph_roots.front());
graph_roots.pop_front();
}
cuda_graphs[first_node_ptr] = std::make_unique<ggml_cuda_graph>();
graph_roots.push_back(first_node_ptr);
return cuda_graphs[first_node_ptr].get();
}
return it->second.get();
Expand Down
Loading