Skip to content

[Perf] Use the non-draining NPUStream getter in the kernel launch paths - #280

Merged
matthewygf merged 1 commit into
LMCache:mainfrom
zihanlin-ai:fix/non-draining-npu-stream
Sep 2, 2026
Merged

matthewygf merged 1 commit into
LMCache:mainfrom
zihanlin-ai:fix/non-draining-npu-stream

Conversation

@zihanlin-ai

@zihanlin-ai zihanlin-ai commented Aug 29, 2026

Copy link
Copy Markdown
Contributor

Fixes #219

What changed

Replace all six remaining
c10_npu::getCurrentNPUStream().stream() calls in kernel-launch paths with
stream(false).

stream() drains the torch_npu task queue before returning. These streams are
only consumed by OpCommand custom handlers enqueued on the current stream, so
queue and device-stream ordering already preserve dependencies. stream(false)
returns the same aclrtStream without the redundant drain.

csrc/pac_kernels.cpp already uses this form. csrc/framework_hal.cpp is
unchanged because device_index() does not drain the queue.

Validation

Environment: 910B2, CANN 8.5.1, torch_npu 2.9.0.post1.

Host-side multi_layer_kv_transfer latency over 200 iterations:

Operation Pending ops Mean (us) P99 (us) Max (us)
Scatter 0 127.8 → 18.0 163.4 → 34.1 236.6 → 34.7
Scatter 32 33.9 → 22.4 51.6 → 33.1 52.3 → 79.0
Gather 0 88.6 → 20.4 107.4 → 23.6 138.8 → 24.0
Gather 32 27.9 → 25.7 57.4 → 33.0 1098.1 → 33.1
  • Three repeats measured 85–128 us before and 16–18 us after.
  • All affected operations produced identical checksums; scatter/gather
    round trips were bit-exact.
  • Device execution time was unchanged.
  • With TASK_QUEUE_ENABLE=0, both builds measured 40–44 us.

csrc/mem_kernels.cpp and csrc/utils.cpp also overlap with #246; I can rebase
if needed.

`c10_npu::getCurrentNPUStream().stream()` calls `repo->MakeSureQueueEmpty()`,
blocking the caller until the task queue has drained. The `stream(false)`
overload returns the same `aclrtStream` without it.

At every site changed here the stream is only captured and then consumed
inside the lambda given to `OpCommand::SetCustomHandler()`, which the task
queue executes in FIFO order, so ordering against previously enqueued work is
guaranteed by the queue and the drain is redundant. `csrc/pac_kernels.cpp`
already relies on this. `csrc/framework_hal.cpp` is left alone: it calls
`device_index()`, which never drains.

Measured on 910B2, CANN 8.5.1, torch_npu 2.9.0, the host-side cost of the
`multi_layer_kv_transfer` call drops from 127.8 to 18.0 us for scatter and
88.6 to 20.4 us for gather. Every op whose stream acquisition changed keeps
its exact checksum against a build of this tree without the patch.

Fixes LMCache#219

Signed-off-by: zihanlin-ai <linzihan.huawei@gmail.com>
@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review Completed 2026-08-29T03:00:38.216017Z 32d4862 PR opened
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@zihanlin-ai

Copy link
Copy Markdown
Contributor Author

Also ran the repository's own kernel suites on 910B2 / CANN 8.5.1 / torch_npu 2.9.0.post1: tests/v1/test_mem_kernels.py plus tests/v1/test_pos_kernels.py give 341 passed on upstream main and 341 passed on this branch.

@matthewygf

Copy link
Copy Markdown
Collaborator

Thanks for the contribution, I was wondering whether you can add a simple integration test with gsm8k or the likes ? Wdyt ? @chloroethylene

@chloroethylene

Copy link
Copy Markdown
Collaborator

Could you please provide some information on the testing results regarding accuracy, such as on MMLU or GSM8K?

Copy link
Copy Markdown
Contributor Author

Validated 169ee728 vs 32d4862f on Qwen3-8B/A3 with 5-shot MMLU (250 questions, LMCache cold+warm): accuracy was 66.4% vs 66.4%, with identical predictions/token IDs. Cold p95: 50.96→50.57 ms; warm p95: 49.15→49.58 ms; total time: +0.01%. No accuracy or end-to-end performance regression observed.

@zihanlin-ai

Copy link
Copy Markdown
Contributor Author

GSM8K (250, Qwen3-8B/A3): base/head 85.6%; all outputs identical; wall +0.02%. No regression.

@zihanlin-ai

Copy link
Copy Markdown
Contributor Author

@matthewygf @chloroethylene FYI—MMLU/GSM8K results above.

@chloroethylene

Copy link
Copy Markdown
Collaborator

lgtm! thanks for your contributions!

@matthewygf
matthewygf merged commit 26ce511 into LMCache:main Sep 2, 2026
1 check passed
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.

[BUG] Replace all c10_npu::getCurrentNPUStream().stream() with the non-draining variants

3 participants