[TRTLLM-12670][feat] add /start_profile and /stop_profile endpoints to trtllm… - #13872
[TRTLLM-12670][feat] add /start_profile and /stop_profile endpoints to trtllm…#13872JunyiXu-nv wants to merge 8 commits into
Conversation
69b3d68 to
86b3237
Compare
|
/bot run --disable-fail-fast |
|
PR_Github #47712 [ run ] triggered by Bot. Commit: |
0b70c55 to
029a94b
Compare
|
PR_Github #47712 [ run ] completed with state
|
…ints Two follow-up fixes for the CI failures introduced by the profile endpoints PR (NVIDIA#13872): 1. API stability reference: add start_profile / stop_profile entries to tests/unittest/api_stability/references/llm.yaml so TestLLM::test_{signature,docstring,modified_init,...} stops failing with "LLM has different methods: {'start_profile', 'stop_profile'}". 2. Ray WorkerExtension collision: allow a user-supplied WorkerExtension to override BaseWorker.start_profile / stop_profile. These names are explicitly designated extension points (the RLHF WorkerExtension in tensorrt_llm/llmapi/rlhf_utils.py uses torch.cuda.profiler directly), so _inject_worker_extension now copies overridable extension methods into the derived class dict, which makes the extension's impl win regardless of MRO order and keeps the conflict check strict for every other name. Fixes the cascade of "RayGPUWorker died during initialization / Worker class RayGPUWorker already defines 'start_profile'" errors across the Ray test suites (H100, H100_PCIe, B200). Signed-off-by: JunyiXu-nv <219237550+JunyiXu-nv@users.noreply.github.com>
|
/bot run |
|
PR_Github #48187 [ run ] triggered by Bot. Commit: |
|
PR_Github #48187 [ run ] completed with state
|
The Pre-commit Check on PR NVIDIA#13872 failed with two issue families: * ruff-format (PEP-8 + 80-col yapf-equivalent) reformatted multiline asserts and f-strings in the new test files. * ruff-legacy reported 8 D205 regressions ('1 blank line required between summary line and description') vs the baseline. Some are in docstrings I added (proxy._wait_profile_ack, proxy.start_profile, proxy.stop_profile, test_e2e.test_trtllm_serve_profile_example, 4 docstrings in tests/unittest/executor/test_proxy_profile_sync.py) and a few are pre-existing violations whose line numbers shifted because of my additions (proxy.submit, test_e2e bench tests). Fix all 8 by inserting the required blank line after each docstring summary line and tightening summary lines so they are real sentences (D415 fix on the test_proxy_profile_sync.py module docstring). Also accept the ruff-format reflow on the two test files. Verified locally with: pre-commit run --files <touched files> — all hooks pass (yapf, ruff, ruff-format, ruff-legacy, autoflake, codespell, etc.). Signed-off-by: Junyi Xu <219237550+JunyiXu-nv@users.noreply.github.com>
a07a3da to
41eca47
Compare
…ints Two follow-up fixes for the CI failures introduced by the profile endpoints PR (NVIDIA#13872): 1. API stability reference: add start_profile / stop_profile entries to tests/unittest/api_stability/references/llm.yaml so TestLLM::test_{signature,docstring,modified_init,...} stops failing with "LLM has different methods: {'start_profile', 'stop_profile'}". 2. Ray WorkerExtension collision: allow a user-supplied WorkerExtension to override BaseWorker.start_profile / stop_profile. These names are explicitly designated extension points (the RLHF WorkerExtension in tensorrt_llm/llmapi/rlhf_utils.py uses torch.cuda.profiler directly), so _inject_worker_extension now copies overridable extension methods into the derived class dict, which makes the extension's impl win regardless of MRO order and keeps the conflict check strict for every other name. Fixes the cascade of "RayGPUWorker died during initialization / Worker class RayGPUWorker already defines 'start_profile'" errors across the Ray test suites (H100, H100_PCIe, B200). Signed-off-by: JunyiXu-nv <219237550+JunyiXu-nv@users.noreply.github.com>
The Pre-commit Check on PR NVIDIA#13872 failed with two issue families: * ruff-format (PEP-8 + 80-col yapf-equivalent) reformatted multiline asserts and f-strings in the new test files. * ruff-legacy reported 8 D205 regressions ('1 blank line required between summary line and description') vs the baseline. Some are in docstrings I added (proxy._wait_profile_ack, proxy.start_profile, proxy.stop_profile, test_e2e.test_trtllm_serve_profile_example, 4 docstrings in tests/unittest/executor/test_proxy_profile_sync.py) and a few are pre-existing violations whose line numbers shifted because of my additions (proxy.submit, test_e2e bench tests). Fix all 8 by inserting the required blank line after each docstring summary line and tightening summary lines so they are real sentences (D415 fix on the test_proxy_profile_sync.py module docstring). Also accept the ruff-format reflow on the two test files. Verified locally with: pre-commit run --files <touched files> — all hooks pass (yapf, ruff, ruff-format, ruff-legacy, autoflake, codespell, etc.). Signed-off-by: Junyi Xu <219237550+JunyiXu-nv@users.noreply.github.com>
|
/bot run --disable-fail-fast |
|
PR_Github #48285 [ run ] triggered by Bot. Commit: |
|
PR_Github #48285 [ run ] completed with state
|
41eca47 to
fcb8426
Compare
…ints Two follow-up fixes for the CI failures introduced by the profile endpoints PR (NVIDIA#13872): 1. API stability reference: add start_profile / stop_profile entries to tests/unittest/api_stability/references/llm.yaml so TestLLM::test_{signature,docstring,modified_init,...} stops failing with "LLM has different methods: {'start_profile', 'stop_profile'}". 2. Ray WorkerExtension collision: allow a user-supplied WorkerExtension to override BaseWorker.start_profile / stop_profile. These names are explicitly designated extension points (the RLHF WorkerExtension in tensorrt_llm/llmapi/rlhf_utils.py uses torch.cuda.profiler directly), so _inject_worker_extension now copies overridable extension methods into the derived class dict, which makes the extension's impl win regardless of MRO order and keeps the conflict check strict for every other name. Fixes the cascade of "RayGPUWorker died during initialization / Worker class RayGPUWorker already defines 'start_profile'" errors across the Ray test suites (H100, H100_PCIe, B200). Signed-off-by: JunyiXu-nv <219237550+JunyiXu-nv@users.noreply.github.com>
The Pre-commit Check on PR NVIDIA#13872 failed with two issue families: * ruff-format (PEP-8 + 80-col yapf-equivalent) reformatted multiline asserts and f-strings in the new test files. * ruff-legacy reported 8 D205 regressions ('1 blank line required between summary line and description') vs the baseline. Some are in docstrings I added (proxy._wait_profile_ack, proxy.start_profile, proxy.stop_profile, test_e2e.test_trtllm_serve_profile_example, 4 docstrings in tests/unittest/executor/test_proxy_profile_sync.py) and a few are pre-existing violations whose line numbers shifted because of my additions (proxy.submit, test_e2e bench tests). Fix all 8 by inserting the required blank line after each docstring summary line and tightening summary lines so they are real sentences (D415 fix on the test_proxy_profile_sync.py module docstring). Also accept the ruff-format reflow on the two test files. Verified locally with: pre-commit run --files <touched files> — all hooks pass (yapf, ruff, ruff-format, ruff-legacy, autoflake, codespell, etc.). Signed-off-by: Junyi Xu <219237550+JunyiXu-nv@users.noreply.github.com>
The Pre-commit Check on PR NVIDIA#13872 failed with two issue families: * ruff-format (PEP-8 + 80-col yapf-equivalent) reformatted multiline asserts and f-strings in the new test files. * ruff-legacy reported 8 D205 regressions ('1 blank line required between summary line and description') vs the baseline. Some are in docstrings I added (proxy._wait_profile_ack, proxy.start_profile, proxy.stop_profile, test_e2e.test_trtllm_serve_profile_example, 4 docstrings in tests/unittest/executor/test_proxy_profile_sync.py) and a few are pre-existing violations whose line numbers shifted because of my additions (proxy.submit, test_e2e bench tests). Fix all 8 by inserting the required blank line after each docstring summary line and tightening summary lines so they are real sentences (D415 fix on the test_proxy_profile_sync.py module docstring). Also accept the ruff-format reflow on the two test files. Verified locally with: pre-commit run --files <touched files> — all hooks pass (yapf, ruff, ruff-format, ruff-legacy, autoflake, codespell, etc.). Signed-off-by: JunyiXu-nv <219237550+JunyiXu-nv@users.noreply.github.com>
1e41133 to
941d851
Compare
|
/bot run --disable-fail-fast |
|
PR_Github #48498 [ run ] triggered by Bot. Commit: |
|
PR_Github #48498 [ run ] completed with state
|
|
/bot run |
|
PR_Github #50699 [ run ] triggered by Bot. Commit: |
|
PR_Github #50699 [ run ] completed with state
|
|
/bot run |
|
PR_Github #50949 [ run ] triggered by Bot. Commit: |
|
PR_Github #50949 [ run ] completed with state
|
|
/bot run |
| with self._profile_state_lock: | ||
| self._profile_enabled = False | ||
|
|
||
| def start_profile(self, |
There was a problem hiding this comment.
This adds quite a bit of profiling state-machine logic directly into py_executor.py. Could we move it into a separate profiling.py helper/controller instead?
A separate PyExecutorProfileManager that owns start_profile(), stop_profile(), apply_start_config(), apply_stop_config(), profile_step(), and cleanup would keep py_executor.py mostly as orchestration/thin forwarding, and make the profiling lifecycle much easier to review and maintain.
There was a problem hiding this comment.
Thanks for reviewing! I've extracted them to a standalone file. Could you help take another look?
| if request is None: | ||
| request = StartProfileRequest() | ||
| try: | ||
| await asyncio.to_thread( |
There was a problem hiding this comment.
One concern with the current /start_profile and /stop_profile implementation is the threading model around the IPC queues.
The HTTP handlers call the backend through asyncio.to_thread() so the FastAPI event loop is not blocked while waiting for the worker ack. However, in the IPC proxy path, GenerationExecutorProxy.start_profile()/stop_profile() then directly access ZMQ-backed IpcQueue objects from that thread-pool thread:
- request_queue.put(StartProfileRequest(...)) /request_queue.put(StopProfileRequest())
- profile_ack_queue.get(...)to writes. Even read-only access to the same socket from different threads is unsafe. Also, asyncio.to_thread() does not guarantee that repeated calls will run on the same worker thread, so concurrent or back-to-back profile calls may touch the same sockets from different threads.
Could we avoid doing ZMQ socket operations inside the to_thread() callback? I think there are two possible directions:
-
Make the endpoints scheduled/best-effort controls: enqueue the control request on the existing queue owner path and return once the request is accepted/scheduled. In that case the docs/response should not promise that /stop_profile returns only after the trace is on disk.
-
Keep the synchronous “trace is flushed before /stop_profile returns” contract, but introduce a safe ownership boundary: for example, a dedicated control/queue-owner thread that is the only thread touching the ZMQ queues, while the HTTP handler waits on a thread-safe Future/Queue for the ack.
The key requirement is that all operations on a given ZMQ-backed IpcQueue stay on one owning thread.
There was a problem hiding this comment.
Thanks for the review! Pinned all profile-control IPC to a dedicated single-worker ThreadPoolExecutor, start_profile/stop_profile now .submit() onto it so every ZMQ op on request_queue/profile_ack_queue runs on one owning thread, and _wait_profile_ack still blocks the HTTP handler so the "trace flushed before /stop_profile returns" contract is preserved.
Bundle of correctness, validation and lint fixes from the PR NVIDIA#13872 review pass: * IPC ack-wait was a no-op (proxy.py + ipc.py). ZeroMqQueue.get() did not accept a timeout kwarg, so profile_ack_queue.get(timeout=...) raised TypeError, was caught by the broad except in _wait_profile_ack, and silently treated as a timeout. Net effect: start_profile()/stop_profile() returned before the worker had even processed the request, breaking the documented "trace is on disk by the time /stop_profile returns 200" contract. Fix: add timeout: Optional[float] to ZeroMqQueue.get() (uses zmq.Socket.poll() and raises queue.Empty on timeout) and rewrite _wait_profile_ack to: - catch only queue.Empty for the timeout branch; - loop on stale ack-kind mismatches (a stale ack from a previous /start_profile that timed out would otherwise be consumed by the next /stop_profile, breaking the synchronous contract). * PyExecutor.start_profile failed silently on bad output_dir. Replace logger.warning + continue with a RuntimeError so the HTTP layer rejects the request instead of scheduling a profile window that later blows up in export_chrome_trace() on the executor thread (after the caller already got a 200). * PyExecutor.{start,stop}_profile dropped broadcast-enqueue errors. Three call sites caught Exception broadly, logged a warning and continued, leaving _runtime_profile_pending_start_iter set on rank 0 while subordinate ranks never received the broadcast (TP/PP drift). Now we narrow the catch to (RuntimeError, OSError, ValueError, AttributeError), roll back the local pending markers in the start_profile path, and re-raise as RuntimeError so the HTTP layer can return 5xx instead of a stale 200. * StartProfileRequest accepted invalid inputs. - activities: List[str] -> List[Literal["CPU","GPU","CUDA_PROFILER"]] so unknown values are rejected at schema validation time. - num_steps: Optional[int] -> Optional[PositiveInt]. num_steps == 0 used to make stop_iter == start_iter; profile_step() discards the stop marker as stale and the window runs forever. Now rejected with a 422 at the request boundary. - start_step: int -> NonNegativeInt. * HTTP handlers in openai_server.py. - /start_profile: fix E126 over-indented JSONResponse returns (flake8 lint blocker); narrow the second except from Exception to (OSError, ValueError, TimeoutError) so unexpected errors hit FastAPI's middleware with a real stack trace. - /stop_profile: narrow except Exception to (RuntimeError, OSError, TimeoutError) for the same reason. * Tests. Replace hardcoded "/tmp/..." paths with the pytest tmp_path fixture (Ruff S108) in test_profile_endpoints.py, test_proxy_profile_sync.py, and _test_trtllm_serve_profile.py. The /_torch/pyexecutor test actually creates the directory via os.makedirs() so tmp_path is also more correct, not just lint-clean. Signed-off-by: JunyiXu-nv <219237550+JunyiXu-nv@users.noreply.github.com>
2646179 to
6da8a38
Compare
Bundle of correctness, validation and lint fixes from the PR NVIDIA#13872 review pass: * IPC ack-wait was a no-op (proxy.py + ipc.py). ZeroMqQueue.get() did not accept a timeout kwarg, so profile_ack_queue.get(timeout=...) raised TypeError, was caught by the broad except in _wait_profile_ack, and silently treated as a timeout. Net effect: start_profile()/stop_profile() returned before the worker had even processed the request, breaking the documented "trace is on disk by the time /stop_profile returns 200" contract. Fix: add timeout: Optional[float] to ZeroMqQueue.get() (uses zmq.Socket.poll() and raises queue.Empty on timeout) and rewrite _wait_profile_ack to: - catch only queue.Empty for the timeout branch; - loop on stale ack-kind mismatches (a stale ack from a previous /start_profile that timed out would otherwise be consumed by the next /stop_profile, breaking the synchronous contract). * PyExecutor.start_profile failed silently on bad output_dir. Replace logger.warning + continue with a RuntimeError so the HTTP layer rejects the request instead of scheduling a profile window that later blows up in export_chrome_trace() on the executor thread (after the caller already got a 200). * PyExecutor.{start,stop}_profile dropped broadcast-enqueue errors. Three call sites caught Exception broadly, logged a warning and continued, leaving _runtime_profile_pending_start_iter set on rank 0 while subordinate ranks never received the broadcast (TP/PP drift). Now we narrow the catch to (RuntimeError, OSError, ValueError, AttributeError), roll back the local pending markers in the start_profile path, and re-raise as RuntimeError so the HTTP layer can return 5xx instead of a stale 200. * StartProfileRequest accepted invalid inputs. - activities: List[str] -> List[Literal["CPU","GPU","CUDA_PROFILER"]] so unknown values are rejected at schema validation time. - num_steps: Optional[int] -> Optional[PositiveInt]. num_steps == 0 used to make stop_iter == start_iter; profile_step() discards the stop marker as stale and the window runs forever. Now rejected with a 422 at the request boundary. - start_step: int -> NonNegativeInt. * HTTP handlers in openai_server.py. - /start_profile: fix E126 over-indented JSONResponse returns (flake8 lint blocker); narrow the second except from Exception to (OSError, ValueError, TimeoutError) so unexpected errors hit FastAPI's middleware with a real stack trace. - /stop_profile: narrow except Exception to (RuntimeError, OSError, TimeoutError) for the same reason. * Tests. Replace hardcoded "/tmp/..." paths with the pytest tmp_path fixture (Ruff S108) in test_profile_endpoints.py, test_proxy_profile_sync.py, and _test_trtllm_serve_profile.py. The /_torch/pyexecutor test actually creates the directory via os.makedirs() so tmp_path is also more correct, not just lint-clean. Signed-off-by: JunyiXu-nv <219237550+JunyiXu-nv@users.noreply.github.com>
|
/bot run --disable-fail-fast |
|
PR_Github #54206 [ run ] triggered by Bot. Commit: |
|
PR_Github #54206 [ run ] completed with state
|
Bundle of correctness, validation and lint fixes from the PR NVIDIA#13872 review pass: * IPC ack-wait was a no-op (proxy.py + ipc.py). ZeroMqQueue.get() did not accept a timeout kwarg, so profile_ack_queue.get(timeout=...) raised TypeError, was caught by the broad except in _wait_profile_ack, and silently treated as a timeout. Net effect: start_profile()/stop_profile() returned before the worker had even processed the request, breaking the documented "trace is on disk by the time /stop_profile returns 200" contract. Fix: add timeout: Optional[float] to ZeroMqQueue.get() (uses zmq.Socket.poll() and raises queue.Empty on timeout) and rewrite _wait_profile_ack to: - catch only queue.Empty for the timeout branch; - loop on stale ack-kind mismatches (a stale ack from a previous /start_profile that timed out would otherwise be consumed by the next /stop_profile, breaking the synchronous contract). * PyExecutor.start_profile failed silently on bad output_dir. Replace logger.warning + continue with a RuntimeError so the HTTP layer rejects the request instead of scheduling a profile window that later blows up in export_chrome_trace() on the executor thread (after the caller already got a 200). * PyExecutor.{start,stop}_profile dropped broadcast-enqueue errors. Three call sites caught Exception broadly, logged a warning and continued, leaving _runtime_profile_pending_start_iter set on rank 0 while subordinate ranks never received the broadcast (TP/PP drift). Now we narrow the catch to (RuntimeError, OSError, ValueError, AttributeError), roll back the local pending markers in the start_profile path, and re-raise as RuntimeError so the HTTP layer can return 5xx instead of a stale 200. * StartProfileRequest accepted invalid inputs. - activities: List[str] -> List[Literal["CPU","GPU","CUDA_PROFILER"]] so unknown values are rejected at schema validation time. - num_steps: Optional[int] -> Optional[PositiveInt]. num_steps == 0 used to make stop_iter == start_iter; profile_step() discards the stop marker as stale and the window runs forever. Now rejected with a 422 at the request boundary. - start_step: int -> NonNegativeInt. * HTTP handlers in openai_server.py. - /start_profile: fix E126 over-indented JSONResponse returns (flake8 lint blocker); narrow the second except from Exception to (OSError, ValueError, TimeoutError) so unexpected errors hit FastAPI's middleware with a real stack trace. - /stop_profile: narrow except Exception to (RuntimeError, OSError, TimeoutError) for the same reason. * Tests. Replace hardcoded "/tmp/..." paths with the pytest tmp_path fixture (Ruff S108) in test_profile_endpoints.py, test_proxy_profile_sync.py, and _test_trtllm_serve_profile.py. The /_torch/pyexecutor test actually creates the directory via os.makedirs() so tmp_path is also more correct, not just lint-clean. Signed-off-by: JunyiXu-nv <219237550+JunyiXu-nv@users.noreply.github.com>
6da8a38 to
6f2d4d5
Compare
|
/bot run --disable-failf-fast |
|
PR_Github #54808 Bot args parsing error: usage: /bot [-h] |
…o trtllm-serve
Expose iteration-scoped runtime profiling over HTTP on the
OpenAI-compatible trtllm-serve endpoint, mirroring SGLang's
developer-profiling interface.
HTTP endpoints:
* ``POST /start_profile`` accepts ``output_dir``, ``num_steps``,
``start_step``, ``activities`` (any of ``"CPU"``, ``"GPU"``,
``"CUDA_PROFILER"``). With ``["CUDA_PROFILER"]`` the server skips
``torch.profiler`` entirely so it composes cleanly with
``nsys profile -c cudaProfilerApi``. Returns 200 on acceptance, 409
when a profile window is already active or pending.
* ``POST /stop_profile`` terminates the active window and flushes the
chrome trace to disk before returning 200.
Backend mechanics:
* ``PyExecutor.start_profile``/``stop_profile`` reuse the existing
``profile_start_iters`` / ``profile_stop_iters`` /
cudaProfilerStart-Stop plumbing so env-var-based profiling behavior
is preserved.
* ``_profiler()`` builds ``torch.profiler`` lazily on the first start
iteration so runtime configuration takes precedence over the
``TLLM_TORCH_PROFILE_TRACE`` env var and consecutive start/stop
windows work.
* Profile-start/stop are dispatched to every rank via dedicated
``PROFILE_START_REQUEST_ID`` / ``PROFILE_STOP_REQUEST_ID`` items on
the existing ``RequestBroadcaster``, so under TP/PP > 1 every rank
applies the window in lockstep and writes its own
``trtllm-trace-<profile_id>-rank-<N>.json``. The broadcast carries
``start_step`` (offset) rather than an absolute ``start_iter`` so
every rank computes the same ``start_iter`` from its own in-sync
``iter_counter`` at apply time -- otherwise rank 0's local apply got
a head start and rank 1 missed the fire point under load.
* No synthetic ``generate_async([0])`` tickle is submitted: the
PROFILE_START / PROFILE_STOP queue items themselves wake the idle
``get_from_request_queue``, and because they're consumed in
``_handle_special_queue_items`` no forward pass runs, so the chrome
trace contains only real workload events.
* A unique ``profile_id`` (monotonic timestamp + uuid4 fragment) is
embedded in the trace filename so back-to-back /start_profile cycles
don't silently overwrite each other.
* Double-start is rejected via ``RequestError`` (a ``RuntimeError``
subclass that the executor's error monitor treats as per-call), so
an already-active window doesn't kill the MPI worker on TP>=2.
* ``stop_profile()`` blocks until ``profile_step()`` clears
``_profile_enabled``, so when /stop_profile returns 200 the chrome
trace is on disk.
API surface:
* ``GenerationExecutor``, ``BaseWorker``, ``GenerationExecutorProxy``,
and ``GenerationExecutorRpcProxy`` proxy the new calls into the
worker process. Non-PyTorch backends fall through to a warning.
* ``LLM.start_profile`` / ``stop_profile`` added as
``@set_api_status("prototype")``.
* HTTP handlers run the executor calls via ``asyncio.to_thread`` so the
IPC-proxy synchronous wait does not block the FastAPI event loop.
Tests:
* ``tests/unittest/_torch/pyexecutor/test_profile_endpoints.py``
covers pending-iteration tracking, cancellation of a start that
never fired, the scheduled-stop path, multi-rank apply via
``_apply_profile_start_config``, and filename-uniqueness across
cycles.
* ``tests/unittest/executor/test_proxy_profile_sync.py`` covers the
IPC-proxy synchronous start/stop path.
* ``tests/unittest/llmapi/apps/_test_trtllm_serve_profile.py`` covers
HTTP-handler argument defaulting and error response shapes via a
mocked generator (no GPU required), wired through the apps/ wrapper
convention and registered in the l0_a10 pre-merge list.
Docs:
* ``docs/source/commands/trtllm-serve/trtllm-serve.rst`` adds a
"Runtime Profiling Endpoints" section with a request-body table and
a curl example.
* ``docs/source/commands/trtllm-serve/run-benchmark-with-trtllm-serve.md``
adds "Profile a server with HTTP API endpoints" with a steady-state
workflow, manual-stop variant, ``activities=["CUDA_PROFILER"]`` +
``nsys -c cudaProfilerApi`` composition, and troubleshooting table.
Verified end-to-end against TP=2 TinyLlama on H100: three back-to-back
benchmark cycles (concurrency 2/4/8, 16/48/48 requests) each produce
two rank traces with matching profile_id and identical step-scope
counts, no request failures, engine alive across all cycles.
Signed-off-by: JunyiXu-nv <219237550+JunyiXu-nv@users.noreply.github.com>
Bundle of correctness, validation and lint fixes from the PR NVIDIA#13872 review pass: * IPC ack-wait was a no-op (proxy.py + ipc.py). ZeroMqQueue.get() did not accept a timeout kwarg, so profile_ack_queue.get(timeout=...) raised TypeError, was caught by the broad except in _wait_profile_ack, and silently treated as a timeout. Net effect: start_profile()/stop_profile() returned before the worker had even processed the request, breaking the documented "trace is on disk by the time /stop_profile returns 200" contract. Fix: add timeout: Optional[float] to ZeroMqQueue.get() (uses zmq.Socket.poll() and raises queue.Empty on timeout) and rewrite _wait_profile_ack to: - catch only queue.Empty for the timeout branch; - loop on stale ack-kind mismatches (a stale ack from a previous /start_profile that timed out would otherwise be consumed by the next /stop_profile, breaking the synchronous contract). * PyExecutor.start_profile failed silently on bad output_dir. Replace logger.warning + continue with a RuntimeError so the HTTP layer rejects the request instead of scheduling a profile window that later blows up in export_chrome_trace() on the executor thread (after the caller already got a 200). * PyExecutor.{start,stop}_profile dropped broadcast-enqueue errors. Three call sites caught Exception broadly, logged a warning and continued, leaving _runtime_profile_pending_start_iter set on rank 0 while subordinate ranks never received the broadcast (TP/PP drift). Now we narrow the catch to (RuntimeError, OSError, ValueError, AttributeError), roll back the local pending markers in the start_profile path, and re-raise as RuntimeError so the HTTP layer can return 5xx instead of a stale 200. * StartProfileRequest accepted invalid inputs. - activities: List[str] -> List[Literal["CPU","GPU","CUDA_PROFILER"]] so unknown values are rejected at schema validation time. - num_steps: Optional[int] -> Optional[PositiveInt]. num_steps == 0 used to make stop_iter == start_iter; profile_step() discards the stop marker as stale and the window runs forever. Now rejected with a 422 at the request boundary. - start_step: int -> NonNegativeInt. * HTTP handlers in openai_server.py. - /start_profile: fix E126 over-indented JSONResponse returns (flake8 lint blocker); narrow the second except from Exception to (OSError, ValueError, TimeoutError) so unexpected errors hit FastAPI's middleware with a real stack trace. - /stop_profile: narrow except Exception to (RuntimeError, OSError, TimeoutError) for the same reason. * Tests. Replace hardcoded "/tmp/..." paths with the pytest tmp_path fixture (Ruff S108) in test_profile_endpoints.py, test_proxy_profile_sync.py, and _test_trtllm_serve_profile.py. The /_torch/pyexecutor test actually creates the directory via os.makedirs() so tmp_path is also more correct, not just lint-clean. Signed-off-by: JunyiXu-nv <219237550+JunyiXu-nv@users.noreply.github.com>
Defense-in-depth follow-up to the previous fix commit. The Pydantic ``StartProfileRequest`` schema rejects ``num_steps <= 0`` at the HTTP boundary, but programmatic callers (``LLM.start_profile``, env-var-driven paths, unit tests) bypass that schema. Without a check in the executor itself, ``num_steps == 0`` makes ``_apply_profile_start_config`` compute ``stop_iter == start_iter``; ``profile_step()`` then discards the stop marker as stale and the profile window runs forever. Add an explicit ValueError at the top of ``PyExecutor.start_profile`` and two regression tests. Signed-off-by: JunyiXu-nv <219237550+JunyiXu-nv@users.noreply.github.com>
queue.Queue.get(timeout=...) raises queue.Empty on timeout, not TimeoutError. The test for the timeout-warning path was raising TimeoutError from its mock side_effect, which is not caught by the production except Empty branch in proxy.py:_wait_profile_ack, so the exception bubbled up and failed the test instead of exercising the documented warn-and-return path. Switch the mock to raise queue.Empty() so the test actually drives the production except branch the docstring claims to test. Signed-off-by: JunyiXu-nv <219237550+JunyiXu-nv@users.noreply.github.com>
…de comments Reword inline comments and user-facing docs that previously described the per-iteration profiler scope as 'sglang-compatible'. The label format (`step[EXTEND bs=N toks=M]` / `step[DECODE bs=N]`) is unchanged; only the prose framing is neutralised. Pre-existing copyright/attribution headers for code actually ported from SGLang are left in place. Signed-off-by: JunyiXu-nv <219237550+JunyiXu-nv@users.noreply.github.com>
Address @QiJune's review feedback on the threading model around the new /start_profile and /stop_profile endpoints. Previously GenerationExecutorProxy.start_profile / stop_profile invoked request_queue.put() and profile_ack_queue.get() directly on whatever asyncio.to_thread() worker happened to dispatch the call, which violated pyzmq's single-thread-per-socket requirement and could touch the same ZMQ socket from two different threads on back-to-back calls. - Add a dedicated single-thread executor (_profile_control_executor) in GenerationExecutorProxy.__init__ with thread name prefix "proxy_profile_control". This thread is the sole owner of all profile-related ZMQ socket ops on the proxy side. - Refactor start_profile() / stop_profile() to submit the request_queue.put + _wait_profile_ack pair to the executor and block on Future.result(). The HTTP handler's asyncio.to_thread wrapper continues to work but never touches the ZMQ sockets. - Drain the executor in shutdown() before tearing down sockets. - Close profile_ack_queue in shutdown() (was missing). - Pin the invariant with a new regression test test_profile_control_pinned_to_single_owner_thread, which invokes the proxy from two distinct caller threads and asserts every queue operation observed runs on the single proxy_profile_control worker rather than the callers. The synchronous "chrome trace is on disk by the time /stop_profile returns 200" contract is preserved -- Future.result() blocks until _wait_profile_ack returns and re-raises any RuntimeError so the HTTP layer still sees worker rejections (e.g. 409 "already in progress"). Signed-off-by: JunyiXu-nv <219237550+JunyiXu-nv@users.noreply.github.com>
…g.py Pull the entire runtime-profiling state machine out of py_executor.py into a dedicated PyExecutorProfileManager in tensorrt_llm/_torch/pyexecutor/profiling.py. PyExecutor now keeps thin forwarders for start_profile / stop_profile / _apply_profile_start_config / _apply_profile_stop_config / _profiler so existing call sites and unit tests are unchanged. The manager owns: - the caller-thread entry points start_profile / stop_profile - the per-rank broadcast handlers apply_start_config / apply_stop_config - the per-iteration driver profile_step (torch.profiler / cudaProfilerStart/Stop on the executor thread) - best-effort cleanup on shutdown Profile state (profile_start_iters, _runtime_profile_*, _profile_enabled, _profile_state_lock) stays on the PyExecutor instance so callers and existing tests can still introspect it. The manager holds a weakref back to the executor to avoid GC cycles. Also picks up CodeRabbit review nits: - Add StartProfileRequest / StopProfileRequest to executor.request.__all__ - Collapse multi-line JSONResponse(content=..., status_code=...) calls in /start_profile error paths - Wire PyExecutorProfileManager into the bare _bare_executor() helper in tests/unittest/_torch/pyexecutor/test_profile_endpoints.py so the manager-based delegation works for the mock-style tests. Cuts py_executor.py by ~600 lines without changing observable behavior. Signed-off-by: JunyiXu-nv <219237550+JunyiXu-nv@users.noreply.github.com>
Add three regression tests that pin the single-owner-thread contract for
profile-control ZMQ ops (QiJune PR review §2):
* test_profile_ops_serialized_under_concurrent_callers — 16 concurrent
callers must never produce overlapping ZMQ access (peak_concurrency
must stay at 1) since pyzmq sockets are not thread-safe.
* test_profile_control_thread_name_is_stable_across_calls — 25 calls
across 8 caller threads must all share the same single owner thread,
guarding against an accidental future bump of max_workers > 1.
* test_caller_thread_never_touches_zmq_queue — caller threads (the
asyncio.to_thread fake) and ZMQ queue-op owners must be disjoint
sets; the dedicated proxy_profile_control worker is the only owner.
The existing test_profile_control_pinned_to_single_owner_thread covers
the happy path; these stress-test the same invariant under concurrency.
Signed-off-by: JunyiXu-nv <219237550+JunyiXu-nv@users.noreply.github.com>
6f2d4d5 to
cdfb36d
Compare
…-serve
Expose iteration-scoped runtime profiling over HTTP on the OpenAI-compatible trtllm-serve endpoint, mirroring SGLang's developer-profiling interface.
nsys profile -c cudaProfilerApi.Summary by CodeRabbit
Release Notes
New Features
/start_profileand/stop_profilefor controlling server profiling during execution with configurable output directory, duration, and profiling activities.Documentation
Tests
Description
Test Coverage
PR Checklist
Please review the following before submitting your PR:
PR description clearly explains what and why. If using CodeRabbit's summary, please make sure it makes sense.
PR Follows TRT-LLM CODING GUIDELINES to the best of your knowledge.
Test cases are provided for new code paths (see test instructions)
Any new dependencies have been scanned for license and vulnerabilities
CODEOWNERS updated if ownership changes
Documentation updated as needed
Update tava architecture diagram if there is a significant design change in PR.
The reviewers assigned automatically/manually are appropriate for the PR.
Please check this after reviewing the above items as appropriate for this PR.
GitHub Bot Help
To see a list of available CI bot commands, please comment
/bot help.