feat: standalone profiling script for a given endpoint #2386
Conversation
tedzhouhk
requested review from
a team,
GuanLuo,
PeaBrane,
alec-flowers,
biswapanda,
grahamking,
ishandhanani,
jthomson04,
kkranen,
nnshah1,
paulhendricks,
piotrm-nvidia,
ptarasiewiczNV,
rmccorm4,
ryanolson,
tanmayv25 and
tmonty12
as code owners
WalkthroughA new profiling script, Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant profile_endpoint.py
participant profile_prefill.py
participant profile_deocde.py
participant plot.py
User->>profile_endpoint.py: Run with CLI arguments (mode, model, etc.)
alt mode == prefill
profile_endpoint.py->>profile_prefill.py: profile_prefill(...)
profile_prefill.py->>plot.py: plot_prefill_interpolation(...)
else mode == decode
profile_endpoint.py->>profile_deocde.py: profile_decode(...)
profile_deocde.py->>plot.py: plot_decode_3d_surface(...)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~18 minutes Possibly related PRs
Poem
Note 🔌 MCP (Model Context Protocol) integration is now available in Early Access!Pro users can now connect to remote MCP servers under the Integrations page to get reviews and chat conversations that understand additional development context. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (4)
benchmarks/profiler/utils/profile_prefill.py (1)
10-18: Repeated logger handler attachment across modulesEach utility module adds an unconditional
StreamHandler, so importing them multiple times duplicates log lines.
Wrap the handler addition behindif not logger.handlers:to avoid spam.benchmarks/profiler/utils/profile_deocde.py (1)
10-18: Duplicate logging handlersSame duplication issue as in
profile_prefill.py; guard withif not logger.handlers:.benchmarks/profiler/utils/plot.py (2)
176-183:griddatawith method"cubic"requires ≥ 16 pointsIf fewer samples are recorded the cubic interpolator will raise. Either fall back to
linear/nearestor skip the surface plot with a warning.method = "cubic" if len(x_kv_usage) >= 16 else "linear" Z_itl = griddata((x_kv_usage, y_context_length), z_itl, (X, Y), method=method)
148-154: Consider fitting a curve or at least connecting pointsThroughput is now displayed as scattered dots without any visual trend, unlike TTFT.
A simple line or polynomial fit would help users interpret the data.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
benchmarks/profiler/profile_endpoint.py(1 hunks)benchmarks/profiler/profile_sla.py(3 hunks)benchmarks/profiler/utils/plot.py(3 hunks)benchmarks/profiler/utils/profile_deocde.py(1 hunks)benchmarks/profiler/utils/profile_prefill.py(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Build and Test - dynamo
🔇 Additional comments (1)
benchmarks/profiler/profile_sla.py (1)
417-425: Propagation of missing work-dir creation
profile_prefill()is called withwork_dirthat is created just above (os.makedirs(work_dir, exist_ok=True)), so OK.
No action needed.
tedzhouhk
force-pushed
the
hzhou/standalone-profile
branch
from
1bace0a to
934335f
Compare
Summary by CodeRabbit
New Features
Refactor
Style
Documentation