Skip to content

feat: support python dspark acl graphs with data parallelism. - #2473

Merged
XuZhang99 merged 1 commit into
xLLM-AI:mainfrom
PixelFlat:feat/python-dspark-acl-graph-dp
Oct 11, 2026
Merged

XuZhang99 merged 1 commit into
xLLM-AI:mainfrom
PixelFlat:feat/python-dspark-acl-graph-dp

Conversation

@PixelFlat

@PixelFlat PixelFlat commented Oct 11, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Enable Python DSpark ACL graphs with data parallelism, building on the graph
replay fixes already merged in #2471. This PR contains only the DP feature.

  • Select the block-draft graph runner for DP configurations and pad each rank
    to a common whole-query request bucket.
  • Coordinate graph keys and capture presence across DP peers, including
    asymmetric local batches and page-table capacities.
  • Materialize a complete dummy query on idle peers while preserving raw request
    counts and using invalid cache slots. Clear padding when local batches shrink.
  • Add C++ input-builder and Python graph/executor regression coverage.

Validation

  • Local pre-commit checks, including clang-format 20.1.6: passed.
  • Incremental NPU build with python setup.py build --device npu --tilelang-jobs 4: passed on the rebased commit.
  • All 23 SpecDecodeInputBuilderTest CTest cases: passed.
  • All 68 CTest targets under python_tests.* and
    python_distributed_tests.*: passed (65 Python modules and three distributed
    NZ tests), including graph, empty-DP, model-executor, and prepared-graph tests.
  • CTest runs were serial with --output-on-failure; distributed tests used
    automatically assigned socket ports. This validation does not claim a full
    native C++ test-suite run.

Concurrent-request functional validation

Using the same DP=2 graph-enabled configuration, warmup was followed by three
repetitions each at concurrency 2 and 3 (six and nine measured requests,
respectively). All 15 requests completed successfully with the expected 1,868
input tokens, 128 output tokens, and finish reasons; no OOM, timeout, or server
error was observed. These concurrent scenarios validate request completion and
functional behavior, not full numerical output parity or a high-concurrency
throughput target.

Single-request benchmark

GLM-5.2 static W8A8 target with GLM-5.2-DSpark-NPU-0805 draft, Python execution,
16 NPUs, DP=2 / attention TP=8 / EP=1, 7 speculative tokens, ACL graphs and
asynchronous scheduling enabled. Memory utilization is unchanged at 0.88;
temperature is explicitly 0. Each measured request has 1,868 input tokens and
128 output tokens. Results are means of five runs after warmup.

Metric Average
TTFT 761.00 ms
TPOT 16.72 ms
Total latency 2883.76 ms
Accepted tokens per request 89.2
Draft tokens per request 278.6
Draft rounds per request 39.8
Accepted tokens per draft round 2.24
Request acceptance rate 32.22%

Runtime measurements were obtained on the feature before rebase
(ee795338); rebase leaves the feature patch unchanged. Acceptance rate is the
mean of per-request rates; accepted length is total accepted tokens divided by
total draft rounds. This is a functional/performance smoke test, not a
before/after performance comparison.

Summary by CodeRabbit

  • New Features
    • Added support for data-parallel execution with ACL graph acceleration, including configurations with multiple parallel ranks.
    • Graph execution now accommodates idle ranks and batches with fewer active sequences, while keeping query blocks aligned across ranks.
    • Improved graph preparation and replay to handle varying batch sizes and cache capacities across parallel ranks.

Coordinate whole-query padding, graph keys, and capture presence across DP ranks. Materialize safe block-draft inputs for idle peers without writing real cache slots, and cover asymmetric batches and executor selection with unit tests.
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 11, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-11T15:32:39.690905Z 15d77b7 PR opened
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@coderabbitai

coderabbitai Bot commented Oct 11, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: defaults
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 9aa0c047-65d8-45ac-8bb6-dda1d1352c0b

📥 Commits

Reviewing files that changed from the base of the PR and between 6de5178 and 15d77b7.


📒 Files selected for processing (8)
  • tests/core/framework/speculative/spec_input_builder_test.cpp
  • tests/python/test_decode_acl_graph.py
  • tests/python/test_model_executor.py
  • xllm/core/framework/speculative/spec_input_builder.cpp
  • xllm/core/framework/speculative/spec_input_builder.h
  • xllm/core/runtime/dflash_worker_impl.cpp
  • xllm/python/model_executor/executor.py
  • xllm/python/model_executor/runners/block_draft_acl_graph.py

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review.



📝 Walkthrough

Walkthrough

The change adds host input materialization for empty block-draft ranks and extends the ACL graph runner to support data-parallel admission, coordinated graph preparation, and padded replay.

