Skip to content

[BugFix] Fix order of compile logging#38012

Merged
zou3519 merged 1 commit intovllm-project:mainfrom
zou3519:reorder_compile_logs
Mar 24, 2026
Merged

[BugFix] Fix order of compile logging#38012
zou3519 merged 1 commit intovllm-project:mainfrom
zou3519:reorder_compile_logs

Conversation

@zou3519
Copy link
Copy Markdown
Collaborator

@zou3519 zou3519 commented Mar 24, 2026

Previously the logging went:

torch.compile took X s in total
Directly load AOT compilation from path ...
Initial profiling/warmup run took X s

This was a little weird because loading the AOT compilation is a part of "torch.compile".

This PR fixes it to

Directly load AOT compilation from path ...
torch.compile took X s in total
Initial profiling/warmup run took X s

Previously the logging went:
```
torch.compile took X s in total
Directly load AOT compilation from path ...
Initial profiling/warmup run took X s
```
This was a little weird because loading the AOT compilation is a part of
"torch.compile".

This PR fixes it to
```
Directly load AOT compilation from path ...
torch.compile took X s in total
Initial profiling/warmup run took X s
```

Signed-off-by: Richard Zou <zou3519@gmail.com>
@zou3519
Copy link
Copy Markdown
Collaborator Author

zou3519 commented Mar 24, 2026

cc @zhxchen17 @gmagogsfm

@mergify mergify bot added the bug Something isn't working label Mar 24, 2026
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 addresses a bug in the AOT compilation loading logic. The change correctly moves the counter increment for loaded AOT artifacts and the corresponding log message to be inside the conditional block that checks for the existence of the compiled artifact. This ensures that the metric and log are only updated when an artifact is actually loaded from the cache, fixing a bug where they would be updated even if the cache was missed. While the PR description focuses on logging order, the core of this change is a correctness fix for when the loading log and metric are recorded. The implementation is correct and improves the reliability of compilation metrics and logs.

compilation_counter.num_aot_artifacts_loaded += 1
logger.info(
"Directly load AOT compilation from path %s", aot_compilation_path
)
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

nit: why changing indent here?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

the context manager around it prints the "torch.compile takes Xs in total" when it exists

@zou3519 zou3519 added the ready ONLY add when PR is ready to merge/full CI is needed label Mar 24, 2026
@zou3519 zou3519 enabled auto-merge (squash) March 24, 2026 16:56
@zou3519 zou3519 merged commit 71a4a2f into vllm-project:main Mar 24, 2026
61 checks passed
RhizoNymph pushed a commit to RhizoNymph/vllm that referenced this pull request Mar 26, 2026
Signed-off-by: Richard Zou <zou3519@gmail.com>
HenryTangDev pushed a commit to HenryTangMain/vllm that referenced this pull request Mar 27, 2026
Signed-off-by: Richard Zou <zou3519@gmail.com>
malaiwah pushed a commit to malaiwah/vllm that referenced this pull request Mar 27, 2026
Signed-off-by: Richard Zou <zou3519@gmail.com>
Signed-off-by: Michel Belleau <michel.belleau@malaiwah.com>
khairulkabir1661 pushed a commit to khairulkabir1661/vllm that referenced this pull request Mar 27, 2026
Signed-off-by: Richard Zou <zou3519@gmail.com>
Monishver11 pushed a commit to Monishver11/vllm that referenced this pull request Mar 27, 2026
Signed-off-by: Richard Zou <zou3519@gmail.com>
Signed-off-by: Monishver Chandrasekaran <monishverchandrasekaran@gmail.com>
nithinvc pushed a commit to nithinvc/vllm that referenced this pull request Mar 27, 2026
Signed-off-by: Richard Zou <zou3519@gmail.com>

Signed-off-by: Nithin Chalapathi <nithin.ch10@gmail.com>
JiantaoXu pushed a commit to JiantaoXu/vllm that referenced this pull request Mar 28, 2026
Signed-off-by: Richard Zou <zou3519@gmail.com>
vrdn-23 pushed a commit to vrdn-23/vllm that referenced this pull request Mar 30, 2026
Signed-off-by: Richard Zou <zou3519@gmail.com>
Signed-off-by: Vinay Damodaran <vrdn@hey.com>
EricccYang pushed a commit to EricccYang/vllm that referenced this pull request Apr 1, 2026
Signed-off-by: Richard Zou <zou3519@gmail.com>
Signed-off-by: EricccYang <yangyang4991@gmail.com>
bhargav-patel-29 pushed a commit to Bharatgen-Tech/vllm that referenced this pull request Apr 1, 2026
Signed-off-by: Richard Zou <zou3519@gmail.com>
Signed-off-by: bhargav-patel-29 <bhargav.patel@tihiitb.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working ready ONLY add when PR is ready to merge/full CI is needed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants