Skip to content

Commit d7f8578

Browse files
committed
fix ups
Signed-off-by: Athena Cai <athenac@nvidia.com>
1 parent 50d6331 commit d7f8578

9 files changed

Lines changed: 170 additions & 69 deletions

File tree

cpp/include/tensorrt_llm/batch_manager/kvCacheManager.h

Lines changed: 7 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -682,15 +682,13 @@ class GenerationRequest
682682
++mNumFrontBlocksRemovedPerWindow.at(windowSize);
683683
}
684684

685-
//! \brief Advance ``mNumFrontBlocksRemoved`` without touching cache blocks.
685+
//! \brief Advance the per-window front-block counter without touching cache blocks.
686686
//! \details Used by ``BlockManager::releasePrefixBlocks`` to advance the
687-
//! shared front-block counter once after every ``WindowBlockManager`` has
688-
//! processed the same prefix range. Has clearer intent than calling
689-
//! ``removeFrontBlock`` with a sentinel ``windowSize`` value, and is robust
690-
//! to future changes that consume the ``windowSize`` argument.
691-
void incrementNumFrontBlocksRemoved()
687+
//! single-window front-block counter once after every ``WindowBlockManager`` has
688+
//! processed the same prefix range.
689+
void incrementNumFrontBlocksRemoved(SizeType32 windowSize)
692690
{
693-
++mNumFrontBlocksRemoved;
691+
++mNumFrontBlocksRemovedPerWindow.at(windowSize);
694692
}
695693

696694
void removeLastBlock(SizeType32 windowSize)
@@ -990,7 +988,7 @@ class WindowBlockManager
990988
//! for blocks whose data has already been transferred. Reuses the
991989
//! detachFrontBlock mechanism (decRefCount + eviction policy release).
992990
//! Called by BlockManager::releasePrefixBlocks which coordinates the
993-
//! shared mNumFrontBlocksRemoved counter across all window managers.
991+
//! single-window front-block counter across all window managers.
994992
void releasePrefixBlocks(GenerationRequest& sequence, SizeType32 startIdx, SizeType32 numBlocks);
995993

996994
//! \brief Simulate freeing all blocks for that sequence to check impact on number of free blocks
@@ -1536,7 +1534,7 @@ class BlockManager
15361534

15371535
//! \brief Release the first numBlocks prefix blocks of a sequence.
15381536
//! \details Mirrors detachFrontBlock logic: decRefCount + eviction policy
1539-
//! release for each prefix block. The mNumFrontBlocksRemoved counter on
1537+
//! release for each prefix block. The front-block counter on
15401538
//! GenerationRequest ensures releaseBlocks (called during removeSequence)
15411539
//! skips already-freed prefix blocks.
15421540
void releasePrefixBlocks(GenerationRequest& sequence, SizeType32 numBlocks);

cpp/tensorrt_llm/batch_manager/kvCacheManager.cpp

Lines changed: 24 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -2894,22 +2894,22 @@ void BlockManager::releasePrefixBlocks(GenerationRequest& sequence, SizeType32 n
28942894
// today (gated by should_store_blocks: not is_vswa in the executor and
28952895
// beamWidth == 1 assertion in WindowBlockManager::releasePrefixBlocks).
28962896
//
2897+
auto const windowSize = mWindowBlockManagers.cbegin()->first;
28972898
// Snapshot the counter before iterating so that every WindowBlockManager
28982899
// releases the same range. Without this, the first manager would advance
2899-
// the shared mNumFrontBlocksRemoved counter and subsequent managers would
2900-
// see the counter already at the target, skipping their own blocks.
2901-
SizeType32 const startIdx = sequence.getNumFrontBlocksRemoved();
2900+
// the single-window front-block counter and subsequent managers would see
2901+
// the counter already at the target, skipping their own blocks.
2902+
SizeType32 const startIdx = sequence.getNumFrontBlocksRemoved(windowSize);
29022903
for (auto& [_, manager] : mWindowBlockManagers)
29032904
{
29042905
manager.releasePrefixBlocks(sequence, startIdx, numBlocks);
29052906
}
2906-
// Advance the shared counter once, after all managers have released.
2907+
// Advance the single-window counter once, after all managers have released.
29072908
// Uses incrementNumFrontBlocksRemoved (counter-only) instead of
2908-
// removeFrontBlock so the intent is explicit and we do not depend on
2909-
// removeFrontBlock ignoring its windowSize argument.
2910-
while (sequence.getNumFrontBlocksRemoved() < numBlocks)
2909+
// removeFrontBlock so the intent is explicit.
2910+
while (sequence.getNumFrontBlocksRemoved(windowSize) < numBlocks)
29112911
{
2912-
sequence.incrementNumFrontBlocksRemoved();
2912+
sequence.incrementNumFrontBlocksRemoved(windowSize);
29132913
}
29142914
}
29152915