Changes

Data-parallel block-draft execution

Layer / File(s) Summary
Materialize empty draft rows
xllm/core/framework/speculative/spec_input_builder.h, xllm/core/framework/speculative/spec_input_builder.cpp, xllm/core/runtime/dflash_worker_impl.cpp, tests/core/framework/speculative/spec_input_builder_test.cpp
The helper creates placeholder host input and metadata for empty ranks. Two worker paths call it for Python model implementations. C++ tests check the generated input, cache slots, readiness flags, and token counts.
Configure and admit DP graph inputs
xllm/python/model_executor/executor.py, xllm/python/model_executor/runners/block_draft_acl_graph.py, tests/python/test_decode_acl_graph.py, tests/python/test_model_executor.py
The executor passes global sequence capacity, DP size, and DP rank to the runner. The runner validates DP counts and phases, then selects a padded sequence bucket. Tests cover construction, padding, empty ranks, and rejected or unsupported inputs.
Coordinate capture and pad replay
xllm/python/model_executor/runners/block_draft_acl_graph.py, tests/python/test_decode_acl_graph.py
Warmup synchronizes graph keys and graph presence across DP ranks. Graph allocation uses the bucket capacity. Replay pads unused inputs and metadata rows; tests cover stale-row cleanup, peer page-capacity changes, and warmup recapture.

Priority: ➖ Normal

Estimated code review effort: 4 (Complex) | ~45 minutes

Change: Feature

Sequence Diagram(s)

sequenceDiagram
  participant Executor
  participant Runner as BlockDraftAclGraphRunner
  participant Peers as DP peers
  participant Entry as Graph entry
  Executor->>Runner: Pass global capacity, DP size, and rank
  Runner->>Peers: Synchronize graph key and graph presence
  Runner->>Entry: Prepare graph for padded sequence capacity
  Runner->>Entry: Copy live inputs and pad unused rows for replay
Loading

Suggested reviewers: xllm-org


Merge Risk | ⚪ Minimal · up to 15d77

Merge Risk: ⚪ Minimal · up to 15d77

This change enables data-parallel ACL graphs for the block-draft path. No concrete merge-blocking defect was found in the reviewed code, and the reported tests and benchmark passed.

Security Architecture Review

Security architecture risk: 🔵 Low · up to 15d77

The change adds coordination between parallel workers and clears unused data between requests. No introduced security defect was confirmed, but device-level cache-write safety and recovery after a participant fails were not fully established.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • inferred — The directly affected state is each runner's persistent graph buffers and bound layer caches, with execution progress coupled across its DP peers. Consequently, a participant failure can affect group progress rather than only one local batch; broader tenant or service exposure cannot be determined from the inspected scope.

Trust Boundaries and Controls

  • observed — Padded cache-write isolation depends on the device provider treating slot mapping -1 as a no-write sentinel. The inspected wrappers forward that value without local masking; provider enforcement was not established, so this dependency is not reported as a verified vulnerability.
  • observed — The block-attention sink uses a page table and per-row device-length mask, not the padded paged-indptr ranges. With padded KV length one and a zero page-table row, its mask permits the first cache position; output slicing, rather than an empty page range, contains those extra outputs. This does not establish cross-request disclosure.

Resilience and Maintainability Implications

  • inferred — Successful local capture has a clear publication boundary, but peer-wide cleanup and recovery after one rank throws remain unproven. Collective groups have configured timeouts; those timeouts alone do not establish coordinated rollback, safe retry, or cache consistency after failure.

Hardening Proposals

  • proposed — Establish the supported device-provider contract for negative cache slots with sink-level isolation checks covering idle peers, shrinking batches, and repeated replay. Check cache contents as well as returned outputs.
  • proposed — Define and verify a group-wide terminal policy for asymmetric capture failure or interruption, including peer release, stale-entry cleanup, and whether recovery requires recapture or worker-group restart.

Pre-merge checks | Passed 4 | Failed 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 38 functions across 8 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check Passed The title clearly identifies the main change: enabling Python DSpark ACL graphs with data parallelism. It is concise, specific, and follows the repository's feat: <subject>. format.
Description check Passed The description clearly explains the feature, implementation scope, testing, validation limits, and benchmark context. It is mostly complete, although it does not include the template headings for Rel…
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.

  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
🛠️ Fix failing CI checks 💡
  • Commit to this branch
  • Create a new PR

🧪 Generate unit tests (beta)
  • Create a new PR

  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Comment @coderabbitai help to get the list of available commands.

@XuZhang99
XuZhang99 merged commit af64dee into xLLM-AI:main Oct 11, 2026
5 of 15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants