Fix HiCache transfer timing and enable_metrics threading#2
Open
ishandhanani wants to merge 21 commits into
Open
Conversation
1. Transfer durations were always host wall-clock, never device time. Every event pair fed to elapsed_time() was created without enable_timing=True, so on CUDA elapsed_time() raises 'Both events must be created with enable_timing=True' and _transfer_elapsed_us silently falls into its except-fallback. The fallback measures enqueue -> ack-poll latency, not DMA time: an idle onboard logged xfer_us=5751205 (5.75 s, 0.03 GB/s) for a 161 MB transfer that takes ~25 ms, because the ack sat until the next request triggered loading_check. Create the pairs used for timing (write start/finish, LayerLoadingEvent start + last layer event) with enable_timing=True so the device path actually runs. 2. enable_metrics was only threaded into HiRadixCache's controller. The decode-side offload manager and all five HybridCacheController construction sites in hybrid_pool_assembler.py left it defaulted to False, so disagg-decode / hybrid / unified stacks recorded DEBUG logs but never Prometheus, even with --enable-metrics. Pass enable_metrics and extra_metric_labels through. 3. Drop the unrelated docs_new/index.mdx blog-card rotation (same change was already removed from the sibling PR #26976).
5 tasks
694bd4a to
45ac1b0
Compare
aa6964d to
7d2dbed
Compare
39dc35a to
e90f3e3
Compare
be53ff3 to
169c7ad
Compare
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.
Follow-up to sgl-project#26974 — re-tested the updated branch on 1x L40S (Qwen3-0.6B, HiCache, write_through, page_size 64,
--enable-metrics --log-level debug). All previously-flagged issues are fixed: the decode-side ack consumers work, write-back paths record, the redundant counters are gone, and counters are exact (bytes/token = 114688 = 2 x 28 layers x 8 KV heads x 128 dim x 2B for Qwen3-0.6B). Two remaining issues found, fixed here.1. Transfer durations were always host wall-clock, never device time
Every event pair fed to
elapsed_time()is created withoutenable_timing=True, so on CUDAelapsed_time()raisesand
_transfer_elapsed_ussilently falls into itsexceptfallback on every transfer. The fallback measures enqueue -> ack-poll latency, not DMA time: an idle onboard of 161 MB loggedxfer_us=5751205(5.75 s, 0.03 GB/s) for a ~7 ms transfer, because the ack sat inack_load_queueuntil the next request triggeredloading_check. The "prefer device timing" path was dead code.Fix: create the pairs actually used for timing with
enable_timing=True(write start/finish in both controllers,LayerLoadingEvent.start_event+ last layer event, which doubles asfinish_event). The host fallback remains for backends withoutelapsed_time().2.
enable_metricsonly reached HiRadixCache's controllerdecode_kvcache_offload_manager.pyand all fiveHybridCacheControllerconstruction sites inhybrid_pool_assembler.pyleftenable_metricsdefaulted toFalse, so the disagg-decode / hybrid / unified stacks emitted DEBUG logs but never Prometheus, even with--enable-metrics. Threadedenable_metrics+extra_metric_labelsthrough.Also reverts the unrelated
docs_new/index.mdxblog-card rotation (same change already removed from sibling PR sgl-project#26976).Verification (same box/config)
xfer_us=5751205(5.75 s, 0.03 GB/s)xfer_us=6781(6.8 ms, 25.98 GB/s)_count/_sumSGLANG_ENABLE_UNIFIED_RADIX_TREE=1+ HiCache)test_specv2_kvcache_offloading.py)--enable-metrics(INFO level, load-back traffic)hi_mamba / SWA hybrid stacks need hybrid models this box cannot run; the threading change there is identical to the unified path verified above.