Limit arena size in C++ tests - #2506
Conversation
|
Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually. Contributors can view more details about this message here. |
|
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:
📝 WalkthroughSummary by CodeRabbit
WalkthroughArena memory resource tests now use an explicit 8 GiB capacity. The C++ nightly workflow targets GB300 and disables selected CI jobs. The C++ test script reports kernel and NVIDIA driver allocation settings. ChangesArena test sizing and CI workflow
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
9eb6018 to
221ee4e
Compare
|
/ok to test |
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 @.github/workflows/pr.yaml:
- Line 59: Replace each constant if: false condition in the workflow with an
appropriate feature flag, or remove the disabled jobs and corresponding
pr-builder.needs entries. Ensure no unconditional constant-false conditions
remain so actionlint passes.
- Around line 282-283: Update the GB300 nightly matrix entry by setting its
build_workflow_name input to pr.yaml, ensuring artifact lookup targets the C++
artifacts produced by conda-cpp-build in pr.yaml while preserving the existing
matrix_type and matrix_filter 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: 1415b8aa-3e59-4a55-9e04-52f2c78ac5fc
📒 Files selected for processing (1)
.github/workflows/pr.yaml
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@ci/test_cpp.sh`:
- Line 39: Update the `/proc/cmdline` check in `ci/test_cpp.sh` to validate
readability before searching for `init_on_alloc`. Report the setting as
unavailable with a meaningful error message when the file is missing or
unreadable, and retain the existing “not set” message only when a readable file
lacks the parameter.
🪄 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: 6e520b46-c658-4e82-8933-30214a8f73f6
📒 Files selected for processing (1)
ci/test_cpp.sh
|
I have some temporary testing changes for GB300 so that I can figure out why the tests are so slow. I think it's an issue with missing the |
wence-
left a comment
There was a problem hiding this comment.
Approving C++ changes, which seem reasonable independent of the runner setup issues.
7fa0e5a to
0067d3e
Compare
This reverts commit 96f2303.
e128f67 to
437beed
Compare
Description
Use an explicit 8 GiB arena in the C++ arena MR reference tests instead of the default half of available device memory. This prevents 116 fixture constructions from each reserving roughly 125 GiB on GB300 while preserving the capacity available on the original 16 GiB test target.
Measured on DGX Spark (128GB integrated memory), total test time for the arena MR reference tests decreased from 64.41 seconds to 11.88 seconds (81.6% reduction).
Closes #2505. Follows the Python test precedent in #2273.
Checklist