8000 feat: multi-thread (via asyncio.task) in processor by tedzhouhk · Pull Request #904 · ai-dynamo/dynamo · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

feat: multi-thread (via asyncio.task) in processor #904

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your accou 8000 nt

Jump to bottom
Merged
merged 4 commits into from
May 5, 2025
Merged

feat: multi-thread (via asyncio.task) in processor #904

merged 4 commits into from
May 5, 2025

Conversation

tedzhouhk
Copy link
Contributor
@tedzhouhk tedzhouhk commented Apr 29, 2025 •
edited
Loading

Implement an asyncio tasks + queue architecture in processor.py to improve tokenization perf at high load. This partially solves #873

  • Theoretical dp4 throughput (raw vllm serve x4): 1429.44
  • Throughput without multi-thread: 1142.65 +- 71.38
  • Throughput with this PR: 1374.49 +- 9.63 (20.3% improv)

Note that by eyeballing htop, we're still GIL-bounded. Not sure if the vllm tokenization process release GIL or not.

Another benefit of the queue architecture is that it naturally enables queuing request at processor instead of engine.

@copy-pr-bot copy-pr-bot
Copy link
copy-pr-bot bot commented Apr 29, 2025

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

rmccorm4
rmccorm4 reviewed Apr 29, 2025
View reviewed changes
rmccorm4
rmccorm4 reviewed Apr 29, 2025
View reviewed changes
rmccorm4
rmccorm4 approved these changes Apr 30, 2025
View reviewed changes
Copy link
Contributor
@rmccorm4 rmccorm4 left a comment

Choose a reason for hiding this comment

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

Generally I'm not sold on the approach of this change as described here: https://github.com/ai-dynamo/dynamo/pull/904/files#r2069528912

But I don't want to block you from improving the performance if you deem this strategy necessary over just increasing the number of processor replicas.

@tedzhouhk tedzhouhk marked this pull request as draft May 2, 2025 00:48
@tedzhouhk tedzhouhk changed the title feat: multi-thread in processor feat: multi-thread (via asyncio.task) in processor May 2, 2025
@tedzhouhk tedzhouhk force-pushed the hzhou/multithread-proc branch from 288915f to 6f8fdf0 Compare May 2, 2025 01:13
@tedzhouhk tedzhouhk assigned tedzhouhk and unassigned tedzhouhk May 2, 2025
@tedzhouhk tedzhouhk marked this pull request as ready for review May 2, 2025 01:14
@tedzhouhk
Copy link
Contributor Author

Set to auto-merge. Note that multi-process/thread/asycio-task are all temporary solution before we refactor processor in Rust.

@tedzhouhk tedzhouhk merged commit e0cd848 into main May 5, 2025
6 checks passed
@tedzhouhk tedzhouhk deleted the hzhou/multithread-proc branch May 5, 2025 20:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rmccorm4 rmccorm4 rmccorm4 approved these changes

@GuanLuo GuanLuo GuanLuo approved these changes

@hutm hutm Awaiting requested review from hutm hutm is a code owner

@biswapanda biswapanda Awaiting requested review from biswapanda biswapanda is a code owner

@ishandhanani ishandhanani Awaiting requested review from ishandhanani ishandhanani is a code owner

@julienmancuso julienmancuso Awaiting requested review from julienmancuso julienmancuso is a code owner

@hhzhang16 hhzhang16 Awaiting requested review from hhzhang16 hhzhang16 is a code owner

@nnshah1 nnshah1 Awaiting requested review from nnshah1 nnshah1 is a code owner

@tanmayv25 tanmayv25 Awaiting requested review from tanmayv25 tanmayv25 is a code owner

@piotrm-nvidia piotrm-nvidia Awaiting requested review from piotrm-nvidia piotrm-nvidia is a code owner

@ptarasiewiczNV ptarasiewiczNV Awaiting requested review from ptarasiewiczNV ptarasiewiczNV is a code owner

@ryanolson ryanolson Awaiting requested review from ryanolson ryanolson is a code owner

@grahamking grahamking Awaiting requested review from grahamking grahamking is a code owner

@paulhendricks paulhendricks Awaiting requested review from paulhendricks paulhendricks is a code owner

@tmonty12 tmonty12 Awaiting requested review from tmonty12 tmonty12 is a code owner

@alec-flowers alec-flowers Awaiting requested review from alec-flowers alec-flowers is a code owner

@kkranen kkranen Awaiting requested review from kkranen kkranen is a code owner

@sshchoi sshchoi Awaiting requested review from sshchoi sshchoi is a code owner

@PeaBrane PeaBrane Awaiting requested review from PeaBrane PeaBrane is a code owner

@mohammedabdulwahhab mohammedabdulwahhab Awaiting requested review from mohammedabdulwahhab mohammedabdulwahhab is a code owner

Assignees
No one assigned
Labels
size/L
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@tedzhouhk @rmccorm4 @GuanLuo
0