Skip to content

Revert "[Disagg][Perf] Use NPU event sync instead of blocking tolist#3194

Merged
Yikun merged 1 commit intovllm-project:mainfrom
wangxiyuan:revert_2788
Sep 25, 2025
Merged

Revert "[Disagg][Perf] Use NPU event sync instead of blocking tolist#3194
Yikun merged 1 commit intovllm-project:mainfrom
wangxiyuan:revert_2788

Conversation

@wangxiyuan
Copy link
Copy Markdown
Collaborator

@wangxiyuan wangxiyuan commented Sep 25, 2025

…to avoid unintentional copy ops blocking across different NPU streams, improving disagg TTIT/TTFT (#2788)"

What this PR does / why we need it?

This reverts commit 6995a7b. We'll add it back once the issue is fixed.

related issue: #3195

How was this patch tested?

…to avoid unintentional copy ops blocking across different NPU streams, improving disagg TTIT/TTFT (vllm-project#2788)"

This reverts commit 6995a7b.
@github-actions
Copy link
Copy Markdown
Contributor

👋 Hi! Thank you for contributing to the vLLM Ascend project. The following points will speed up your PR merge:‌‌

  • A PR should do only one thing, smaller PRs enable faster reviews.
  • Every PR should include unit tests and end-to-end tests ‌to ensure it works and is not broken by other future PRs.
  • Write the commit message by fulfilling the PR description to help reviewer and future developers understand.

If CI fails, you can run linting and testing checks locally according Contributing and Testing.

@wangxiyuan wangxiyuan added ready read for review ready-for-test start test by label for PR labels Sep 25, 2025
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request reverts a previous performance optimization that used NPU event synchronization to avoid blocking tolist() calls. The changes correctly remove the custom _to_list implementation and its associated test, reverting to the standard tolist() method. The revert is clean and complete, with all related code and tests being removed or reverted as expected. While this reintroduces a performance bottleneck for disaggregated setups, it is presumably done to fix a correctness or stability issue introduced by the optimization. No issues were found in the revert itself.

@wangxiyuan wangxiyuan changed the title Revert "[Disagg][Perf] Use NPU event sync instead of blocking tolist … Revert "[Disagg][Perf] Use NPU event sync instead of blocking tolist Sep 25, 2025
@Yikun Yikun merged commit 0794f64 into vllm-project:main Sep 25, 2025
48 checks passed
huangdong2022 pushed a commit to huangdong2022/vllm-ascend that referenced this pull request Sep 26, 2025
…llm-project#3194)

…to avoid unintentional copy ops blocking across different NPU streams,
improving disagg TTIT/TTFT (vllm-project#2788)"

### What this PR does / why we need it?
This reverts commit 6995a7b. We'll add
it back once the issue is fixed.

related issue: vllm-project#3195

### How was this patch tested?

- vLLM version: v0.10.2
- vLLM main:
vllm-project/vllm@52d0cb8

Signed-off-by: huangdong2022 <huangdong51@huawei.com>
Angazenn pushed a commit to Angazenn/vllm-ascend that referenced this pull request Oct 21, 2025
…llm-project#3194)

…to avoid unintentional copy ops blocking across different NPU streams,
improving disagg TTIT/TTFT (vllm-project#2788)"



### What this PR does / why we need it?
This reverts commit 6995a7b. We'll add
it back once the issue is fixed.

related issue: vllm-project#3195

### How was this patch tested?

- vLLM version: v0.10.2
- vLLM main:
vllm-project/vllm@52d0cb8
@wangxiyuan wangxiyuan deleted the revert_2788 branch November 18, 2025 08:59
luolun pushed a commit to luolun/vllm-ascend that referenced this pull request Nov 19, 2025
…llm-project#3194)

…to avoid unintentional copy ops blocking across different NPU streams,
improving disagg TTIT/TTFT (vllm-project#2788)"



### What this PR does / why we need it?
This reverts commit 6995a7b. We'll add
it back once the issue is fixed.

related issue: vllm-project#3195

### How was this patch tested?

- vLLM version: v0.10.2
- vLLM main:
vllm-project/vllm@52d0cb8
Signed-off-by: luolun <luolun1995@cmbchina.com>
luolun pushed a commit to luolun/vllm-ascend that referenced this pull request Nov 19, 2025
…llm-project#3194)

…to avoid unintentional copy ops blocking across different NPU streams,
improving disagg TTIT/TTFT (vllm-project#2788)"

### What this PR does / why we need it?
This reverts commit 6995a7b. We'll add
it back once the issue is fixed.

related issue: vllm-project#3195

### How was this patch tested?

- vLLM version: v0.10.2
- vLLM main:
vllm-project/vllm@52d0cb8

Signed-off-by: luolun <luolun1995@cmbchina.com>
hwhaokun pushed a commit to hwhaokun/vllm-ascend that referenced this pull request Nov 19, 2025
…llm-project#3194)

…to avoid unintentional copy ops blocking across different NPU streams,
improving disagg TTIT/TTFT (vllm-project#2788)"

### What this PR does / why we need it?
This reverts commit 6995a7b. We'll add
it back once the issue is fixed.

related issue: vllm-project#3195

### How was this patch tested?

- vLLM version: v0.10.2
- vLLM main:
vllm-project/vllm@52d0cb8

Signed-off-by: hwhaokun <haokun0405@163.com>
NSDie pushed a commit to NSDie/vllm-ascend that referenced this pull request Nov 24, 2025
…llm-project#3194)

…to avoid unintentional copy ops blocking across different NPU streams,
improving disagg TTIT/TTFT (vllm-project#2788)"

### What this PR does / why we need it?
This reverts commit 6995a7b. We'll add
it back once the issue is fixed.

related issue: vllm-project#3195

### How was this patch tested?

- vLLM version: v0.10.2
- vLLM main:
vllm-project/vllm@52d0cb8

Signed-off-by: nsdie <yeyifan@huawei.com>
Clorist33 pushed a commit to Clorist33/vllm-ascend that referenced this pull request Dec 9, 2025
…llm-project#3194)

…to avoid unintentional copy ops blocking across different NPU streams,
improving disagg TTIT/TTFT (vllm-project#2788)"



### What this PR does / why we need it?
This reverts commit 6995a7b. We'll add
it back once the issue is fixed.

related issue: vllm-project#3195

### How was this patch tested?

- vLLM version: v0.10.2
- vLLM main:
vllm-project/vllm@52d0cb8
yangzhe-2026 pushed a commit to yangzhe-2026/vllm-ascend that referenced this pull request May 6, 2026
…llm-project#3194)

…to avoid unintentional copy ops blocking across different NPU streams,
improving disagg TTIT/TTFT (vllm-project#2788)"



### What this PR does / why we need it?
This reverts commit 6995a7b. We'll add
it back once the issue is fixed.

related issue: vllm-project#3195

### How was this patch tested?

- vLLM version: v0.10.2
- vLLM main:
vllm-project/vllm@52d0cb8
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

module:tests ready read for review ready-for-test start test by label for PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants