Skip mm feature pool init to avoid EPD OOM#16388
Skip mm feature pool init to avoid EPD OOM#16388ShangmingCai merged 19 commits intosgl-project:mainfrom
Conversation
liusy58
requested review from
ByronHsu,
JustinTong0323,
ShangmingCai,
hnyls2002,
mickqian and
yhyang201
as code owners
Summary of ChangesHello @liusy58, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request addresses an Out-Of-Memory (OOM) issue, likely within the Encode Processor Disaggregation (EPD) component, by refining the conditions under which multimodal feature memory pools are initialized. The changes ensure that CUDA IPC-specific memory allocations only occur when the Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request aims to fix an Out-Of-Memory (OOM) issue in the Encoder-Prefill-Decoder (EPD) disaggregation setup by disabling CUDA IPC transport for the multimodal processor in the MMReceiver. This is achieved by hardcoding the transport_mode to "default" and making the CUDA IPC feature initialization conditional on the transport_mode.
While the changes are in the right direction, they are incomplete and introduce a critical bug. The logic for using CUDA IPC features in BaseMultimodalProcessor is not consistently updated. There are other code paths that still rely solely on the SGL_USE_CUDA_IPC flag, which will lead to an AttributeError at runtime if SGL_USE_CUDA_IPC is enabled but the transport_mode is not cuda_ipc. I've added a detailed comment on how to fix this.
|
/tag-run-ci-label |
@liusy58 Frequent pushes will trigger CI cooldown, and you need to wait an hour before rerunning CI. |
liusy58
requested review from
Ying1123,
merrymercy and
xiezhq-hermann
as code owners
|
/rerun-failed-ci |
|
Good Fix! LGTM. |
|
/rerun-failed-ci |
2 similar comments
|
/rerun-failed-ci |
|
/rerun-failed-ci |
|
It looks like this PR Test / unit-test-backend-8-gpu-h200 (3) (pull_request) is unable to finish within 20 minutes? |
|
/rerun-failed-ci |
We moved one test from per commit test to nightly so it should work now |
|
/rerun-failed-ci |
1 similar comment
|
/rerun-failed-ci |
|
/rerun-failed-ci |
|
/tag-and-rerun-ci |
|
/rerun-failed-ci |
|
/rerun-failed-ci |
1 similar comment
|
/rerun-failed-ci |
CI has passed.
|
…ial backport Applied changes to: - base_processor.py: Add skip_mm_pool parameter support - multimodal_processor.py: Add **kwargs to pass skip_mm_pool through Note: encode_receiver.py changes not applied due to file structure differences
7 participants
Motivation
The BaseMultimodalProcessor currently initializes a large
cudaipc_mmfeature_poolwhenever the environment variableSGLANG_USE_CUDA_IPC_TRANSPORTis set, regardless of whether that specific instance actually needs to perform IPC transfers.Since
MMReceiveronly handles orchestration and metadata and does not participate in the actual GPU-to-GPU tensor transport, this allocation is entirely unused and wasted.Modifications
Accuracy Tests
Benchmarking and Profiling
Checklist
Review Process
/tag-run-ci-label,/rerun-failed-ci,/tag-and-rerun-ci) or contact authorized users to do so.