Skip to content

fix(server): answer /v1/resources during a batch instead of after it - #549

Merged
justinchuby merged 3 commits into
mainfrom
split/476-resources-during-batch
Jul 31, 2026
Merged

justinchuby merged 3 commits into
mainfrom
split/476-resources-during-batch

Conversation

@justinchuby

Copy link
Copy Markdown
Owner

Extracted from #476 so it can be reviewed on its own. Depends on nothing else in that PR.

The bug

/v1/resources reports on in-flight work, but it was queued behind that same work — so the endpoint answered only once the batch it was reporting on had finished. A monitoring endpoint that blocks until the thing it monitors completes cannot observe the state anyone would want to query.

Verification

cargo test -p onnx-genai-server --lib → 126 passed, 1 failed.

The one failure is pre-existing on main and unrelated: sidecar_free_compatibility_package_builds_server_pipeline_and_preprocesses_image fails on a missing fixture (vlm-executable/vision.onnx). Confirmed by running the same test on clean origin/main, where it fails identically.

Original commit: a6fefde2.


Split out of #476 (bug-fix extraction). Dashboard/demo work stays in that PR.

The static/scatter driver drains commands in a loop that runs until the
batch goes idle, and its catch-all pushed everything that was not a new
generation onto the deferred queue. `DriverCommand::ResourceSnapshot`
landed there, so `/v1/resources` did not return until every in-flight
generation had finished.

That inverts the endpoint's purpose: it reports batch occupancy, and it
was readable only when there was no batch left to report on. Under the
demo's 512-token generations the request appears to hang for the entire
run, which is also what the dashboard's occupancy gauge polls.

Extracts `handle_or_defer_during_batch` so the rule is named and
testable: read-only observability is answered inline, and commands
needing `&mut Engine` stay deferred because `ContinuousBatchManager`
holds the borrow.

Tested via the helper rather than over HTTP deliberately. A racing
integration test was written first and discarded: the fixtures generate
in under a millisecond, so no request can be landed inside the batch
window, and instrumenting the loop showed it never observed the probe at
all. That test passed with the bug reintroduced. The unit test fails.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@codecov

codecov Bot commented Jul 31, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 80.60%. Comparing base (cdc5af9) to head (7706e6f).
⚠️ Report is 11 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #549      +/-   ##
==========================================
+ Coverage   80.59%   80.60%   +0.01%     
==========================================
  Files         315      315              
  Lines      123446   123528      +82     
  Branches   123446   123528      +82     
==========================================
+ Hits        99487    99567      +80     
- Misses      19908    19910       +2     
  Partials     4051     4051              
Flag Coverage Δ
cli-ort-linux 83.27% <ø> (ø)
cli-ort-windows 82.78% <ø> (ø)
mlas 77.91% <ø> (ø)
offline 80.52% <ø> (+0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.
see 2 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@github-actions

github-actions Bot commented Jul 31, 2026 •

Copy link
Copy Markdown

🔴 Benchmark Regression Detected

Comparison of criterion micro-benchmarks: PR head vs merge-base, measured on the same runner in the same job (base first → PR second).

ℹ️ Absolute times are informational only — they vary with runner load. The % change column is the reliable signal because both sides ran under identical conditions.

Status Scenario Base PR Change
🔴 grammar_masking/llguidance_compute_mask/32 90.53 µs 129.95 µs +43.6%
🔴 kv_cache/alloc_dealloc_pages 36.67 µs 50.87 µs +38.7%
🔴 logit_processing/seven_processor_chain_per_step 1.45 ms 1.89 ms +30.2%
⚠️ sampling_latency/greedy_per_token 3.15 µs 4.04 µs +28.3%
⚠️ gather/large_bf16_threads=1-internal/131072 12.91 µs 16.14 µs +25.1%
⚠️ tokenization/encode_tokens_per_second 380.98 µs 452.90 µs +18.9%
⚠️ add/large_bf16_threads=1-internal/4194304 46.92 ms 55.53 ms +18.3%
⚠️ sampling_latency/top_k_per_token 466.76 µs 543.72 µs +16.5%
⚠️ sampling_latency/top_p_per_token 994.56 µs 1.15 ms +15.9%
⚠️ reduce_mean/small_f32_threads=1-internal/4096 16.30 µs 18.84 µs +15.6%
⚠️ sampling_latency/min_p_per_token 335.57 µs 386.38 µs +15.1%
✅ reduce_mean/large_f32_threads=1-internal/262144 1.01 ms 1.10 ms +9.6%
✅ gather/large_f16_threads=1-internal/131072 14.39 µs 15.61 µs +8.5%
✅ gather/large_f32_threads=1-internal/131072 31.81 µs 33.58 µs +5.6%
✅ tokenization/decode_tokens_per_second 7.10 ms 7.46 ms +5.2%
✅ add/small_f16_threads=1-internal/1024 15.29 µs 15.59 µs +2.0%
✅ matmul/medium_generic_f16_threads=1/32x512x512 30.53 µs 30.77 µs +0.8%
✅ gather/medium_f16_threads=1-internal/32768 2.72 µs 2.72 µs -0.0%
✅ matmul/large_generic_bf16_threads=1/32x1024x1024 2.00 ms 1.95 ms -2.5%
✅ gather/medium_bf16_threads=1-internal/32768 2.79 µs 2.72 µs -2.5%
✅ add/large_f32_threads=1-internal/4194304 48.57 ms 46.90 ms -3.4%
✅ add/small_bf16_threads=1-internal/1024 15.70 µs 15.07 µs -4.0%
✅ add/medium_f32_threads=1-internal/262144 3.14 ms 3.00 ms -4.6%
✅ gather/small_f32_threads=1-internal/4096 796.5 ns 759.0 ns -4.7%
✅ matmul/medium_generic_f32_threads=8/32x512x512 964.11 µs 914.63 µs -5.1%
✅ matmul/large_generic_f16_threads=8/32x1024x1024 90.50 µs 85.62 µs -5.4%
✅ matmul/small_generic_f32_threads=8/1x256x256 36.59 µs 34.21 µs -6.5%
✅ reduce_mean/medium_f32_threads=1-internal/65536 297.76 µs 278.42 µs -6.5%
✅ matmul/large_generic_f16_threads=1/32x1024x1024 87.39 µs 81.23 µs -7.1%
✅ add/small_f32_threads=1-internal/1024 254.5 ns 235.8 ns -7.4%
✅ matmul/medium_generic_f32_threads=1/32x512x512 2.53 ms 2.31 ms -8.6%
✅ matmul/large_generic_f32_threads=1/32x1024x1024 10.58 ms 9.26 ms -12.5%
✅ add/medium_f16_threads=1-internal/262144 3.54 ms 3.10 ms -12.6%
✅ matmul/small_generic_bf16_threads=1/1x256x256 37.83 µs 32.34 µs -14.5%
✅ matmul/small_generic_f32_threads=1/1x256x256 45.45 µs 38.66 µs -15.0%
🟢 matmul/small_generic_f16_threads=1/1x256x256 36.14 µs 30.55 µs -15.5%
🟢 gather/small_f16_threads=1-internal/4096 634.6 ns 534.4 ns -15.8%
🟢 matmul/large_generic_f32_threads=8/32x1024x1024 4.77 ms 3.82 ms -19.9%
🟢 matmul/small_generic_f16_threads=8/1x256x256 37.07 µs 29.45 µs -20.5%
🟢 matmul/medium_generic_f16_threads=8/32x512x512 37.95 µs 29.88 µs -21.3%
🟢 matmul/medium_generic_bf16_threads=8/32x512x512 476.95 µs 373.22 µs -21.7%
🟢 add/large_f16_threads=1-internal/4194304 67.50 ms 52.22 ms -22.6%
🟢 add/medium_bf16_threads=1-internal/262144 4.14 ms 3.10 ms -25.1%
🟢 matmul/small_generic_bf16_threads=8/1x256x256 41.83 µs 31.12 µs -25.6%
🟢 matmul/medium_generic_bf16_threads=1/32x512x512 719.55 µs 525.74 µs -26.9%
🟢 gather/medium_f32_threads=1-internal/32768 5.37 µs 3.85 µs -28.3%
🟢 matmul/large_generic_bf16_threads=8/32x1024x1024 2.34 ms 1.63 ms -30.3%
🟢 gather/small_bf16_threads=1-internal/4096 741.8 ns 478.2 ns -35.5%

Visual flags: ⚠️ ≥ 15% slower, 🔴 ≥ 30% slower — calibrated against measured runner noise (~27% worst-case on multi-threaded matmul)

Host info
CPU: Apple M1 (Virtual)
Cores: 3
OS: Darwin 25.5.0 arm64
Rust: rustc 1.97.1 (8bab26f4f 2026-07-14)
Load avg: { 3.72 3.95 6.22 }
What this cannot catch
  • Regressions in code paths not covered by these benchmarks (e.g., end-to-end decode with a real model)
  • Sub-threshold regressions that compound over multiple PRs
  • Performance changes that only manifest under GPU execution
  • Latency changes in the ORT integration path (these benchmarks exercise the native Rust kernels)

Copilot AI left a comment

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.

Pull request overview

Fixes /v1/resources responsiveness during continuous/static batching by answering resource snapshots immediately (instead of deferring them behind the in-flight batch), and adds targeted unit coverage to prevent regressions.

Changes:

  • Route DriverCommand::ResourceSnapshot through a new handle_or_defer_during_batch helper so it is answered during an active batch.
  • Keep other commands deferred until the batch drains, preserving existing driver behavior for non-observability commands.
  • Add unit tests that pin the “answer resource snapshots now, defer other commands” contract without relying on timing-sensitive HTTP integration.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
crates/onnx-genai-server/src/driver.rs Adds handle_or_defer_during_batch and uses it to answer ResourceSnapshot immediately during batch execution.
crates/onnx-genai-server/src/tests.rs Adds unit tests asserting snapshots are answered immediately and other commands remain deferred.

Comment thread crates/onnx-genai-server/src/tests.rs Outdated
Comment thread crates/onnx-genai-server/src/driver.rs Outdated
@justinchuby

Copy link
Copy Markdown
Owner Author

Will fix this later in a focused run instead

justinchuby and others added 2 commits July 31, 2026 07:06
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
@justinchuby
justinchuby merged commit 2efdaae into main Jul 31, 2026
@justinchuby
justinchuby deleted the split/476-resources-during-batch branch July 31, 2026 14:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

type:bug Something broken

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants