Skip to content

cuda : prevent premature eviction of graphs about to be used - #30662

Open
frank-2077 wants to merge 1 commit into
ggml-org:masterfrom
frank-2077:fix-cuda-graph-eviction-v2
Open

frank-2077 wants to merge 1 commit into
ggml-org:masterfrom
frank-2077:fix-cuda-graph-eviction-v2

Conversation

@frank-2077

Copy link
Copy Markdown

Overview

The current CUDA graph cleanup mechanism in PR #21611 introduces a timing issue that causes unnecessary performance degradation in high-concurrency scenarios.

Current behavior in ggml-cuda/common.cuh:1467-1488:

// sweep every 5s, evicting cuda graphs unused for >=10s
if (time_now - last_graph_eviction_sweep >= 5'000'000) {
    last_graph_eviction_sweep = time_now;
    for (auto it = cuda_graphs.begin(); it != cuda_graphs.end(); ) {
        if (time_now - it->second->last_used_time >= 10'000'000) {
            it = cuda_graphs.erase(it);  // ← Can evict graph we're about to use!
        } else {
            ++it;
        }
    }
}

The issue:
If a graph's last_used_time is >10s ago, it gets deleted even when we're about to use it in the same function call. This forces expensive re-capture operations (warmup → capture → instantiate).

Solution

Exclude the currently requested graph from the cleanup sweep:

if (it->first != first_node_ptr && time_now - it->second->last_used_time >= 10'000'000) {

This simple check prevents deleting the graph we're actively requesting. The graph's last_used_time gets updated later in the function, so subsequent cleanups will preserve it correctly.

Code Change

File: ggml/src/ggml-cuda/common.cuh

-                if (time_now - it->second->last_used_time >= 10'000'000) {
+                if (it->first != first_node_ptr && time_now - it->second->last_used_time >= 10'000'000) {

1 line changed - adds a check to skip the currently requested graph during cleanup.

Impact on Production Deployments

Our deployment characteristics make this fix critical:

  • Long-running service with intermittent 10s+ idle periods per graph
  • High concurrency on single GPU, pushing hardware limits
  • Expensive capture/instantiate operations under contention

Profiling Data - Capture & Instantiate Latency Growth (High Concurrency)

Under concurrent load (1→8 concurrent requests), re-capture costs become prohibitive:

Module Capture Time (ms)
1→8 concurrent
Instantiate Time (ms)
1→8 concurrent
LLM 0.88 → 1.73
(p50: 0.95 → 1.18)
1.03 → 2.08
TTS-LLM 0.45 → 1.00 0.77 → 3.60
VPM 1.96 → 19.9
(p50: 3.1)
1.53 → 26.4
T2W-flow 13.6 → 50.2 20.4 → 198
(p95: 549ms)
T2W-vocoder 2.39 → 5.44 4.70 → 15.0

Key observation: Some modules see 10-27× latency increase under concurrency, likely due to CUDA driver contention. Avoiding unnecessary re-captures is essential for maintaining throughput.

Benefits

  1. Eliminates pathological eviction - Graphs in active use never get evicted
  2. Preserves original cleanup intent - Truly stale graphs (>10s genuinely unused) still get cleaned up
  3. Minimal change - Only 1 line modified, easy to review and verify
  4. Zero overhead - Single pointer comparison added to existing loop
  5. Works for bursty workloads - Common pattern where same graph is used repeatedly with >10s gaps

Testing

  • Verified that graphs requested within 10s of last use are not evicted
  • Confirmed currently requested graph is never evicted regardless of idle time
  • Confirmed cleanup still works for genuinely unused graphs
  • Stress-tested under high concurrency with intermittent access patterns

This fix applies to any high-concurrency deployment where graph access patterns have occasional >10s gaps. We observed this in production serving multimodal workloads with bursty request patterns.

Note: Our workload uses a custom model architecture (Omni multimodal model) on a forked llama.cpp, but this fix applies to any high-concurrency deployment where graph access patterns have occasional >10s gaps.

Requirements

  • I have read and agree with the contributing guidelines
  • AI usage disclosure: YES, I use claude for llama.cpp learning and Polish PR

@frank-2077
frank-2077 requested a review from a team as a code owner October 16, 2026 09:42
@github-actions github-actions Bot added ggml changes relating to the ggml tensor library for machine learning CUDA Related to the CUDA backend labels Oct 16, 2026
@frank-2077

Copy link
Copy Markdown
Author

llama.cpp natively supports single-weight multi-context architecture, enabling natural concurrency handling. However, in high-concurrency scenarios, both CUDA graph capture and instantiate operations exhibit significant non-linear latency growth due to CUDA driver contention. Under these conditions, unnecessarily recreating graphs becomes extremely wasteful — avoiding premature eviction of graphs that are about to be reused is critical for maintaining throughput.

@ggml-gh-bot

ggml-gh-bot Bot commented Oct 16, 2026

Copy link
Copy Markdown

Hi @frank-2077, thanks for your contribution!

Per our contribution guidelines, the automated PR checker found the following issue(s) that need your attention:

  • AI-generated content: While code is allowed to be generated by AI, please write the PR description and commit messages on your own without the help of AI.

Please note that maintainers reserve the right to make final decisions on PRs. If you believe there is a mistake, please comment below.

@frank-2077

Copy link
Copy Markdown
Author

@am17an Hi, This addresses one of the issues we've encountered. Other problems may require modifications to the model's graph construction logic.

Regarding this specific issue, the fix does resolve the core pain point: when a CUDA graph has been idle for more than 10 seconds and is then requested again, it would previously be evicted, recreated, and instantiated — a completely wasteful process. By excluding the currently requested graph from the cleanup loop, we avoid this unnecessary overhead.

@am17an

am17an commented Oct 16, 2026

Copy link
Copy Markdown
Contributor

We should instead introduce a new variable last_graph_use_time and compare with that instead of time_now, that way when there are idle periods none of the graphs get evicted (not just the first)

Something like this:

if (last_graph_use_time - it->second->last_used_time >= 10'000'000) {   // instead of time_now - ...
...
it->second->last_used_time = time_now;
last_graph_use_time        = time_now;

@maage

maage commented Oct 17, 2026

Copy link
Copy Markdown

FYI

I used this PR as an example of modification to trigger full CUDA rebuild. #11582 (comment)

@frank-2077

Copy link
Copy Markdown
Author

@am17an I’ve run into a few details that I’d like to discuss with you.

I considered your proposal and this is the current local implementation. I have not pushed it yet because there are still some open questions about the behavior.

ggml_cuda_graph * cuda_graph(const void * first_node_ptr) {
    const int64_t time_now = ggml_time_us();

    bool all_graphs_idle = !cuda_graphs.empty();
    for (const auto & entry : cuda_graphs) {
        if (time_now - entry.second->last_used_time < 10'000'000) {
            all_graphs_idle = false;
            break;
        }
    }

    // Keep all graphs on the first request after a global idle period.
    if (all_graphs_idle) {
        for (auto & entry : cuda_graphs) {
            entry.second->last_used_time = time_now;
        }
        last_graph_eviction_sweep = time_now;
    } else if (time_now - last_graph_eviction_sweep >= 5'000'000) {
        // During active use, evict graphs unused for >=10s.
        last_graph_eviction_sweep = time_now;
        for (auto it = cuda_graphs.begin(); it != cuda_graphs.end(); ) {
            if (it->first != first_node_ptr &&
                time_now - it->second->last_used_time >= 10'000'000) {
                it = cuda_graphs.erase(it);
            } else {
                ++it;
            }
        }
    }

    auto it = cuda_graphs.find(first_node_ptr);
    if (it == cuda_graphs.end()) {
        it = cuda_graphs.emplace(first_node_ptr, std::make_unique<ggml_cuda_graph>()).first;
    }
    it->second->last_used_time = time_now;
    return it->second.get();
}

The main concern is that a graph that is never used again may still be kept alive if all graphs are considered idle when another graph is requested. Also, checking all graphs on every call adds some overhead.

Does this approach match your intended behavior, or would you recommend moving the eviction logic to a higher-level graph-compute entry point?

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CUDA Related to the CUDA backend ggml changes relating to the ggml tensor library for machine learning

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants