Skip to content

Cache NVRTC output on disk, and stop concurrent CUDA work from breaking graph capture - #1612

Merged
justinchuby merged 1 commit into
mainfrom
nvrtc-cache-and-capture-gate
Aug 20, 2026
Merged

justinchuby merged 1 commit into
mainfrom
nvrtc-cache-and-capture-gate

Conversation

@justinchuby

Copy link
Copy Markdown
Owner

Two changes that have to land together: the first halves the one-time
first-token stall, and in doing so it exposed a latent capture hazard that
the second fixes.

NVRTC disk cache

nvrtc_function memoized compiled kernels in a process-local HashMap, so
every process start recompiled all of them from source. Because compilation is
lazy, the decode-only kernels all landed on the first decode step: a ~500 ms
stall, once per process, sitting directly on time-to-first-token.

kernel_cache persists NVRTC output under $XDG_CACHE_HOME/onnx-genai/nvrtc.
The key covers the module key, source, arch, include paths and artifact kind,
plus the NVRTC version so a toolchain upgrade invalidates rather than silently
serves stale PTX. Writes publish via a temp file and rename, so a torn write
is never observable. The cache is never load-bearing: a miss, an unreadable
entry or a disabled cache all fall through to compiling.

PTX rather than CUBIN, because the driver's own ~/.nv/ComputeCache already
handles PTX->SASS; what is expensive and uncached is the NVRTC front end. The
existing PTX-version fallback is preserved -- a cached PTX that fails to load
still falls back to the CUBIN path and caches that.

Measured on Muse Glimmer 30B, cold/warm interleaved: first-token stall
504 ms -> 253 ms. Steady-state p50 is unchanged (23.4 ms), as expected, and
96-token greedy output is byte-identical cold vs warm.

The key uses a spelled-out FNV-1a 128, not DefaultHasher, whose output is
only stable within one Rust version -- a toolchain bump would have silently
emptied every user's cache. Fields are length-prefixed so ("ab","c") and
("a","bc") cannot collide and hand one kernel another's code.

Capture gate

With the cache in place the crate's test suite began failing ~1-3 tests per
run with CUDA_ERROR_STREAM_CAPTURE_INVALIDATED, always inside some other
test's capture region, naming a kernel that had done nothing wrong. Serial runs
were clean; the same tests passed in isolation.

The cause is not the cache. CUDA invalidates an in-progress capture whenever
the process performs a device-wide synchronization -- cuMemAlloc,
cuMemFree, VMM map/unmap, module load and unload, stream destruction,
synchronous copies. CU_STREAM_CAPTURE_MODE_THREAD_LOCAL relaxes CUDA's
legality check on unsafe calls; it does not stop another thread's genuine
synchronization from killing the capture. The cache only changed the timing:
faster kernel resolution packs more threads into the CUDA-heavy phase at once.

So the exclusion cannot live on the stream, the graph lifecycle or the runtime
-- each admits a second instance per process. capture_gate is a single
process-wide reader/writer gate: capture takes the exclusive side, everything
that can synchronize the device takes the shared side. Three details that are
load-bearing rather than incidental:

  • The capturing thread bypasses the gate entirely, because capturing a decode
    step is allocating during capture.
  • A waiting capture blocks new shared sections, so a busy allocator cannot
    starve it -- which in turn forces re-entrant shared sections to be carved
    out, since commit recurses once per granule.
  • Acquiring the exclusion sets aside a section the calling thread already
    holds. Without that, two threads each holding one and both trying to capture
    wait on each other forever.

Spelled out by hand rather than built on RwLock because the capture guard is
stored in the graph lifecycle across the begin/end boundary, and
RwLockWriteGuard is !Send.

Two supporting fixes fell out:

  • The primary CUDA context is now cached per device for the process lifetime.
    CudaContext::new retains the primary context, so runtimes always shared
    one CUcontext and caching the Arc changes no semantics -- but it stops
    the refcount reaching zero, and a primary-context teardown synchronizes the
    device. Also makes the suite faster (89s -> 83.5s).
  • The teardown section is held in the last-declared field, not a local in
    drop. Fields drop after the body returns, so a local would have been
    released before the modules and streams whose destruction is the hazard.

Production cost is a single uncontended mutex per real allocation; pooled
allocations, which are what decode steady state uses, never touch the gate.
Decode measures 38.6 tok/s, unchanged.

This is also a correctness fix outside tests: a server capturing on one thread
while another request allocates had exactly this hazard.

Validation: 10/10 clean parallel runs of the 468-test suite (previously 6/6
failing with the cache alone, 6/6 passing without it), 9 unit tests for the
gate itself including a two-thread deadlock regression, engine suite 585/0,
no new clippy warnings.

Also fixes a genuine bug in the new cache's own guard test: it asserted an
exact delta on process-global compile counters, which other test threads move
concurrently.

Co-authored-by: Copilot 223556219+Copilot@users.noreply.github.com
Copilot-Session: 0190e2eb-abe4-451f-b36d-44a035a99b7e

…ng graph capture

Two changes that have to land together: the first halves the one-time
first-token stall, and in doing so it exposed a latent capture hazard that
the second fixes.

## NVRTC disk cache

`nvrtc_function` memoized compiled kernels in a process-local `HashMap`, so
every process start recompiled all of them from source. Because compilation is
lazy, the decode-only kernels all landed on the *first decode step*: a ~500 ms
stall, once per process, sitting directly on time-to-first-token.

`kernel_cache` persists NVRTC output under `$XDG_CACHE_HOME/onnx-genai/nvrtc`.
The key covers the module key, source, arch, include paths and artifact kind,
plus the NVRTC version so a toolchain upgrade invalidates rather than silently
serves stale PTX. Writes publish via a temp file and `rename`, so a torn write
is never observable. The cache is never load-bearing: a miss, an unreadable
entry or a disabled cache all fall through to compiling.

PTX rather than CUBIN, because the driver's own `~/.nv/ComputeCache` already
handles PTX->SASS; what is expensive and uncached is the NVRTC front end. The
existing PTX-version fallback is preserved -- a cached PTX that fails to load
still falls back to the CUBIN path and caches that.

Measured on Muse Glimmer 30B, cold/warm interleaved: first-token stall
504 ms -> 253 ms. Steady-state p50 is unchanged (23.4 ms), as expected, and
96-token greedy output is byte-identical cold vs warm.

The key uses a spelled-out FNV-1a 128, not `DefaultHasher`, whose output is
only stable within one Rust version -- a toolchain bump would have silently
emptied every user's cache. Fields are length-prefixed so `("ab","c")` and
`("a","bc")` cannot collide and hand one kernel another's code.

## Capture gate

With the cache in place the crate's test suite began failing ~1-3 tests per
run with `CUDA_ERROR_STREAM_CAPTURE_INVALIDATED`, always inside some *other*
test's capture region, naming a kernel that had done nothing wrong. Serial runs
were clean; the same tests passed in isolation.

The cause is not the cache. CUDA invalidates an in-progress capture whenever
the **process** performs a device-wide synchronization -- `cuMemAlloc`,
`cuMemFree`, VMM map/unmap, module load and unload, stream destruction,
synchronous copies. `CU_STREAM_CAPTURE_MODE_THREAD_LOCAL` relaxes CUDA's
legality check on unsafe calls; it does not stop another thread's genuine
synchronization from killing the capture. The cache only changed the timing:
faster kernel resolution packs more threads into the CUDA-heavy phase at once.

So the exclusion cannot live on the stream, the graph lifecycle or the runtime
-- each admits a second instance per process. `capture_gate` is a single
process-wide reader/writer gate: capture takes the exclusive side, everything
that can synchronize the device takes the shared side. Three details that are
load-bearing rather than incidental:

* The capturing thread bypasses the gate entirely, because capturing a decode
  step *is* allocating during capture.
* A waiting capture blocks new shared sections, so a busy allocator cannot
  starve it -- which in turn forces re-entrant shared sections to be carved
  out, since `commit` recurses once per granule.
* Acquiring the exclusion sets aside a section the calling thread already
  holds. Without that, two threads each holding one and both trying to capture
  wait on each other forever.

Spelled out by hand rather than built on `RwLock` because the capture guard is
stored in the graph lifecycle across the begin/end boundary, and
`RwLockWriteGuard` is `!Send`.

Two supporting fixes fell out:

* The primary CUDA context is now cached per device for the process lifetime.
  `CudaContext::new` retains the *primary* context, so runtimes always shared
  one `CUcontext` and caching the `Arc` changes no semantics -- but it stops
  the refcount reaching zero, and a primary-context teardown synchronizes the
  device. Also makes the suite faster (89s -> 83.5s).
* The teardown section is held in the last-declared field, not a local in
  `drop`. Fields drop *after* the body returns, so a local would have been
  released before the modules and streams whose destruction is the hazard.

Production cost is a single uncontended mutex per real allocation; pooled
allocations, which are what decode steady state uses, never touch the gate.
Decode measures 38.6 tok/s, unchanged.

This is also a correctness fix outside tests: a server capturing on one thread
while another request allocates had exactly this hazard.

Validation: 10/10 clean parallel runs of the 468-test suite (previously 6/6
failing with the cache alone, 6/6 passing without it), 9 unit tests for the
gate itself including a two-thread deadlock regression, engine suite 585/0,
no new clippy warnings.

Also fixes a genuine bug in the new cache's own guard test: it asserted an
exact delta on process-global compile counters, which other test threads move
concurrently.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 0190e2eb-abe4-451f-b36d-44a035a99b7e
@justinchuby
justinchuby merged commit a5bafbd into main Aug 20, 2026
3 checks passed
@justinchuby
justinchuby deleted the nvrtc-cache-and-capture-gate branch August 20, 2026 21:03
@codecov

codecov Bot commented Aug 20, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 96.50655% with 8 lines in your changes missing coverage. Please review.
✅ Project coverage is 80.97%. Comparing base (69329c9) to head (26c6b40).
⚠️ Report is 10 commits behind head on main.

Files with missing lines Patch % Lines
...rates/onnx-runtime-cuda-memory/src/capture_gate.rs 98.22% 0 Missing and 4 partials ⚠️
...s/onnx-runtime-cuda-memory/src/device_allocator.rs 0.00% 2 Missing ⚠️
...tes/onnx-runtime-cuda-memory/src/virtual_memory.rs 0.00% 2 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1612      +/-   ##
==========================================
+ Coverage   80.69%   80.97%   +0.27%     
==========================================
  Files         379      383       +4     
  Lines      170734   178885    +8151     
  Branches   170734   178885    +8151     
