Skip to content

Conversation

@bnellnm
Copy link
Collaborator

@bnellnm bnellnm commented Oct 12, 2025

Purpose

Cache result of disable_inplace

cc @youkaichao , @mgoin , @ProExpertProg

Test Plan

Test Result


Essential Elements of an Effective PR Description Checklist
  • The purpose of the PR, such as "Fix some issue (link existing issues this PR will resolve)".
  • The test plan, such as providing test command.
  • The test results, such as pasting the results comparison before and after, or e2e results
  • (Optional) The necessary documentation update, such as updating supported_models.md and examples for a new model.
  • (Optional) Release notes update. If your change is user facing, please update the release notes draft in the Google Doc.

Signed-off-by: Bill Nell <bnell@redhat.com>
Copy link
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 aims to optimize the disable_inplace function by caching its result, which is a good performance improvement as the function's return value is constant during program execution. However, the implementation uses functools.cache, a feature introduced in Python 3.9. Since vLLM supports Python 3.8, this will lead to a runtime error. I've suggested a fix to use the compatible @functools.lru_cache(maxsize=None) decorator to ensure compatibility with Python 3.8.

@bnellnm bnellnm changed the title cache result of disable_inplace [Misc] cache result of disable_inplace Oct 12, 2025
@bnellnm bnellnm force-pushed the cache-disable-inplace branch from 0d8b746 to 403bd63 Compare October 12, 2025 20:51
Signed-off-by: Bill Nell <bnell@redhat.com>
@mgoin mgoin added performance Performance-related issues ready ONLY add when PR is ready to merge/full CI is needed labels Oct 12, 2025
@mgoin mgoin enabled auto-merge (squash) October 12, 2025 22:24
@mgoin mgoin merged commit 60e419c into vllm-project:main Oct 13, 2025
60 checks passed
1994 pushed a commit to 1994/vllm that referenced this pull request Oct 14, 2025
Signed-off-by: Bill Nell <bnell@redhat.com>
Signed-off-by: 1994 <1994@users.noreply.github.com>
Dhruvilbhatt pushed a commit to Dhruvilbhatt/vllm that referenced this pull request Oct 14, 2025
Signed-off-by: Bill Nell <bnell@redhat.com>
Signed-off-by: Dhruvil Bhatt <bhattdbh@amazon.com>
bbartels pushed a commit to bbartels/vllm that referenced this pull request Oct 16, 2025
Signed-off-by: Bill Nell <bnell@redhat.com>
Signed-off-by: bbartels <benjamin@bartels.dev>
lywa1998 pushed a commit to lywa1998/vllm that referenced this pull request Oct 20, 2025
Signed-off-by: Bill Nell <bnell@redhat.com>
alhridoy pushed a commit to alhridoy/vllm that referenced this pull request Oct 24, 2025
Signed-off-by: Bill Nell <bnell@redhat.com>
xuebwang-amd pushed a commit to xuebwang-amd/vllm that referenced this pull request Oct 24, 2025
Signed-off-by: Bill Nell <bnell@redhat.com>
xuebwang-amd pushed a commit to xuebwang-amd/vllm that referenced this pull request Oct 24, 2025
Signed-off-by: Bill Nell <bnell@redhat.com>
Signed-off-by: xuebwang-amd <xuebwang@amd.com>
0xrushi pushed a commit to 0xrushi/vllm that referenced this pull request Oct 26, 2025
Signed-off-by: Bill Nell <bnell@redhat.com>
Signed-off-by: 0xrushi <6279035+0xrushi@users.noreply.github.com>
0xrushi pushed a commit to 0xrushi/vllm that referenced this pull request Oct 26, 2025
Signed-off-by: Bill Nell <bnell@redhat.com>
Signed-off-by: 0xrushi <6279035+0xrushi@users.noreply.github.com>
rtourgeman pushed a commit to rtourgeman/vllm that referenced this pull request Nov 10, 2025
Signed-off-by: Bill Nell <bnell@redhat.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

performance Performance-related issues 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.

3 participants