-
Notifications
You must be signed in to change notification settings - Fork 257
[AgentX] Tune DSV4 GB300 MTP / 调优 DSV4 GB300 MTP #2571
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
ivanium
wants to merge
3
commits into
main
Choose a base branch
from
codex/dsv4-gb300-agentx-clean
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
3 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🟡 This PR adds
AIPERF_DATASET_MMAP_CACHE_ENABLED: "false"(to avoid concurrent aiperf readers observing a mismatched mmap data/index pair) to the benchmark.env of all four other GB300 AgentX MTP configs it touches, butagg-gb300-tp8-mtp-agentic.yamlis skipped even though this same PR tunes it (max-num-seqs, compilation-config, gpu-memory-utilization) and newly wires it into the sweep vianvidia-master.yaml. It still uses the identicalAIPERF_DATASET_MMAP_CACHE_DIRmechanism and remains exposed to the same race; add the same env var here for consistency.Extended reasoning...
This PR introduces
AIPERF_DATASET_MMAP_CACHE_ENABLED: "false"with the comment "Avoid concurrent readers observing a mismatched mmap data/index pair" into thebenchmark.envblock of four configs:agg-gb300-tp4-mtp-agentic.yaml,disagg-gb300-1p1d-dep4-dep8-c256-mtp-agentic.yaml,disagg-gb300-2p1d-dep8-dep12-c1152-mtp-agentic.yaml, and the brand-newdisagg-gb300-2p1d-dep8-dep16-c1024-mtp-agentic.yaml. All five configs, includingagg-gb300-tp8-mtp-agentic.yaml, share the same benchmark harness invocation (bash /infmax-workspace/benchmarks/multi_node/agentic_srt.sh) and the sameAIPERF_DATASET_MMAP_CACHE_DIR: "/aiperf_mmap_cache"setting, which is what makes them susceptible to the underlying aiperf dataset mmap-cache race in the first place.agg-gb300-tp8-mtp-agentic.yamlis not merely an untouched bystander file: this PR actively edits it (VLLM_V2_WARMUP_MAX_NUM_SEQS32→16,max-num-seqs32→16, expandedcompilation-configcudagraph capture sizes,gpu-memory-utilization0.90→0.94, droppedenable-cumem-allocator), andconfigs/nvidia-master.yamlgains a brand-new search-space entry (spec-decoding: mtp,prefill.tp: 8,additional-settings: CONFIG_FILE=recipes/vllm/deepseek-v4/agentic/agg-gb300-tp8-mtp-agentic.yaml) that pulls this exact recipe into the sweep. So the PR both tunes this config and newly exercises it in CI, yet itsbenchmark.env(around line 111-126 in the diff / ~146 in the merged file) still only hasAIPERF_DATASET_MMAP_CACHE_DIRwithout the pairedAIPERF_DATASET_MMAP_CACHE_ENABLED: "false"guard that all four sibling configs received.Concrete walk-through of the gap:
agg-gb300-tp4-mtp-agentic.yamlbenchmark.env (post-diff): has bothAIPERF_DATASET_MMAP_CACHE_ENABLED: "false"andAIPERF_DATASET_MMAP_CACHE_DIR: "/aiperf_mmap_cache".disagg-gb300-1p1d-dep4-dep8-c256-mtp-agentic.yaml,disagg-gb300-2p1d-dep8-dep12-c1152-mtp-agentic.yaml, and the newdisagg-gb300-2p1d-dep8-dep16-c1024-mtp-agentic.yaml: same pairing added.agg-gb300-tp8-mtp-agentic.yamlbenchmark.env (post-diff): onlyAIPERF_DATASET_MMAP_CACHE_DIR: "/aiperf_mmap_cache"remains — theENABLED: "false"line was never added, despite this file being edited elsewhere in the same diff for scheduler/cudagraph tuning.agentic_srt.shbenchmark client against the same mmap cache directory mechanism, it remains exposed to the intermittent race where a concurrent reader can observe a data file and index file from two different write generations (i.e., a torn/mismatched mmap pair), which the ENABLED:false flag was added everywhere else specifically to prevent.This looks like a simple "missed one file" inconsistency during a multi-file find-and-fix pass rather than an intentional decision — there's no comment or rationale anywhere explaining why tp8-agg should be treated differently, and the mechanism it uses is byte-for-byte the same as its siblings.
Fix: add the same two lines to
agg-gb300-tp8-mtp-agentic.yaml'sbenchmark.env, mirroring the other four configs exactly:Impact/severity: this is a benchmark-harness reliability issue (a flaky aiperf dataset reader race), not a change in served-model correctness — a hit would produce an occasional re-runnable benchmark flake rather than a silent wrong result or a hard failure. Given that scope, this is a nit: worth fixing for consistency across the sibling configs newly wired into the same sweep, but not a merge blocker.