[TRTLLM-14628][feat] Support out-of-tree build state via --build_root - #17524
Conversation
|
/bot run |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (1)
WalkthroughThe wheel build now supports ChangesBuild root support
Estimated code review effort: 3 (Moderate) | ~20 minutes Mergeability Score: 🔵 Low · up to The build-root change is otherwise mergeable, but the new waiver may skip sampler coverage beyond DGX_B200-PyTorch-4 and should be narrowed or explicitly accepted; the modified waiver file also needs the standard copyright header. Sequence Diagram(s)sequenceDiagram
participant CLI as build_wheel.py CLI
participant Main as build_wheel.py main
participant CMakeConan as CMake and Conan
participant Extensions as Extension builds
participant Setuptools as setup.py
CLI->>Main: provide --build_root or TRTLLM_BUILD_ROOT
Main->>CMakeConan: use root-specific build and cache paths
Main->>Extensions: use root-specific temporary build paths
Main->>Setuptools: set TRTLLM_WHEEL_STAGING_DIR
Setuptools-->>Main: stage wheel metadata and build state
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
setup.py (1)
439-460: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd
-> dict[str, dict[str, str]]toget_build_state_options.This meets the repository requirement to annotate every Python function.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@setup.py` around lines 439 - 460, Add the return annotation -> dict[str, dict[str, str]] to get_build_state_options, preserving its existing behavior and return values.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@scripts/build_wheel.py`:
- Around line 98-104: Add complete type annotations to get_build_dir() and
build_kv_cache_manager_v2(), including parameter and return types, and annotate
create_venv()’s return value. In setup_venv() and main(), use explicit Path |
None, str | None, and int | None annotations for parameters with None defaults
to satisfy Ruff RUF013.
---
Nitpick comments:
In `@setup.py`:
- Around line 439-460: Add the return annotation -> dict[str, dict[str, str]] to
get_build_state_options, preserving its existing behavior and return values.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: e8874f21-612b-4143-a0c2-4982b2ceac6b
📒 Files selected for processing (3)
docs/source/installation/build-from-source.mdscripts/build_wheel.pysetup.py
|
/bot run |
|
PR_Github #65409 [ run ] triggered by Bot. Commit: |
|
PR_Github #65409 [ run ] completed with state
|
|
/bot run |
|
PR_Github #65452 [ run ] triggered by Bot. Commit: |
|
PR_Github #65452 [ run ] completed with state
|
|
/bot run |
|
PR_Github #65475 [ run ] triggered by Bot. Commit: |
|
PR_Github #65475 [ run ] completed with state
|
|
/bot run |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/integration/test_lists/waives.txt`:
- Line 1: Add the repository-standard NVIDIA copyright header at the beginning
of the waiver file before the existing Wrapper entry, using 2026 as the latest
meaningful modification year; preserve the waiver entry unchanged.
- Line 1: Replace the waiver entry with the supported bare test path and
existing reason, removing the “Wrapper:” prefix and backticks so matching works.
Add the required NVIDIA copyright header at the top of waives.txt.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 13b7bb49-f106-48b6-9162-af9fde345aa0
📒 Files selected for processing (1)
tests/integration/test_lists/waives.txt
|
PR_Github #65507 [ run ] triggered by Bot. Commit: |
|
PR_Github #65707 [ run ] completed with state
|
|
/bot run |
|
PR_Github #65738 [ run ] triggered by Bot. Commit: |
|
PR_Github #65738 [ run ] completed with state
|
|
/bot run |
4dc9732 to
67e3f94
Compare
|
PR_Github #65786 [ run ] triggered by Bot. Commit: |
|
PR_Github #65786 [ run ] completed with state
|
|
/bot run |
|
PR_Github #65802 [ run ] triggered by Bot. Commit: |
|
PR_Github #65802 [ run ] completed with state
|
Developers increasingly build from checkouts on network filesystems (Lustre, NFS, GPFS), which are slow for metadata-heavy workloads. The build currently writes high-churn state into the checkout: the CMake build dir (default cpp/build*), the build venv (~70k files), the setuptools wheel staging tree and *.egg-info, extension-module object files, and (by default) the ccache directory. Add --build_root DIR (env: TRTLLM_BUILD_ROOT) to build_wheel.py. When set, all of the above default under DIR so it can be pointed at fast node-local storage while the checkout stays on shared storage. Each piece remains individually overridable (--build_dir, CCACHE_DIR, TRTLLM_WHEEL_STAGING_DIR). Only final artifacts (tensorrt_llm/libs, include, bindings, stubs, wheels) are still written into the checkout. setup.py learns TRTLLM_WHEEL_STAGING_DIR, redirecting setuptools build_base and egg_base out of the source tree. Behavior without --build_root is unchanged. Documented in docs/source/installation/build-from-source.md along with CCACHE_DIR / CONAN_HOME / --use-3rdparty-cache guidance for shared-storage workflows. Signed-off-by: Brian Nguyen <brnguyen@nvidia.com>
Signed-off-by: Brian Nguyen <brnguyen@nvidia.com>
…t_unittests_v2[unittest/_torch/attention/sparse/deepseek_v4/test_compressor_kernel.py] (pre-existing failure, tracking bug pending) Signed-off-by: Brian Nguyen <brnguyen@nvidia.com>
Signed-off-by: Brian Nguyen <brnguyen@nvidia.com>
With --build_root the setuptools build_base moves under TRTLLM_WHEEL_STAGING_DIR, so clean_wheel (which only clears dist_dir) no longer wipes it. Stale copies of deleted package files could then be re-packed into the next "clean" wheel. Clear the external staging build tree too when it is configured. Signed-off-by: Brian Nguyen <brnguyen@nvidia.com>
…s_v2[unittest/_torch/sampler -k "not test_speculative_d2h_parity_real_predictor"] (pre-existing failure, tracking bug pending) Signed-off-by: Brian Nguyen <brnguyen@nvidia.com>
Signed-off-by: Brian Nguyen <brnguyen@nvidia.com>
67e3f94 to
1b4fec3
Compare
|
/bot run --skip-test |
|
PR_Github #65926 [ run ] triggered by Bot. Commit: |
|
PR_Github #65926 [ run ] completed with state |
|
/bot skip --comment "build-only change, no functional changes" |
|
PR_Github #65974 [ skip ] triggered by Bot. Commit: |
|
PR_Github #65974 [ skip ] completed with state |
7 participants
Description
Developers increasingly build TensorRT-LLM from checkouts on shared/network
filesystems (Lustre, NFS, GPFS) inside containers on cluster nodes. These
filesystems handle streaming I/O well but are slow for metadata-heavy
workloads, and today's build writes a large amount of high-churn state into
the checkout:
cpp/build*): objects, CMake state,FetchContent
_deps— millions of small files on a full build.venv-3.x, ~70k files)build/) andtensorrt_llm.egg-info--use_ccache, the ccache directory (container default is ephemeral)This PR adds a supported way to keep all of that on fast local storage while
the checkout stays on shared storage:
build_wheel.pyoption--build_root DIR(env:TRTLLM_BUILD_ROOT).When set, the CMake build dir, the build venv, wheel staging/egg-info,
intermediate objects, and (with
--use_ccache) the ccache directory alldefault under
DIR. Each piece remains individually overridable(
--build_dir,CCACHE_DIR,TRTLLM_WHEEL_STAGING_DIR).setup.pyhonors a newTRTLLM_WHEEL_STAGING_DIRenv var that redirectssetuptools
build_baseandegg_baseout of the source tree.docs/source/installation/build-from-source.md("Building from a checkout on a network filesystem") covering the flag and
related knobs (
CCACHE_DIRpersistence,CONAN_HOME,--use-3rdparty-cache).Only final artifacts are still written into the checkout (
tensorrt_llm/libs,tensorrt_llm/include, bindings and stubs, generated FMHA kernel sources,and the wheel output dir). Behavior without
--build_rootis unchanged;plain local-disk builds are unaffected.
Follow-ups planned under the same ticket: a fully hermetic wheel build
(no checkout writes at all, validated with a read-only source mount) and
archive-based transfer for the remaining artifact copy-back.
Test Coverage
--build_root /tmp/... --use_ccache -a 100-real) from a checkout on a network filesystem, in the pinnedaarch64 devel container with no home mount: build succeeds and an
mtime-based audit of the checkout afterwards shows only the expected
final artifacts, no build state.
get_build_dir()defaults unchanged without--build_root;setup.pystaging/egg-info land in their usual in-treelocations when
TRTLLM_WHEEL_STAGING_DIRis unset.PR Checklist
Dev Engineer Review
--build_root DIRandTRTLLM_BUILD_ROOTsupport inscripts/build_wheel.py.TRTLLM_WHEEL_STAGING_DIRsupport insetup.py.CCACHE_DIR,CONAN_HOME, and--use-3rdparty-cache.unittest/_torch/sampler::test_speculative_d2h_parity_real_predictor, with a pending-bug reference.QA Engineer Review
tests/integration/test_lists/waives.txt.unittest/_torch/sampler::test_speculative_d2h_parity_real_predictor.pr-babysitter waive set-bug NVIDIA/TensorRT-LLM#17524 <bug-url>— the waives entry carries a placeholder until then