[Metrics] Log multi-modal cache stats #26285
Conversation
Signed-off-by: DarkLight1337 <[email protected]>
DarkLight1337
requested review from
WoosukKwon,
robertgshaw2-redhat,
njhill,
ywang96,
comaniac,
alexm-redhat,
heheda12345,
ApostaC,
simon-mo,
aarnphm and
NickLucche
as code owners
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces metrics for the multi-modal cache, including query and hit counts. The implementation involves a nice refactoring of the existing prefix caching metrics into a more generic CachingMetrics class, which is then used for both prefix and multi-modal cache statistics. The changes are well-structured and the tests have been updated to cover both text and multimodal models. I found one issue in the test suite where an assertion relies on a coincidence in test data, which I've commented on.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
vllm/vllm/v1/core/kv_cache_manager.py
Lines 188 to 194 in db606d9
Restore preempted prefix cache fields
The refactor in PrefixCacheStats removes the preempted_requests, preempted_queries, and preempted_hits attributes, but KVCacheManager.get_computed_blocks still increments those fields when a request is preempted. Any preempted request with stats logging enabled will now raise AttributeError: 'PrefixCacheStats' object has no attribute 'preempted_requests' before the scheduler returns, breaking request handling whenever preemption occurs. Either keep these attributes on the stats object or stop writing to them.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Signed-off-by: DarkLight1337 <[email protected]>
Signed-off-by: DarkLight1337 <[email protected]>
Signed-off-by: DarkLight1337 <[email protected]>
Signed-off-by: DarkLight1337 <[email protected]>
Signed-off-by: DarkLight1337 <[email protected]>
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces metrics for multi-modal cache statistics and refactors the testing framework to support both text and multi-modal models, removing obsolete v0 engine logic. The changes are well-structured, particularly the generalization of caching metrics. However, I've identified a critical issue in one of the metric tests where an expected value is calculated incorrectly, which could lead to false positives in testing.
Signed-off-by: DarkLight1337 <[email protected]>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
|
I'm unable to get |
DarkLight1337
changed the title
|
Also cc @njhill |
Signed-off-by: DarkLight1337 <[email protected]>
Purpose
This is a redo of #16478 . It is much simplified from the original PR as we have recently consolidated the code related to multi-modal cache. Also, we now only log the stats of the cache from P0, which addresses the concern about IPC overhead.
I have also removed all of the dead V0 logic from the metrics tests.
cc @markmc
Test Plan
Test Result
Essential Elements of an Effective PR Description Checklist
supported_models.mdandexamplesfor a new model.