[tp] Consolidate branches and validate vLLM/SGLang#6
Draft
rchalamala wants to merge 125 commits into
Draft
Conversation
Co-authored-by: Rahul Chalamala <rchalamala@users.noreply.github.com>
Co-authored-by: Rahul Chalamala <rchalamala@users.noreply.github.com>
Signed-off-by: xenshinu <liuxs@umich.edu>
Co-Authored-By: Rahul Chalamala <22563365+rchalamala@users.noreply.github.com> Signed-off-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Co-Authored-By: Rahul Chalamala <22563365+rchalamala@users.noreply.github.com> Signed-off-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Co-Authored-By: Rahul Chalamala <22563365+rchalamala@users.noreply.github.com> Signed-off-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Co-Authored-By: Rahul Chalamala <22563365+rchalamala@users.noreply.github.com> Signed-off-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Co-Authored-By: Rahul Chalamala <22563365+rchalamala@users.noreply.github.com> Signed-off-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Co-Authored-By: Rahul Chalamala <22563365+rchalamala@users.noreply.github.com> Signed-off-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Signed-off-by: xenshinu <liuxs@umich.edu>
Co-authored-by: Rahul Chalamala <rchalamala@users.noreply.github.com>
Co-authored-by: Rahul Chalamala <rchalamala@users.noreply.github.com>
Co-authored-by: Rahul Chalamala <rchalamala@users.noreply.github.com>
Co-authored-by: Rahul Chalamala <rchalamala@users.noreply.github.com>
Co-authored-by: Rahul Chalamala <rchalamala@users.noreply.github.com>
Co-authored-by: Rahul Chalamala <rchalamala@users.noreply.github.com>
Co-authored-by: Rahul Chalamala <rchalamala@users.noreply.github.com>
Co-authored-by: Rahul Chalamala <rchalamala@users.noreply.github.com>
Co-authored-by: Rahul Chalamala <rchalamala@users.noreply.github.com>
Co-authored-by: Rahul Chalamala <rchalamala@users.noreply.github.com>
Co-authored-by: Rahul Chalamala <rchalamala@users.noreply.github.com>
Co-authored-by: Rahul Chalamala <rchalamala@users.noreply.github.com>
Co-authored-by: Rahul Chalamala <rchalamala@users.noreply.github.com>
Co-authored-by: Rahul Chalamala <rchalamala@users.noreply.github.com>
Co-authored-by: Rahul Chalamala <rchalamala@users.noreply.github.com>
Co-authored-by: Rahul Chalamala <rchalamala@users.noreply.github.com>
Co-authored-by: Rahul Chalamala <rchalamala@users.noreply.github.com>
Co-authored-by: Rahul Chalamala <rchalamala@users.noreply.github.com>
Co-authored-by: Rahul Chalamala <rchalamala@users.noreply.github.com>
Co-authored-by: Rahul Chalamala <rchalamala@users.noreply.github.com>
Co-authored-by: Rahul Chalamala <rchalamala@users.noreply.github.com>
Co-authored-by: Rahul Chalamala <rchalamala@users.noreply.github.com>
Co-authored-by: Rahul Chalamala <rchalamala@users.noreply.github.com>
Co-authored-by: Rahul Chalamala <rchalamala@users.noreply.github.com>
Co-authored-by: Rahul Chalamala <rchalamala@users.noreply.github.com>
The strongest remaining divergence from the proven PR4 backend: our LOAD short-circuited DecodeCudaGraphRunner.capture and manually rebuilt a SimpleNamespace metadata wrapper + pool + seq fill + load_all_graphs. That built the LOAD-time allocator trajectory and FlashInfer metadata wrappers differently from SAVE, so captured kernels referenced wrong workspace addresses and LOAD produced corrupt output despite one graph/rank, positive symmetric AR/AG kernels, zero NCCL nodes, and a stable symmetric buffer. Port PR4's design into the current FullCudaGraphBackend subclass: * backend.py: extend the subclass with LOAD state (_pending / _graph_files / _load_index / _closed) and a capture_session override. On LOAD it establishes the inherited shared pool + stream, preallocates, scans the archive, issues exactly one start_graph_builds(paths, pool=self._pool), yields to the upstream per-shape loop, and on exit fails closed unless every archived file was consumed (then logs the final offset/count). capture_one on LOAD validates the requested dense ShapeKey against the archive order and calls finish_one_graph_load per index — no forward, native capture, or warmup. cleanup() latches _closed to block recapture. SAVE capture path and its skip of the two warmups are unchanged. * hooks.py: the DecodeCudaGraphRunner.capture AROUND LOAD branch now calls original, so the pinned upstream executes graph_capture(), the seq_lens fill, capture_prepare, ForwardContext, real init_forward_metadata_out_graph, and per-shape dispatch — the identical lifecycle SAVE runs. The capture_bs==[256] fail-closed guard and SAVE finalization are unchanged. * graph_ops.py: remove the now-dead manual LOAD path (load_all_graphs, get_or_create_graph_pool) and the SimpleNamespace metadata helpers (build_capture_fb_view, initialize_attention_metadata_for_bs); upstream now owns metadata via a real ForwardBatch. Tests: repurpose test_sglang_graph_load.py to drive the backend LOAD lifecycle through a fake upstream capture() (one-start / per-index finish / exact map+output / order-mismatch / extra / missing-on-exit / no-forward / recapture guard); update test_sglang_capture_hooks.py so LOAD proves it calls original with no manual orchestration and the backend __init__/capture_session/SAVE contracts hold; delete the obsolete attention-metadata helper test. Co-authored-by: Rahul Chalamala <rchalamala@users.noreply.github.com>
…NONE-transparent cleanup Finding 1: the pinned LOAD lifecycle runs the real upstream capture() traversal, so rank0 emits "Capturing batches" progress on LOAD even though the Foundry backend restores archived graphs without any CUDA capture. Using that progress count as a no-recapture gate now falsely fails LOAD/focused/full acceptance. Keep native_capture_progress_count diagnostic and prove no-recapture via positive archived-load counts + zero Foundry graph-save logs instead: rename the focused check (sglang_tp_validation.focused_treatment_checks) and the full check (modal_sglang_tp) to load_restored_without_resave with predicate saved_graph_log_count == 0 and sum(loaded_graphs) > 0. The backend LOAD no-forward/no-CUDAGraph/no-context + exact-restored-maps proof remains in the CPU backend contracts (test_sglang_graph_load.py). Focused contract now also asserts a high native_capture_progress_count still VERIFIES (lifecycle traversal), and flips load_restored_without_resave on a save log or zero restored counts. Finding 2: make subclass cleanup() transparent in NONE mode by delegating to super().cleanup() and not latching _closed, so the runner's legitimate hidden-mode recapture keeps working in stock operation. Active SAVE/LOAD retains the fail-closed recapture latch and its extended cleanup. Adds NONE-transparency and active-latch contracts. Co-authored-by: Rahul Chalamala <rchalamala@users.noreply.github.com>
Co-authored-by: Rahul Chalamala <rchalamala@users.noreply.github.com>
Co-authored-by: Rahul Chalamala <rchalamala@users.noreply.github.com>
Co-authored-by: Rahul Chalamala <rchalamala@users.noreply.github.com>
Co-authored-by: Rahul Chalamala <rchalamala@users.noreply.github.com>
Co-authored-by: Rahul Chalamala <rchalamala@users.noreply.github.com>
Co-authored-by: Rahul Chalamala <rchalamala@users.noreply.github.com>
Co-authored-by: Rahul Chalamala <rchalamala@users.noreply.github.com>
decode_log_interval defaults to 40, so the short concurrent bs>1 decode in the TP acceptance harness emitted no '#running-req ... cuda graph: True' metrics line and the restored_graph_replay_observed evidence silently disappeared. Add --decode-log-interval 1 to the TP recipe and require the resolved value 1 in the supported-profile guard (fails closed on missing/other values), so every phase logs every decode batch. No generation/output logic changed; the bs>1 gate is unchanged. Co-authored-by: Rahul Chalamala <rchalamala@users.noreply.github.com>
Co-authored-by: Rahul Chalamala <rchalamala@users.noreply.github.com>
Co-authored-by: Rahul Chalamala <rchalamala@users.noreply.github.com>
Co-authored-by: Rahul Chalamala <rchalamala@users.noreply.github.com>
Co-authored-by: Rahul Chalamala <rchalamala@users.noreply.github.com>
Co-authored-by: Rahul Chalamala <rchalamala@users.noreply.github.com>
Address Task 1 review findings 1, 2, 5, 6 (plus policy/source contracts for finding 3): - Claim process ownership at the very start of cuIpcGetMemHandle, cuIpcOpenMemHandle, and cuIpcCloseMemHandle, before any allocation metadata, chunk map, or magic-dependent state is touched. Applied uniformly to plain and VMM handles (conservative all-IPC claim) so fork behavior is consistent; server init is also covered. - Export invalidation now returns before any map access unless the caller is the exact owner: an unclaimed process (-1) has nothing exported and a fork child must not touch the inherited concurrent map. - Fork-child rejection is async-signal-safe: a fixed compile-time message (including 'use spawn/exec') via write(2), plus close(2) of the inherited listener fd; no fprintf/snprintf/malloc/locks. The parked-fd registry is intentionally left untouched (documented) since its concurrent map cannot be locked safely in a fork child. - require_exact_ipc_reservation cleanup callbacks now return CUresult; both run exactly once on the wrong-VA path (a failed address-free never skips the handle release -> no leak; neither is retried -> no double release). The owner process logs cleanup failures. - Factor the ownership decision into a pure, CUDA-free policy header (decide_ipc_ownership / owns_ipc_state / reject message) with a C++ unit test and a hook.cpp source contract for the guards that a CUDA fork child cannot be driven to assert natively. - Mark both host-policy test executables EXCLUDE_FROM_ALL so editable builds skip them; explicit target + ctest paths remain. Co-authored-by: Rahul Chalamala <rchalamala@users.noreply.github.com>
Address Task 1 review findings 3 (native) and 4:
- Expand test_vmm_ipc_rejects_use_after_fork so the post-claim fork
child exercises cuIpcGetMemHandle, cuIpcOpenMemHandle, and
cuIpcCloseMemHandle (each must return CUDA_ERROR_NOT_SUPPORTED) and
assert the exact 'use spawn/exec' rejection message on captured
stderr; the owner remains usable afterwards.
- Add durable native lifecycle regressions that assert observable
usability/failure (device memset + memcpy round trips), not prints:
- stale-export-fetch: a live same-process import reads the exported
bytes; after cuMemFree the parked fd is invalidated and a later
open of the same handle fails.
- duplicate-interior-open: opening the same imported interior twice is
refcounted (same VA, usable after the first close, torn down only
after the second, re-openable afterward).
- generation-recreate: freeing and recreating a preallocation at the
same base bumps the chunk generation and maps fresh data.
These reuse the existing LD_PRELOAD spawn helpers and run under the
Modal focused suite (local VM has no CUDA device).
Co-authored-by: Rahul Chalamala <rchalamala@users.noreply.github.com>
Address remaining Task 1 review findings 5 and 6. Finding 5 (claim ordering + lock-free owner pid): - Move vmm_ipc_claim_process() to be the first executable statement in cuIpcGetMemHandle, cuIpcOpenMemHandle, and cuIpcCloseMemHandle, ahead of the cached CUDA_DRIVER_CALL symbol lookup (which takes a lock a post-claim fork child must not touch). A leading typedef is not executable, so the real entry-point resolution now happens only after the claim/reject decision. - Add static_assert(std::atomic<pid_t>::is_always_lock_free): the owner pid is CAS'd on the async-signal-safe fork-child reject path, where a lock-backed atomic could deadlock. Finding 6 (fallback free + parked-fd docs): - Check and log the initial driver-fallback reservation free in the owning importer before re-reserving in the dedicated import zone; a silent failure would leak that reservation. The wrong-zone cleanup callbacks already fail closed and log. - Document the parked-fd limitation accurately: a rejected fork child cannot lock the export-fd map, so it keeps inherited duplicate fds that pin the exporter's physical allocations until the child exits or execs (FD_CLOEXEC closes them on exec). Source contracts in test_vmm_ipc_ownership_contract.py cover the new claim-before-driver-lookup ordering, the lock-free static_assert, the fallback-free logging, and the parked-fd pinning documentation. Co-authored-by: Rahul Chalamala <rchalamala@users.noreply.github.com>
…oofs Address remaining Task 1 review findings 1-4 in the native lifecycle suite. Every lifecycle subprocess and protocol wait is now bounded (_LIFECYCLE_TIMEOUT_S); hangs fail with captured stdout/stderr and kill the child (finding 4). Helpers _run_capture and _wait_for_file centralize the timeout + diagnostics; the fork child keeps its signal.alarm guard. Finding 1 (generation recreate truly reuses the same VA): fully free the preallocation and stop, then set_allocation_region + preallocate at the same base in the same owner process. Assert the recreated source pointer and chunk base are equal and the generation differs. A first-generation mapping is deliberately kept open so that, were the generation dropped from the mapping key, the second open would alias that stale mapping and read the old bytes; with the generation in the key it maps fresh 0xB2. The retained mapping is never dereferenced (only closed at teardown). Finding 2 (cross-process stale exporter over socket + SCM_RIGHTS + token): replace the same-process stale test with a real exporter/importer protocol. The exporter allocates, exports (starting the abstract-socket server thread), publishes the serialized 64-byte handle blob, and stays alive. A fresh importer process opens that blob, fetching the fd across processes via SCM_RIGHTS (token verified server-side) and reads the sentinel. After the exporter frees and exits, a fresh importer using the same serialized handle fails promptly because the socket is gone. Finding 3 (duplicate open/close proves the VA is unreserved): after the second close of a refcounted interior mapping, reserve exactly the former whole-chunk range [local_base, local_base+chunk_size) with local_base as the hint (computed from the interior pointer and the handle's chunk base). A still-reserved range would relocate the reservation; requiring the hint be honored proves the VA is free, with no dereference of the freed range. Then a fresh open remaps and reads current data. These run under the Modal focused suite (the local VM has no CUDA device). Co-authored-by: Rahul Chalamala <rchalamala@users.noreply.github.com>
…free Address Task 1 review findings 1 (pure part), 2, and 4. Finding 1 (generation-sensitive key as a pure, testable contract): - Add include/foundry/vmm_ipc_chunk_key.h: a CUDA-free ChunkKey value type (exporter pid, token, chunk base, generation) with generation-inclusive equality/ordering/hash and a make_chunk_key factory. The hook now keys vmm_ipc_chunk_mappings on foundry::vmm_ipc::ChunkKey via make_chunk_key instead of an ad-hoc std::make_tuple, so the generation can never be dropped by accident. - Add tests/vmm_ipc_chunk_key_test.cpp (CPU-only, EXCLUDE_FROM_ALL + ctest): proves same pid/token/base + different generation are DISTINCT map/hash keys, and that a generation-less identity collapses two recreated chunks onto one key (the exact stale-mapping-reuse bug the field prevents). This replaces the fragile native same-VA discriminator with a hermetic contract; no unsafe production rewind/unset API is introduced. Finding 2 (fail closed on a failed fallback reservation free): - If freeing the driver's fallback reservation fails, release the imported handle exactly once, log both cleanup statuses, and return immediately with CUDA_ERROR_OUT_OF_MEMORY - never continue to the dedicated-zone mapping. The decision is factored into the pure require_fallback_reservation_freed helper (vmm_ipc_reservation.h) with cases 6-7 in the reservation policy test. No double release (the handle is released on the failure branch only, which returns before the zone path). Finding 4 (parked-fd limitation kept explicit and fail-fast): - Guard contract: the fork-child reject path closes the inherited listener fd, clears it, and does not access the inherited concurrent export-fd map. The limitation (inherited duplicate fds pin physical memory until the child exits/execs) stays documented; no complete child-FD reclamation is claimed. Source contracts in test_vmm_ipc_ownership_contract.py cover the pure key type, the fail-closed-before-zone ordering with both statuses logged, and the listener-close / no-map-access reject invariant. Co-authored-by: Rahul Chalamala <rchalamala@users.noreply.github.com>
…aits Address Task 1 review findings 1 (native part) and 3. Finding 1 (native complement, no impossible same-VA re-reservation): - Rewrite the generation-recreate native test to assert only OBSERVABLE behavior: the exported chunk generation increments across a full free + stop + recreate cycle (owner process stays), and a fresh import reads the recreated (fresh) bytes, never stale data. Drop the assertions that the recreated source pointer / chunk base are identical - the peer-import zone hint advances monotonically and is never rewound, so the local import VA legitimately differs; this is named and documented. The generation-sensitive key identity itself is proven hermetically by the pure CPU test tests/vmm_ipc_chunk_key_test.cpp. Finding 3 (every wait bounded, including post-kill cleanup): - Add _kill_and_collect: kills a process and collects its output under a bounded secondary wait; if the process will not terminate after the kill it raises with diagnostics rather than blocking forever. - Route _run_capture (now Popen + communicate(timeout)), _wait_for_file, the exporter signal-exit path, and the orchestrator teardown through it, so no post-kill communicate()/wait() can hang indefinitely. Secondary timeout is _KILL_TIMEOUT_S. These run under the Modal focused suite (the local VM has no CUDA device). Co-authored-by: Rahul Chalamala <rchalamala@users.noreply.github.com>
The two new C++ files briefly carried a stray hash-style SPDX header (the Python-only SPDX fixer was run against them by mistake) on top of the correct C++ // header. Remove the hash lines so the files carry a single, valid //-comment SPDX header and pass clang-format. Co-authored-by: Rahul Chalamala <rchalamala@users.noreply.github.com>
…eration Address the sole remaining Task 1 review finding for the native generation-recreate test. - Keep a SINGLE allocation region for both generations: set the region once and stop it once. Between generations only the preallocation is freed and recreated. Because the region cursor advances monotonically and is never rewound (cuMemFree_v2 / free_preallocated_region do not rewind it), generation 2's preallocation lands at a DIFFERENT chunk base within the same region. Assert the observable behavior: the chunk base differs (cb2 != cb1), the global generation counter increments (gen2 > gen1), and a fresh import reads the recreated bytes (0xB2), never stale generation-1 data. No stop/set between generations and no same-VA claim. - Add tests/test_generation_recreate_contract.py: a pure text source contract (no CUDA/torch/foundry import) that guards the invariant - set/stop each appear exactly once in the generation test, only the preallocation is recreated, and no identical-VA/chunk-base claim can reappear. The generation-sensitivity of the mapping key itself remains proven hermetically by tests/vmm_ipc_chunk_key_test.cpp; this native test complements it with a real device round trip. Runs under the Modal focused suite (the local VM has no CUDA device). Co-authored-by: Rahul Chalamala <rchalamala@users.noreply.github.com>
Address the sole remaining Task 1 review finding for the native generation-recreate test. After closing the importer and deleting each generation's tensor, synchronize and call torch.cuda.empty_cache() BEFORE free_preallocated_region so the caching allocator cannot retain a cached block pointing into that generation's soon-to-be-unmapped backing. empty_cache() releases blocks via cuMemFree_v2, which does not rewind the region cursor, so the monotonic cursor (and thus the distinct generation-2 chunk base) is preserved. Add a source contract asserting the del tensor -> empty_cache -> free_preallocated_region ordering in the generation loop. Co-authored-by: Rahul Chalamala <rchalamala@users.noreply.github.com>
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Summary
mainSGLang main — VERIFIED
Candidate
435518206f501b2f6b20080ee53375d0d6a3ff76Run: https://modal.com/apps/modal-labs/rahul-dev/ap-RHFcD96CAsGSqO7Z236yzF
a25164bda355250015c0ba6d8821f0d4b37abe7db968826d9c46dd6066d109eabc6255188de9121868908220416in every Foundry phaseFocused treatment: https://modal.com/apps/modal-labs/rahul-dev/ap-SwjRFbh7zqLfmKlGGkDuTM
Plain-NCCL restore is explicitly rejected: it restored structurally but produced deterministic corrupt decode output.
Remaining before final handoff
The final branch audit found selective late deltas from the advanced vLLM/no-IMEX branches: native IPC fork/pitched/peer-zone hardening, archive-defined DeepEP transport, strict DeepEP replay output checks, and a complete vLLM SAVE2/LOAD/LOAD2 immutable-archive matrix. These are being ported selectively without merging obsolete SGLang experiments.
Supported scope
Pinned experimental profiles only. SGLang: single-host dense Qwen3-8B TP=2, torch symmetric memory, one full-decode graph. vLLM: single-host dense Qwen3-8B TP=2, V1/full-decode PyNCCL P2P/IPC. Broader topology/backends remain unsupported.