@@ -3727,23 +3727,30 @@ void WindowBlockManager::releasePrefixBlocks(GenerationRequest& sequence, SizeTy
37273727
auto& allocatedBlocks = mAllocatedBlocksPerSeq.at(requestId);
37283728
SizeType32 const target = std::min(numBlocks, static_cast<SizeType32>(allocatedBlocks.size()));
37293729

3730-
// Release blocks in range [startIdx, target). The shared
3731-
// mNumFrontBlocksRemoved counter is advanced by BlockManager after
3730+
// Release blocks in range [startIdx, target). The single-window
3731+
// front-block counter is advanced by BlockManager after
37323732
// all WindowBlockManagers have processed the same range.
37333733
for (SizeType32 blockIdx = startIdx; blockIdx < target; ++blockIdx)
37343734
{
37353735
auto& block = allocatedBlocks.at(blockIdx);
3736+
auto releasedBlock = block;
37363737

37373738
TLLM_LOG_DEBUG("%s::releasePrefixBlocks - Releasing block %d from sequence %lu", mLogPrefix.c_str(),
3738-
block->getBlockId(), requestId);
3739+
releasedBlock->getBlockId(), requestId);
37393740

3740-
if (block->hasRefs())
3741+
// Replace the sequence slot with a placeholder, matching detachFrontBlock().
3742+
// removeSequence later walks allocatedBlocks in releaseBlocks(); leaving the
3743+
// real block here would release it a second time and corrupt the eviction
3744+
// policy's free-block count.
3745+
block = KVCacheBlock::createPlaceholder();
3746+
3747+
if (releasedBlock->hasRefs())
37413748
{
3742-
block->decRefCount();
3749+
releasedBlock->decRefCount();
37433750
}
3744-
if (!block->hasRefs())
3751+
if (!releasedBlock->hasRefs())
37453752
{
3746-
mEvictionPolicy->releaseBlock(block);
3753+
mEvictionPolicy->releaseBlock(releasedBlock);
37473754
}
37483755
}
37493756
}
@@ -3926,8 +3933,8 @@ std::optional<KVCacheBlock::IdType> KVCacheManager::removeSequence(
39263933

39273934
void KVCacheManager::releasePrefixBlocks(RequestIdType requestId, SizeType32 numBlocks)
39283935
{
3929-
// Hard precondition: BlockManager::releasePrefixBlocks advances the shared
3930-
// mNumFrontBlocksRemoved counter to numBlocks for every WindowBlockManager,
3936+
// Hard precondition: BlockManager::releasePrefixBlocks advances the
3937+
// single-window front-block counter to numBlocks for every WindowBlockManager,
39313938
// even when a window has fewer than numBlocks allocated. Under variable
39323939
// sliding window attention (VSWA), that would cause WindowBlockManager::
39333940
// releaseBlocks (called during removeSequence) to underrun rbegin() and

cpp/tests/unit_tests/batch_manager/kvCacheManagerTest.cpp

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -237,6 +237,59 @@ TEST_F(KVCacheManagerTest, BlockManagerTest)
237237
std::runtime_error);
238238
}
239239

