Conversation
Scope filtering no longer walks full NVCC template specializations (~18s to ~0.15s on an 80MB black_scholes trace with ~105k events), Path() is skipped for non-path details (~1.9s), and each JSON is parsed once for all slices and comparison instead of re-reading ~25GB current plus ~25GB baseline MatX traces per slice.
Keep the snapshot index until Git initializes it, fall back to an artifacts-only PR comment when even compact summaries exceed GitHub's size limit, and default RAPIDS to every manifest C++ project when no -target is given.
High Ninja concurrency adds contention noise to NVCC device-time traces.
A quarter of the usual build jobs keeps NVCC device-time traces quieter without pinning a fixed job count.
Build only leaf targets from each RAPIDS job and resolve C++ targets to their owning repository when collecting traces.
It looks like the compiler spends a lot of time to instantiate the tuple like constructor even though there are explicit `tuple` constructors that should be taken.
📝 SummarySummary by CodeRabbit
WalkthroughChangesThe compile-time CI now supports CCCL, PyTorch, MatX, and RAPIDS builds. Matrix validation, project-specific build dispatch, trace collection, grouped reporting, and combined pull-request comments were added. Tuple conversion constructors now exclude Compile-time benchmark pipeline
Tuple constructor constraints
Suggested reviewers: Priority: ➖ Normal Change: Feature Merge Risk: 🔵 Low · up to Benchmark reporting may show outdated results, and default build concurrency differs from the documented value. Both are bounded CI issues that should be corrected. 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: 2
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 79d8d649-3488-4b1a-8d47-94048e8de2f0
📒 Files selected for processing (19)
.github/workflows/ci-workflow-pull-request.yml.github/workflows/compile-time-bench.ymlci/build_compile_time_bench.shci/compile_time/README.mdci/compile_time/collect_traces.pyci/compile_time/combine_pr_comments.pyci/compile_time/parse_matrix.pyci/compile_time/prepare_traces.pyci/compile_time/render_pr_comment.pyci/compile_time/summarize_events.pyci/compile_time/test_summarize_events.pyci/matrix.yamlci/matx/build_matx.shci/pytorch/build_pytorch.shci/rapids/post-create-command.shdocs/infrastructure/ci/references/skip_tags.rstdocs/maintainers/how_tos/compile_time.rstdocs/maintainers/references/compile_time.rstlibcudacxx/include/cuda/std/__tuple_dir/tuple.h
Included review availability: Your plan provides up to 12 included reviews per hour; 11 remain after this review.
| if: >- | ||
| ${{ | ||
| always() && !cancelled() && | ||
| needs.build-workflow.outputs.compile_time_enabled == 'true' && | ||
| needs.build-workflow.outputs.pr_number != '' | ||
| }} | ||
| needs: [build-workflow, compile-time-results] | ||
| permissions: | ||
| actions: read | ||
| pull-requests: write | ||
| runs-on: ubuntu-latest | ||
| steps: | ||
| - name: Download combined compile-time comment | ||
| id: download-comment | ||
| continue-on-error: true | ||
| uses: actions/download-artifact@v4 | ||
| with: | ||
| name: compile-time-comment | ||
| path: compile-time-comment | ||
|
|
||
| - name: Comment on PR | ||
| if: ${{ steps.download-comment.outcome == 'success' && hashFiles('compile-time-comment/comment.md') != '' }} |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🔎 Supported by static analysis
🏁 Script executed:
sed -n '105,145p' .github/workflows/ci-workflow-pull-request.yml
sed -n '550,655p' .github/workflows/ci-workflow-pull-request.yml
rg -n 'compile-time-bench|compile_time_enabled|skip-tpt|sticky|combined comment|no artifact' .github ci/compile_time docs/infrastructure/ci/references/skip_tags.rst docs/maintainersRepository: NVIDIA/cccl
Length of output: 11636
🏁 Script executed:
printf '%s\n' '--- docs reference ---'
sed -n '215,245p' docs/maintainers/references/compile_time.rst
printf '%s\n' '--- how-to ---'
sed -n '90,155p' docs/maintainers/how_tos/compile_time.rst
printf '%s\n' '--- README ---'
sed -n '88,112p' ci/compile_time/README.md
printf '%s\n' '--- workflow tests ---'
sed -n '1725,1805p' ci/compile_time/test_summarize_events.py
printf '%s\n' '--- workflow gates and matrix output ---'
sed -n '35,70p' .github/workflows/ci-workflow-pull-request.yml
sed -n '112,152p' .github/workflows/ci-workflow-pull-request.yml
sed -n '515,575p' .github/workflows/ci-workflow-pull-request.ymlRepository: NVIDIA/cccl
Length of output: 14439
🤖 get_repo_knowledge executed:
get_repo_knowledge NVIDIA/cccl /tmp/coderabbit-repo-knowledge/nvidia-cccl-2f773195/learnings /tmp/coderabbit-repo-knowledge/nvidia-cccl-2f773195/conventions
Length of output: 47829
🏁 Script executed:
sed -n '215,245p' docs/maintainers/references/compile_time.rst
sed -n '90,155p' docs/maintainers/how_tos/compile_time.rst
sed -n '88,112p' ci/compile_time/README.md
sed -n '1725,1805p' ci/compile_time/test_summarize_events.pyRepository: NVIDIA/cccl
Length of output: 7754
important: Clear the sticky comment when no current benchmark result exists.
When benchmarking is disabled, both comment jobs are skipped. When aggregation produces no comment.md, the upload and sticky-comment steps also skip. An earlier compile-time-bench comment then remains visible without being marked outdated.
Add a fallback cleanup or status path for disabled and unavailable benchmark runs. Keep cancelled runs from clearing the last completed result.
🧰 Tools
🪛 zizmor (1.30.0)
[warning] 17-995: overly broad permissions (excessive-permissions): default permissions used due to no permissions: block
(excessive-permissions)
| # Device-time traces are sensitive to machine contention. Default to a quarter | ||
| # of (nproc-1) instead of nproc-1 from ci/build_common.sh. | ||
| if [[ -z "${PARALLEL_LEVEL:-}" ]]; then | ||
| PARALLEL_LEVEL=$(( ($(nproc --all) - 1) / 2 )) |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
important: the divisor does not match the documented default. The code divides by 2, but the inline comment (Line 268), the usage text (Line 78), and docs/maintainers/references/compile_time.rst Line 56 all state (nproc-1)/4. Pick one value and align all four sites.
Fix if /4 is intended
- PARALLEL_LEVEL=$(( ($(nproc --all) - 1) / 2 ))
+ PARALLEL_LEVEL=$(( ($(nproc --all) - 1) / 4 ))📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| PARALLEL_LEVEL=$(( ($(nproc --all) - 1) / 2 )) | |
| PARALLEL_LEVEL=$(( ($(nproc --all) - 1) / 4 )) |
😬 CI Workflow Results🟥 Finished in 3h 46m: Pass: 99%/550 | Total: 23d 20h | Max: 3h 46m | Hits: 30%/3834090See results here. AI failure analysis1. NVCC C++23 kernel argument IR type mismatch · 2 jobsExplanation: Both CTK 13.3, GCC 15, C++23 builds fail inside NVCC with the same pointer-versus-value LLVM signature for CUB kernel arguments. The PR's only library change adds an independent SFINAE template parameter to generic tuple-like constructors, making that change the likely trigger, although a minimal compile is needed to confirm the compiler interaction. Evidence: Copy this prompt into a coding agentJobs: 2. RAPIDS trace collection uses unset rapids_manifest · 3 jobsExplanation: The RAPIDS matrix supplies explicit targets, but ci/build_compile_time_bench.sh initializes rapids_manifest only when no targets were supplied. The builds finish, then set -u terminates all three jobs when trace collection references the unset variable. Evidence: Copy this prompt into a coding agentJobs: 3. PyTorch configuration selects the system NVCC · 1 jobExplanation: The copied CUDA toolkit contains the PR's CCCL headers, but CMake configures PyTorch with /usr/local/cuda/bin/nvcc instead of the copied toolkit's compiler. The script's post-configuration guard correctly aborts because the resulting benchmark would not reliably compile against the intended custom CCCL installation. Evidence: Copy this prompt into a coding agentJobs: 4. Windows HostJIT cannot resolve __assert_fail · 1 jobExplanation: The C Parallel build succeeds, but HostJIT-generated Windows objects reference the Unix assertion symbol __assert_fail from the _CCCL_VERIFY in cub::detail::ptx_compute_cap. That symbol is unavailable to the freestanding lld-link invocation, causing 17 of 20 tests to fail during JIT linking. Evidence: Copy this prompt into a coding agentJobs: 5. MatX trace staging exhausts device storage · 1 jobExplanation: The 175-minute MatX build completes, but collect_traces.py duplicates every object-adjacent trace into a staging directory on the same constrained build volume. The additional full copy exhausts storage before trace reporting or baseline collection can begin. Evidence: Copy this prompt into a coding agentJobs: |
⏱️ CCCL compile-time benchmark comparisonsEach configuration is reported independently below. ⏱️ CCCL compile-time bench — 30 regression row(s), 1 improvement row(s)Result: 30 regression row(s), 1 improvement row(s) above threshold.
Artifacts: reports and traces TU total compilation
🔴 TU total compilation — Regressions
🟢 TU total compilation — Improvements
Direct file processing
🔴 Direct file processing — Regressions
|
2 participants
Currently if a converting tuple constructor is rejected, the compiler seems to try again through the tuple-like constructor which surprisingly is again rejected.
However, we already know that, so instantiating this is wastefull.
Rather than that exit early.