==========================================
+ Hits       137782   144857    +7075     
- Misses      28071    29087    +1016     
- Partials     4881     4941      +60     
Flag Coverage Δ
cli-ort-linux 82.60% <ø> (ø)
cli-ort-windows 82.19% <ø> (+0.09%) ⬆️
mlas 85.19% <ø> (?)
offline 80.84% <96.50%> (+0.21%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
...s/onnx-runtime-cuda-memory/src/device_allocator.rs 0.00% <0.00%> (ø)
...tes/onnx-runtime-cuda-memory/src/virtual_memory.rs 0.00% <0.00%> (ø)
...rates/onnx-runtime-cuda-memory/src/capture_gate.rs 98.22% <98.22%> (ø)

... and 45 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

justinchuby added a commit that referenced this pull request Aug 20, 2026
Brings in #1612 (NVRTC disk cache + the CUDA graph capture gate) and #1613
(retire the m == 1 half-decode handover). Three conflicts, all in
onnx-runtime-cuda-memory, all caused by #1612 landing a capture gate on code
this stack had already restructured.

## Resolutions

1. `src/lib.rs` (modify/modify) -- kept main's `pub mod capture_gate;` and this
   stack's `pub mod release;`, dropped `pub mod device_allocator;`. The eager
   allocator was deleted by this stack (VMM-only); nothing in the workspace
   references that module, and `capture_gate` does not depend on it.

2. `src/device_allocator.rs` (modify/delete) -- kept the deletion. #1612's only
   change to it was adding capture-gate guards around `cuMemAlloc`/`cuMemFree`,
   which is code this stack removed outright. Nothing to port.

3. `src/virtual_memory.rs` (modify/modify) -- took this stack's `release`, then
   ported #1612's guard *deeper* rather than to the same line.

## Why the guard moved, and one place it was missing entirely

#1612's contract is that no device-synchronizing driver call may run while a
CUDA graph capture is in flight on any thread. It enforced that at two trait
methods, `VirtualBacking::commit` and `VirtualBacking::release`, because on main
those were the only routes to the driver.

This stack changed that topology, so guarding the same two lines would have
preserved the diff while losing the guarantee:

- `release`'s driver calls now live in `release_blocks_reporting`, reached by
  four callers (`virtual_memory.rs` x2, `vmm_allocator.rs` x2), not one. The
  guard is placed at the syscalls, so all four are covered. `commit`'s guard
  merged cleanly and is unchanged.

- `ReservationTeardown::execute_outcome` is new in this stack and calls
  `cuMemUnmap`/`cuMemRelease` directly, driven by the deferred-release queue in
  onnx-runtime-ep-cuda. It did not exist when #1612 was written, so it had no
  guard at all. It is arguably the path that most needs one: it runs on a
  teardown queue rather than under a caller's commit, which is exactly the
  concurrent-with-capture case #1612 describes. Now guarded.

`synchronizing_section()` refcounts via `SHARED_DEPTH` and returns a non-
outermost token when nested, so pushing the guard down cannot deadlock against
an outer section. The crate's own `capture_gate` nesting tests cover this and
pass.

Out of scope, recorded rather than fixed: `commit_offsets_with_owned_limit_and_
capacity`, `reserve_and_map_shared_prefix`, and `map_shared_prefix_readonly` all
reach the driver and all predate this stack -- they are unguarded on main too.
That is a gap in #1612, not a merge loss here, and closing it blind on a machine
with no CUDA would be guesswork.

## Audit

- Overlap set is exactly the four files above; every other file main added or
  changed is byte-identical to `origin/main` by blob hash.
- No conflict markers remain anywhere in `crates/`.

## Verified on this macOS host

- `cargo fmt --all -- --check` clean.
- `cargo check --workspace --all-targets` and
  `cargo check -p onnx-runtime-ep-cuda --features cuda --all-targets` both OK.
- `cargo clippy --locked --all-targets` over the five memory crates with
  `-D warnings` clean (`onnx-runtime-cuda-memory` is on CI's offline list).
- `cargo test -p onnx-runtime-cuda-memory --lib`: 35 passed, 0 failed.

## Not verified

`cargo test -p onnx-runtime-ep-cuda --lib` reports 8 failures, all in #1612's
new `kernel_cache::tests`, all panicking inside cudarc at lib.rs:200 because
this host has no CUDA driver. A clean `origin/main` worktree fails the same 8,
so they are inherited, not caused here -- but it does mean the CUDA path was
again never executed. Those tests appear to be missing the GPU guard the rest of
the suite uses.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: c80f8522-983c-47f7-8241-2155a823aabe
justinchuby added a commit that referenced this pull request Aug 20, 2026
fix(ci): collapse the NVRTC cache lookup's nested `if let`

`main` is red on both `CUDA compile (Linux x86_64)` and `CUDA compile
(Windows x86_64)` at `81855858`. This is the third independent casualty
of the Rust 1.98.0 clippy bump, after #1604 and #1609. Refs #1600.

## What is failing

```
error: this `if` statement can be collapsed
   --> crates/onnx-runtime-ep-cuda/src/runtime.rs:942:9
    = note: `-D clippy::collapsible-if` implied by `-D warnings`
error: could not compile `onnx-runtime-ep-cuda` (lib) due to 1 previous error
```

`clippy::collapsible_if` fires across `if let` chains in 1.98.0. The
block
it lands on is the PTX disk-cache lookup added by #1612, which merged
before this lint's reach was understood -- the same way #1602's
`float_widen_entry` block became #1609's problem. Nothing here is a
regression from #1612's logic; the code is correct, the lint is new.

I did not infer the location. I pulled the job log from run

[32421805546](https://github.com/justinchuby/onnx-genai/actions/runs/32421805546)
and read it.

## The fix

Collapse to a let-chain, matching the form #1609 adopted in `qmoe.rs`. A
let-chain is semantically identical to the nested form -- same
short-circuit, same bindings, same scope -- so cache behaviour is
unchanged. `kernel_cache::load` returning `None`, or the bytes failing
UTF-8, still both fall through to a real NVRTC compile.

Only one occurrence exists. The other `kernel_cache::load` call site in
this file (line 973, the cubin path) is a `match`, not a nested `if
let`,
and does not trip the lint.

## Verified on this macOS host

- `cargo clippy -p onnx-runtime-ep-cuda --features cuda --lib -- -D
warnings`
  is clean with this change.
- **Reverse control, which is the part that matters:** I reverted the
file
  to `origin/main` and re-ran the same command. It fails with the exact
  CI error at the exact line --
`error: this if statement can be collapsed -->
crates/onnx-runtime-ep-cuda/src/runtime.rs:942:9`
-- then passes again once the change is restored. So this reproduces the
  CI failure locally and demonstrably clears it, rather than merely
  compiling.
- `cargo fmt --all -- --check` clean.

## Not verified

No CUDA hardware here, so nothing was executed -- but this job is a
compile/lint gate, and the gate itself is what I reproduced. The runtime
path is untouched.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: c80f8522-983c-47f7-8241-2155a823aabe

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: c80f8522-983c-47f7-8241-2155a823aabe
justinchuby added a commit that referenced this pull request Aug 21, 2026
Closes the root cause behind the 1.98.0 CI incidents tracked in #1600.

## What the failure was

Every blocking gate in `ci.yml` runs under `-D warnings` — either as
`RUSTFLAGS` or as a trailing `-- -D warnings` on clippy. CI resolved
`stable` at job time, so when runner images rolled forward to `rustc
1.98.0 (88d9e12ae 2026-08-18)`, every lint that release made
warn-by-default became an instant failure on code that was clean the day
before. No commit to blame, and a contributor on 1.97.0 could not
reproduce it — clippy 0.1.97 does not carry those lints at all, so it
does not even print them.

Four consecutive incidents, all the same cause:

| PR | Lint | Code from |
|---|---|---|
| #1604 | `cargo fmt` drift, `manual_slice_fill`, `needless_late_init` |
assorted |
| #1609 | `collapsible_if` ×5 in `qmoe.rs` + ep-cpu lints | #1602 |
| #1615 | `collapsible_if` in `runtime.rs` | #1612 |
| #1603 | `chunks_exact_to_as_chunks`, 12 crates | assorted |

Each fixed symptoms; none could prevent the next. Formatting is the
self-perpetuating one — an unpinned rustfmt means whoever formats next
re-flows the file back the other way, so the tree oscillates.

## How I reproduced it

I read the version out of a failing job log rather than guessing, then
installed it:

```
rustup toolchain install 1.98.0    # -> rustc 1.98.0 (88d9e12ae 2026-08-18), byte-identical build hash
```

That is what turned "unreproducible locally" into reproducible, and it
is what made #1609 and #1603 diagnosable at all.

## What I changed

**`rust-toolchain.toml`** (new) — `channel = "1.98.0"`, `profile =
"minimal"`, `components = ["clippy", "rustfmt", "llvm-tools-preview"]`.
No `targets` key: `Rust (Windows ARM64)` runs natively on
`windows-11-arm` and needs no cross target, and
`scripts/check_cross_compile.sh` adds its own.

**`ci.yml`** — the eight setup steps collapse from `rustup toolchain
install stable --profile minimal --component …` + `rustup default
stable` to a bare `rustup toolchain install`, which reads the file and
installs the declared components. The `version=` cache-key output is
unchanged. Policy comment rewritten.

**`ci.yml`, `changes` job** — separate bug, fixed while here. It was
gated on `github.event_name == 'pull_request' || 'push'`, with a comment
claiming schedule and `workflow_dispatch` runs would skip only that job
and still get full CI. **They did not.** Every job below is `needs:
changes` with a plain `if:` (no `always()`/`!cancelled()`), and GitHub
skips a job whose `needs` dependency was skipped regardless of its own
`if:`. So the gate skipped the *entire workflow*: the nightly cron and
every manual dispatch reported success without running a single check —
a green with no verdict behind it, which is the same class of problem
this PR is about. The classify step already leaves `docs_only=false` for
any event it does not diff, so removing the gate makes the documented
behaviour real.

## How I verified it

All local, on this branch, with my rustup default left at 1.97.0 so the
file is doing the work:

**Negative control — the pin is what changes the resolution:**

```
without rust-toolchain.toml:  rustc 1.97.0   clippy 0.1.97   rustfmt 1.9.0 (2d8144b788)
with    rust-toolchain.toml:  rustc 1.98.0   clippy 0.1.98   rustfmt 1.9.0 (88d9e12ae1)
```

clippy `0.1.97` → `0.1.98` is precisely the gap that made these failures
invisible to contributors.

**Both gates, run with bare commands** (no `+1.98.0`), which is the
point — a 1.97.0 contributor now gets CI's behaviour by default:

- `cargo fmt --all -- --check` → clean
- the verbatim 30-package clippy gate from `ci.yml` ~326, including the
trailing `-- -D warnings` → **exit 0**

**Component auto-install:** `llvm-tools` was absent from my 1.98.0
install and rustup fetched it on first use inside the repo, so dropping
the per-job `--component` flags is safe. Confirmed end-to-end with
`cargo llvm-cov --locked -p onnx-runtime-cpuinfo` → exit 0, which is the
one job whose component requirement changed.

**rustup override semantics, empirically checked** (I had this wrong
earlier and said so on #1600): `rustup component add` and `rustup target
add` **respect the directory pin** — I expected them to hit the default
toolchain and silently break the coverage and cross-compile jobs. They
do not; both landed on 1.98.0. Explicit `+toolchain` still beats the
file (`rustc +stable -Vv` → 1.97.0 inside the repo), so miri's `cargo
+nightly` is unaffected.

**YAML:** all workflow files parse; `changes` confirmed to have no `if:`
key.

## What I could not verify

- **Everything about the CI runners themselves.** I am on macOS aarch64.
That a Linux or Windows runner materializes 1.98.0 from this file, and
that the cache key still resolves correctly there, is only checkable in
CI. This PR's own run is the oracle.
- **The `workflow_dispatch`/`schedule` fix.** I verified the mechanism
by reading the gating (all 8 downstream jobs are `needs: changes` with
plain `if:`), but I have not observed a dispatch run execute the matrix.
Worth triggering one after merge to confirm.
- **Other workflows.** `audit.yml`, `publish.yml`,
`publish-ep-plugins.yml`, `wheels.yml` still say `rustup default
stable`. They inherit the pin anyway — any `cargo` run inside the repo
resolves through the file — so their behaviour is already correct and
their explicit install is merely redundant. I left them rather than
widen the blast radius onto release workflows. **One exception worth
flagging:** `benchmark.yml` uses `dtolnay/rust-toolchain@stable`, which
sets `RUSTUP_TOOLCHAIN` — that env var *overrides* the file, so
benchmark is genuinely not pinned. It is non-blocking, but it is a real
gap, not an oversight.

## Related, not fixed here

**No CI job runs clippy on macOS.** `rust-coverage` matrixes macOS but
only *installs* clippy (~line 458) and never runs it; all six
clippy-running jobs are Linux/Windows. That means #1609's macOS-only
fixes in `accelerate_gemm.rs` and `matmul.rs` (both `#[cfg(target_os =
"macos")]`) were structurally unverifiable by CI. I ran CI's exact
clippy package list on macOS under 1.98.0 → **exit 0**, so this is a
prevention gap rather than an outstanding defect. Filing separately.

## Conflict risk with #1579

None. This PR touches only `.github/workflows/ci.yml` and a new root
file. No overlap with `crates/onnx-runtime-ep-cpu`,
`crates/onnx-runtime-ir`, or
`crates/onnx-genai-engine/src/{decode,native_decode}/`.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 08760f2f-160f-41e5-828d-9d9b6045c00d
justinchuby added a commit that referenced this pull request Aug 21, 2026
…tionale (#1627)

The disk cache I added in #1612 had no eviction of any kind. I knew this
--
one of my own test comments called it "a directory nothing ever prunes"
--
and did not report it.

That matters more than it looks, because invalidation and reclamation
are
different things here. Every event that changes a cache key (an NVRTC
upgrade, an architecture change, or simply editing a kernel while
iterating)
leaves the previous generation permanently unreachable but still on
disk. A
generation is 21 files / 3.8 MB, and a developer touching a kernel mints
one
per edit. Nothing ever took them back.

`store_in` now prunes oldest-first to a budget after each write:
`ONNX_GENAI_KERNEL_CACHE_MAX_BYTES`, default 256 MiB, 0 meaning
unbounded.
Failures are ignored throughout, as everywhere else in this module: a
cache
that cannot tidy itself must still serve.

Also corrects the doc comment justifying FNV-1a over `DefaultHasher`. It
claimed the harm was that a toolchain bump "would silently empty the
cache".
That framing is wrong. Discarding the cache is not the harm -- this key
discards on purpose whenever NVRTC, the architecture, the include paths
or
the source change, because those decide whether an artifact is still
correct. The actual objections are that a Rust release has no bearing on
whether cached PTX is valid; that `DefaultHasher` guarantees neither
stability nor change between releases, so it cannot be relied upon in
either
direction; and that a key change does not empty anything at all -- it
orphans. Which is precisely the hole this commit closes.

Verification:
- 13 kernel_cache tests pass, 4 new: oldest-first eviction, a cache
already
under budget losing nothing, a zero budget evicting nothing, and pruning
reached through the real `store_in` path so the wiring itself is
guarded.
- Whole lib suite 471 passed / 0 failed.
- End-to-end with a deliberately tiny 1 MiB budget: cache held at 532
KiB,
  p50 inter-token 23.4 ms, i.e. unchanged.
- End-to-end at the default budget: cold max stall 511 ms, warm 261 ms,
so
the warm-cache win from #1612 survives pruning. 21 files / 3.8 MB, well
  under budget, so the default costs nothing.

The one clippy error in this crate (`consts::PI` in optimizer.rs:4726)
predates this branch and is untouched.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 0190e2eb-abe4-451f-b36d-44a035a99b7e

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 0190e2eb-abe4-451f-b36d-44a035a99b7e
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant