Skip to content
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 account

Jump to bottom

[torch.compile] add logging for compilation time #10941

Merged
merged 16 commits into from
Dec 6, 2024
Merged

[torch.compile] add logging for compilation time #10941

merged 16 commits into from
Dec 6, 2024

Conversation

youkaichao
Copy link
Member

No description provided.

@youkaichao
compute compilation time
b9a0328 
Signed-off-by: youkaichao <[email protected]>
@youkaichao
fix one graph
c7b4248 
Signed-off-by: youkaichao <[email protected]>
@github-actions GitHub Actions
Copy link

github-actions bot commented Dec 6, 2024

👋 Hi! Thank you for contributing to the vLLM project.
Just a reminder: PRs would not trigger full CI run by default. Instead, it would only run fastcheck CI which starts running only a small and essential subset of CI tests to quickly catch errors. You can run other CI tests on top of those by going to your fastcheck build on Buildkite UI (linked in the PR checks section) and unblock them. If you do not have permission to unblock, ping simon-mo or khluu to add you in our Buildkite org.

Once the PR is approved and ready to go, your PR reviewer(s) can run CI to test the changes comprehensively before merging.

To run CI, PR reviewers can do one of these:

  • Add ready label to the PR
  • Enable auto-merge.

🚀

@youkaichao
update name
70e6b4e 
Signed-off-by: youkaichao <[email protected]>
@youkaichao
Copy link
Member Author

example output (for the toy tests):

Compiling a graph for general shape takes 1.55 s

see https://buildkite.com/vllm/fastcheck/builds/9379#01939a08-6359-4659-9b3c-c07576a7efee/6465-8353

@youkaichao youkaichao requested a review from WoosukKwon December 6, 2024 04:33
@youkaichao youkaichao marked this pull request as draft December 6, 2024 05:08
@youkaichao
add context manager
3044d17 
Signed-off-by: youkaichao <[email protected]>
@youkaichao
dirty impl
8c0ac0b 
Signed-off-by: youkaichao <[email protected]>
@youkaichao
use api
0f6719b 
Signed-off-by: youkaichao <[email protected]>
@youkaichao
fi
2bce8de 
Signed-off-by: youkaichao <[email protected]>
@youkaichao
use timing at engine level
2b65379 
Signed-off-by: youkaichao <[email protected]>
@youkaichao
engine level logging
81914b1 
Signed-off-by: youkaichao <[email protected]>
@youkaichao
remove the call in the worker
7ab3f8b 
Signed-off-by: youkaichao <[email protected]>
@youkaichao
revert
10c88ee 
Signed-off-by: youkaichao <[email protected]>
@youkaichao
fix logging
6f39058 
Signed-off-by: youkaichao <[email protected]>
@youkaichao
fix symbolic only
70e3534 
Signed-off-by: youkaichao <[email protected]>
@youkaichao
fix format
fa36de5 
Signed-off-by: youkaichao <[email protected]>
@youkaichao
Copy link
Member Author

youkaichao commented Dec 6, 2024 •
edited
Loading

this pr aims to add three logging information:

  1. engine-level initialization, e.g.

INFO 12-05 22:37:22 llm_engine.py:493] init engine (profile, create kv cache, warmup model) took 15.08 seconds
INFO 12-05 22:39:24 llm_engine.py:493] init engine (profile, create kv cache, warmup model) took 40.77 seconds
INFO 12-05 22:41:51 llm_engine.py:493] init engine (profile, create kv cache, warmup model) took 50.18 seconds

  1. graph compilation for every shape (including the symbolic shape compilation), e.g.

INFO 12-05 22:39:02 backends.py:55] Compiling a graph for general shape takes 14.73 s
INFO 12-05 22:41:51 backends.py:58] Compiling a graph for shape 1 takes 9.99 s

  1. aggregation of the numbers in 2, e.g.

INFO 12-05 22:39:04 monitor.py:13] graph compilation takes 14.73 s in total
INFO 12-05 22:41:51 monitor.py:13] graph compilation takes 24.758146286010742 s in total

how to read it:

the increase in 1, when using or not using torch.compile , is the total cost of torch.compile .

2 shows the cost of every shape compilation, so that users can select it according to their budget. Note that some compilation like Dynamo bytecode compilation and triton compilation are not considered here.

3 just aggregates 2.

@youkaichao youkaichao marked this pull request as ready for review December 6, 2024 06:43
WoosukKwon
WoosukKwon approved these changes Dec 6, 2024
View reviewed changes
Copy link
Collaborator

@WoosukKwon WoosukKwon left a comment

Choose a reason for hiding this comment

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

Honestly, I don't have enough familiarity to the code for proper review. However, the code apparently looks OK to me. Please feel free to merge.

vllm/compilation/backends.py Outdated Show resolved Hide resolved
vllm/compilation/monitor.py
Comment on lines +12 to +14
if compilation_config.level == CompilationLevel.PIECEWISE:
logger.info("graph compilation takes %.2f s in total",
compilation_config.compilation_time)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Dumb question: Why does it print only for pw CUDA graphs?

Copy link
Member Author

Choose a reason for hiding this comment

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

CompilationLevel.PIECEWISE is piecewise compile, not piecewise cudagraph. this is orthogonal to cudagraph.

vllm/compilation/backends.py
@@ -108,6 +120,8 @@ def split_graph(graph: fx.GraphModule,
# we share the global graph pool among all the backends
global_graph_pool = None

compilation_start_time = 0.0
Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe None instead of 0.0?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Just wondering: can we somehow make this more robust? Since the code touching this var is scattered into different places, I feel it's error prone...

Copy link
Member Author

Choose a reason for hiding this comment

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

that's why I want to merge some functions in the worker/executor. however, given the current code status, i don't have bandwidth to refactor those files.

@WoosukKwon
Copy link
Collaborator

INFO 12-05 22:41:51 monitor.py:13] graph compilation takes 24.758146286010742 s in total

Please make sure to use %2f for this log (if it hasn't been fixed yet).

youkaichao and others added 2 commits December 5, 2024 23:39
@youkaichao @WoosukKwon
Update vllm/compilation/backends.py
d87ac20 
Co-authored-by: Woosuk Kwon <[email protected]>
@youkaichao
merge code
f502121 
Signed-off-by: youkaichao <[email protected]>
@youkaichao
Copy link
Member Author

@WoosukKwon thanks for the review!

@youkaichao youkaichao enabled auto-merge (squash) December 6, 2024 08:08
@github-actions github-actions bot added the ready ONLY add when PR is ready to merge/full CI is needed label Dec 6, 2024
@youkaichao youkaichao merged commit b031a45 into vllm-project:main Dec 6, 2024
65 checks passed
@youkaichao youkaichao deleted the compilation_time branch December 6, 2024 16:18
sleepwalker2017 pushed a commit to sleepwalker2017/vllm that referenced this pull request Dec 13, 2024
@youkaichao @WoosukKwon
[torch.compile] add logging for compilation time (vllm-project#10941)
ee718e1 
Signed-off-by: youkaichao <[email protected]>
Co-authored-by: Woosuk Kwon <[email protected]>
BKitor pushed a commit to BKitor/vllm that referenced this pull request Dec 30, 2024
@youkaichao @WoosukKwon @BKitor
[torch.compile] add logging for compilation time (vllm-project#10941)
3f78eb4 
Signed-off-by: youkaichao <[email protected]>
Co-authored-by: Woosuk Kwon <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@WoosukKwon WoosukKwon WoosukKwon approved these changes

@robertgshaw2-redhat robertgshaw2-redhat Awaiting requested review from robertgshaw2-redhat robertgshaw2-redhat is a code owner

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

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

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

@alexm-redhat alexm-redhat Awaiting requested review from alexm-redhat alexm-redhat is a code owner

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

Assignees
No one assigned
Labels
ready ONLY add when PR is ready to merge/full CI is needed
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@youkaichao @WoosukKwon