Repository navigation
feat: add EncoderCacheManager to TRT-LLM PrefillHandler - #5714
Conversation
e64d163 to
11bb4f3
Compare
WalkthroughThis PR introduces AsyncEncoderCache, an async wrapper around EncoderCacheManager that implements request coalescing to prevent duplicate concurrent computations for the same key. Configuration changes propagate encoder_cache_capacity_gb through RequestHandlerConfig, CLI arguments, and utility layers. PrefillHandler is updated to accept and utilize an encoder_cache parameter during initialization. Validation for negative capacity in EncoderCacheManager is removed. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ❌ 3❌ Failed checks (2 warnings, 1 inconclusive)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
11bb4f3 to
b53c553
Compare
|
Do we prefer to expose the absolute amount of bytes, or rather the VRAM fraction? Since this will interact with |
Thanks for asking! we're following convention in this PR --
e.g. |
b53c553 to
621c615
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@components/src/dynamo/common/multimodal/async_encoder_cache.py`:
- Around line 110-123: The in-flight Future created for coalescing can be left
unresolved if the compute_fn coroutine is cancelled; update the try/except to
explicitly handle asyncio.CancelledError: upon catching CancelledError, cancel
or set_exception on the future (e.g., call future.cancel() so waiters receive a
cancellation) and then re-raise the CancelledError. Modify the block around
compute_fn/ future (the future variable, self._in_flight[key], and the
try/except/finally) to include an "except asyncio.CancelledError:" clause that
cancels the future and re-raises, leaving the existing finally that deletes
self._in_flight[key] intact; keep the _suppress_unhandled_future_exception
callback behavior.
In `@components/src/dynamo/trtllm/utils/trtllm_utils.py`:
- Around line 289-294: Add a non-negative validation for the
"--encoder-cache-capacity-gb" CLI flag by either supplying a custom argparse
type (e.g., non_negative_float) to the parser.add_argument call for that flag or
by adding a post-parse check that calls parser.error when
args.encoder_cache_capacity_gb < 0; apply the same validation pattern to the
other cache/size flags in the 352-387 block so all capacity/size arguments (the
parser.add_argument entries for those flags) reject negative values with a clear
error message.
🧹 Nitpick comments (1)
components/src/dynamo/trtllm/tests/request_handlers/test_trtllm_prefill_handler.py (1)
40-44: Cover encoder_cache wiring in the init test.
Line 42 only asserts the engine; consider also asserting the encoder cache is stored to exercise the new constructor parameter.♻️ Suggested test enhancement
def test_init_with_encoder_cache(self, mock_config, mock_encoder_cache): """Test PrefillHandler can be initialized with encoder_cache.""" handler = PrefillHandler(mock_config, encoder_cache=mock_encoder_cache) assert handler.engine == mock_config.engine + assert handler._encoder_cache is mock_encoder_cache
621c615 to
a5d4d55
Compare
a5d4d55 to
27389be
Compare
27389be to
bf45fb2
Compare
bf45fb2 to
9785ec9
Compare
9785ec9 to
f194898
Compare
Just plug in as member variable.
Not use it yet.
Test Plan
running a few unit tests inside container --
Summary by CodeRabbit
Release Notes
New Features
--encoder-cache-capacity-gbcommand-line argument.Tests
✏️ Tip: You can customize this high-level summary in your review settings.