240+
TEST_F(KVCacheManagerTest, BlockManagerReleasePrefixBlocksDoesNotDoubleFreeOnTeardown)
241+
{
242+
auto constexpr numLayers = 12;
243+
auto constexpr numKvHeads = 6;
244+
auto constexpr sizePerHead = 128;
245+
auto constexpr tokensPerBlock = 4;
246+
auto constexpr blocksInPrimaryPool = 8;
247+
auto constexpr blocksInSecondaryPool = 0;
248+
auto constexpr maxNumSequences = 8;
249+
auto const stream = std::make_shared<tr::CudaStream>();
250+
251+
auto constexpr beamWidth = 1;
252+
auto constexpr numBlocksPerBeam = 4;
253+
auto constexpr numTokens = tokensPerBlock * numBlocksPerBeam;
254+
auto constexpr maxAttentionWindow = numTokens;
255+
256+
auto const blocksPerWindow = BlocksPerWindow{{maxAttentionWindow, {blocksInPrimaryPool, blocksInSecondaryPool}}};
257+
258+
BlockManager blockManager(std::vector(numLayers, numKvHeads), sizePerHead, tokensPerBlock, blocksPerWindow,
259+
maxNumSequences, stream, maxAttentionWindow, beamWidth,
260+
std::vector<BlockManager::SizeType32>{maxAttentionWindow}, nvinfer1::DataType::kHALF, 0, maxAttentionWindow);
261+
blockManager.allocatePools(false);
262+
263+
SizeType32 constexpr maxNewTokens{0};
264+
tr::SamplingConfig const samplingConfig{beamWidth};
265+
bool constexpr isStreaming{false};
266+
267+
auto tokens = std::make_shared<VecTokens>();
268+
for (SizeType32 i = 0; i < numTokens; ++i)
269+
{
270+
tokens->push_back(i);
271+
}
272+
273+
LlmRequest::RequestIdType constexpr requestId{42};
274+
auto llmReq = std::make_shared<LlmRequest>(requestId, maxNewTokens, tokens, samplingConfig, isStreaming);
275+
GenerationRequest seq{requestId, numTokens, beamWidth, blockManager.getWindowSizesMetadata()};
276+
277+
(void) blockManager.addSequenceBatch(
278+
{&seq}, {numTokens}, {numBlocksPerBeam}, {std::ref(*llmReq)}, maxAttentionWindow, /*isEnableBlockReuse=*/false);
279+
EXPECT_EQ(blockManager.getNumFreeBlocks(), blocksInPrimaryPool - numBlocksPerBeam);
280+
281+
blockManager.releasePrefixBlocks(seq, 2);
282+
EXPECT_EQ(blockManager.getNumFreeBlocks(), blocksInPrimaryPool - 2);
283+
284+
// releasePrefixBlocks has cumulative semantics. This should release only
285+
// one additional block rather than releasing the first two again.
286+
blockManager.releasePrefixBlocks(seq, 3);
287+
EXPECT_EQ(blockManager.getNumFreeBlocks(), blocksInPrimaryPool - 1);
288+
289+
blockManager.releaseBlocks(seq);
290+
EXPECT_EQ(blockManager.getNumFreeBlocks(), blocksInPrimaryPool);
291+
}
292+
240293
template <typename T>
241294
void writePatternToOffloadedBlocksDRAM(T* rawBlockPtr, int blockSize, int mask)
242295
{

tensorrt_llm/_torch/disaggregation/native/transfer.py

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1149,6 +1149,10 @@ def disagg_request_id(self) -> int:
11491149
def status(self) -> SessionStatus:
11501150
if self._terminal_status is not None:
11511151
return self._terminal_status
1152+
if self._exception is not None or any(t.status == TaskStatus.ERROR for t in self.kv_tasks):
1153+
return SessionStatus.ERROR
1154+
if self.aux_task is not None and self.aux_task.status == TaskStatus.ERROR:
1155+
return SessionStatus.ERROR
11521156
kv_all_transferred = bool(self.kv_tasks) and all(
11531157
t.status == TaskStatus.TRANSFERRED for t in self.kv_tasks
11541158
)
@@ -1731,15 +1735,15 @@ def process_kv_agent_result(
17311735
)
17321736

17331737
def process_aux_agent_result(self, _peer_rank: int, status: AgentResult):
1734-
# Aux is session-level (not per-slice); expected_transfers is identical
1735-
# across all kv_tasks, so any task provides the right count.
1738+
# Aux is session-level (not per-slice); use the final KV task's
1739+
# expected transfer count so chunked sessions wait for all senders.
17361740
with self.lock:
17371741
if not self._kv_tasks:
17381742
logger.warning(
17391743
f"Aux result received before any KV tasks for request {self.request_id}"
17401744
)
17411745
return
1742-
task = self._kv_tasks[0]
1746+
task = self._kv_tasks[-1]
17431747
if status == AgentResult.SUCCESS:
17441748
self._aux_count += 1
17451749

tensorrt_llm/_torch/disaggregation/transceiver.py

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -351,6 +351,10 @@ def _make_chunk_callback(self) -> Optional[Callable]:
351351
release_queue = self._pending_prefix_releases
352352

353353
def _on_chunk_transferred(request_id: int, chunk_block_offset: int, num_blocks: int):
354+
logger.debug(
355+
f"Early release _on_chunk_transferred: request_id: {request_id}, "
356+
f"chunk_block_offset: {chunk_block_offset}, num_blocks: {num_blocks}"
357+
)
354358
cumulative_blocks = chunk_block_offset + num_blocks
355359
release_queue.put((request_id, cumulative_blocks))
356360

tensorrt_llm/_torch/models/__init__.py

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@
1212
from .modeling_deepseekv3 import DeepseekV3ForCausalLM
1313
from .modeling_exaone4 import Exaone4ForCausalLM
1414
from .modeling_exaone4_5 import Exaone4_5_ForConditionalGeneration
15-
from .modeling_exaone_moe import ExaoneMoeForCausalLM
1615
from .modeling_gemma3 import Gemma3ForCausalLM
1716
from .modeling_gemma3vl import Gemma3VLM
1817
from .modeling_glm import Glm4MoeForCausalLM
@@ -51,6 +50,11 @@
5150
from .modeling_utils import get_model_architecture
5251
from .modeling_vila import VilaModel
5352

53+
try:
54+
from .modeling_exaone_moe import ExaoneMoeForCausalLM
55+
except ImportError:
56+
ExaoneMoeForCausalLM = None
57+
5458
# Note: for better readiblity, this should have same order as imports above
5559
__all__ = [
5660
"AutoModelForCausalLM",
@@ -59,7 +63,6 @@
5963
"DeepseekV3ForCausalLM",
6064
"Exaone4ForCausalLM",
6165
"Exaone4_5_ForConditionalGeneration",
62-
"ExaoneMoeForCausalLM",
6366
"Gemma3ForCausalLM",
6467
"Gemma3VLM",
6568
"HCXVisionForCausalLM",
@@ -104,6 +107,9 @@
104107
"Step3p7VLForConditionalGeneration",
105108
]
106109

110+
if ExaoneMoeForCausalLM is not None:
111+
__all__.append("ExaoneMoeForCausalLM")
112+
107113
if transformers.__version__ >= "4.45.1":
108114
from .modeling_mllama import MllamaForConditionalGeneration # noqa
109115

tests/integration/defs/accuracy/test_disaggregated_serving.py

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -676,6 +676,49 @@ def test_kv_cache_v2_nixl_python(self):
676676
self.MODEL_PATH) as llm:
677677
run_accuracy_test(llm, self.MODEL_NAME, ["GSM8K"])
678678

679+
@skip_pre_hopper
680+
@pytest.mark.skip_less_device(2)
681+
@parametrize_with_ids("chunk_size_blocks", [64])
682+
@parametrize_with_ids("enable_block_reuse", [False, True])
683+
def test_chunked_kv_transfer_nixl_python_accuracy(self,
684+
chunk_size_blocks: int,
685+
enable_block_reuse: bool):
686+
"""Test chunked KV transfer accuracy using Python transceiver and C++ KVCacheManager."""
687+
kv_cache_config = {
688+
"use_kv_cache_manager_v2": False,
689+
"enable_block_reuse": enable_block_reuse,
690+
}
691+
cache_transceiver_config = {
692+
"backend": "NIXL",
693+
"transceiver_runtime": "PYTHON",
694+
"max_tokens_in_buffer": 4096,
695+
"chunk_size_blocks": chunk_size_blocks,
696+
}
697+
ctx_server_config = {
698+
"disable_overlap_scheduler": True,
699+
"kv_cache_config": dict(kv_cache_config),
700+
"cache_transceiver_config": dict(cache_transceiver_config),
701+
}
702+
gen_server_config = {
703+
"disable_overlap_scheduler": False,
704+
"kv_cache_config": dict(kv_cache_config),
705+
"cache_transceiver_config": dict(cache_transceiver_config),
706+
}
707+
disaggregated_server_config = {
708+
"hostname": "localhost",
709+
"backend": "pytorch",
710+
"context_servers": {
711+
"num_instances": 1,
712+
},
713+
"generation_servers": {
714+
"num_instances": 1,
715+
},
716+
}
717+
with launch_disaggregated_llm(disaggregated_server_config,
718+
ctx_server_config, gen_server_config,
719+
self.MODEL_PATH) as llm:
720+
run_accuracy_test(llm, self.MODEL_NAME, ["GSM8K"])
721+
679722
@pytest.mark.skip_less_device(2)
680723
def test_ngram(self):
681724
speculative_decoding_config = {

0 commit comments

Comments
 (0)