Repository navigation
[dev](perf): DSA pre-attention: plan-driven scorer dispatch, host-built CP layout metadata - #6206
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. |
99f9d47 to
a539bfc
Compare
…tention (NVIDIA#6206) Signed-off-by: Robin Zhang <robinz@nvidia.com>
|
/ok to test bd85b78 |
|
/ok to test 4888213 |
|
/ok to test 9fd365a |
Signed-off-by: jingqiny-99 <jingqiny@nvidia.com>
Signed-off-by: jingqiny-99 <jingqiny@nvidia.com>
Signed-off-by: jingqiny-99 <jingqiny@nvidia.com>
Signed-off-by: jingqiny-99 <jingqiny@nvidia.com>
Signed-off-by: jingqiny-99 <jingqiny@nvidia.com>
Signed-off-by: jingqiny-99 <jingqiny@nvidia.com>
Signed-off-by: jingqiny-99 <jingqiny@nvidia.com>
Signed-off-by: jingqiny-99 <jingqiny@nvidia.com>
|
/ok to test 7986148 |
| return get_attr_wrapped_model(model, "config", allow_none=False) | ||
|
|
||
|
|
||
| def move_host_tensor_to_device(values: torch.Tensor, device: torch.device) -> torch.Tensor: |
There was a problem hiding this comment.
Should we merge this helper with maybe_move_tensor_to_cpu? https://github.com/NVIDIA/Megatron-LM/blob/main/megatron/core/transformer/moe/moe_utils.py#L1238
I think maybe_move_tensor_to_cpu is in a bad place. Moving it here would be better.
BTW looks like maybe_move_tensor_to_cpu is using non_blocking always. We may need a new argument synchronization: bool = True, which determines whether we add a stream sync after the non-blocking copy.
Ignore me if you think this is a bad idea.
There was a problem hiding this comment.
I think you are right, the maybe_move_tensor_to_cpu is in a strange place, I think it's good to move it here, and merge them in the future, because in my opinion, it may become a large helper (move tensor to arbitrary device and maybe do some synchronization if need), and it isn't transparent to its callers.
Signed-off-by: jingqiny-99 <jingqiny@nvidia.com>
Signed-off-by: jingqiny-99 <jingqiny@nvidia.com>
Signed-off-by: jingqiny-99 <jingqiny@nvidia.com>
Preserve packed DSA scoring geometry and Dynamic CP validation while incorporating upstream indexer warnings and graph/offload changes. Signed-off-by: jingqiny-99 <jingqiny@nvidia.com>
|
/ok to test 88a285f |
Signed-off-by: jingqiny-99 <jingqiny@nvidia.com>
|
/ok to test cff3e90 |
|
|
||
| def move_host_tensor_to_device(values: torch.Tensor, device: torch.device) -> torch.Tensor: | ||
| """Move a host tensor to ``device``.""" | ||
| return values.to(device, non_blocking=device.type == "cuda") |
There was a problem hiding this comment.
Do we need values.pin_memory().to(device) to avoid pageable H2D?
There was a problem hiding this comment.
I think it is right, but in my test case, removing pin_memory will save copy time. So I will add an argument here to decide whether run values.pin_memory() here, and its default value will be False.
There was a problem hiding this comment.
Is it because your values are already on pin? Non-pin h2d is dangerous since pagable copy is blocking and non-graphable IIRC.
There was a problem hiding this comment.
soga..Thanks for teaching me, and sorry for my limited knowledge. I will pin the memory by default.
There was a problem hiding this comment.
Happen to read an article about a hang issue caused by pagable memcpy: https://mp.weixin.qq.com/s/uOJALDKb588mFynaeJQJoA
Signed-off-by: jingqiny-99 <jingqiny@nvidia.com>
|
/ok to test 9659ab5 |
Signed-off-by: jingqiny-99 <jingqiny@nvidia.com>
|
/ok to test b7cd2d5 |
buptzyb
left a comment
There was a problem hiding this comment.
I have to admit I cannot understand every single detail of this PR. But the code organization and functionality generally LGTM. One concern is the main branch is also making their refactoring to the dsa code (like #7211), so the main-dev divergency would be larger and larger. Beg @hxbai for a final review, thanks!
Summary
This PR makes DSA indexer scoring correct for general packed THD inputs and removes repeated layout construction from the packed context-parallel hot path.
The central semantic correction is that packed sequence cardinality is independent of context parallelism: CP=1 can contain multiple sequences, and CP>1 can contain either one or many. Scorer selection now follows the actual THD metadata and kernel preconditions instead of using CP size as a proxy for sequence count.
The final branch preserves existing configuration and backend-hook compatibility. It does not claim full CUDA Graph support for packed DSA+CP. Such configurations remain constructible and receive an experimental-support warning; capture/replay support is follow-up work.
What changed
1. Plan-driven general THD scoring
DECLINEoutcome before a cuDNN launch.This removes both invalid implications: CP=1 no longer means “one sequence”, and CP>1 no longer means “multiple sequences”.
2. Host-built packed-CP layout metadata
PackedSeqParams, so later DSA layers reuse the first build.positions_and_reordermemo sites, so equal geometry has an equal cache key.Independent CPU goldens cover CP1/2/4, both/all ranks, multiple sequences, zero-length entries, all-empty packs, padding, distinct Q/K boundaries, and every Q/K cover-flag combination. A production-forward regression test also enters both memo sites and proves that the second site reuses the first result.
3. Compatibility and invariant hardening
use_local_indexer_varlenandsingle_packed_thd_sequenceon their CP-only contract.megatron.core.utils.filter_kwargs_for_callable; exact signatures,**kwargs, positional-only parameters, opaque callables, and the identity-safe signature cache have direct tests.varlen_is_plain_causalcontract.cp_comm_typeinto attention layers for Dynamic CP even when configured CP is 1; the effective microbatch CP group may still be larger than 1.starts/endsautograd contract after the caller's concrete-bounds guard.4. CUDA Graph scope
Packed DSA+CP graph configurations are no longer rejected by an empirical hardware/version matrix. The final check is structural and warning-only:
cuda_graph_impl="none") is unchanged.The evidence is diagnostic, not an allow/deny table. On the validation GB200 image, cuDNN static CP2 eager execution passes, while local attention capture reaches graph warmup and fails when the cloned packed metadata is empty. Backend
nonewas previously measured only for static CP2/local/attention and also failed during graph warmup. TileLang was unavailable in that image and remains unmeasured. No unmeasured combination is presented as known-good, and none is hard-rejected by this PR.Full capture/replay support for changing packed layouts is intentionally deferred.
Validation
All GPU work below ran on Lyris GB200 in the standard
gb200partition (no backfill), usingdsv4-gb200-torch2604-nvrx060.Only the conclusion-relevant H2D nsys report was retained; correctness probes were run without profiling.
Commit map