Repository navigation
Conversation
Signed-off-by: yechank-nvidia <161688079+yechank-nvidia@users.noreply.github.com>
Signed-off-by: yechank-nvidia <161688079+yechank-nvidia@users.noreply.github.com>
Signed-off-by: yechank-nvidia <161688079+yechank-nvidia@users.noreply.github.com>
github-actions
Bot
added
dependencies
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThis PR makes multimodal tensor handling view-based, moves TokenSpeed assembly onto async/blocking execution, and replaces SHM writes with fixed-size memory-mapped buffers. It also updates request-building call sites and adds coverage for the new serialization and cleanup paths. ChangesZero-copy tensor and SHM serialization
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
Suggested labels: Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
![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 refactors the serialization of multimodal encoder inputs and shared memory (SHM) writing to use memory-mapped files (memmap2) and parallelized conversions (rayon), avoiding unnecessary allocations and copying. Specifically, write_tokenspeed_shm_with now maps the SHM file directly, and on Linux, it pre-allocates space using rustix::fs::fallocate. The review feedback recommends adding a fallback to file.set_len when fallocate is unsupported (e.g., in certain containerized or virtualized environments) to prevent hard failures.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| #[cfg(target_os = "linux")] | ||
| fn reserve_tokenspeed_shm_file(file: &std::fs::File, nbytes: usize) -> std::io::Result<()> { | ||
| Ok(rustix::fs::fallocate( | ||
| file, | ||
| FallocateFlags::empty(), | ||
| 0, | ||
| nbytes as u64, | ||
| )?) | ||
| } |
There was a problem hiding this comment.
On Linux, rustix::fs::fallocate can fail with EOPNOTSUPP (if the underlying filesystem, such as certain container mounts or virtualized environments like WSL, does not support allocation) or ENOSYS (on older kernels). To prevent hard failures in these environments, fall back to file.set_len if fallocate returns either of these errors.
| #[cfg(target_os = "linux")] | |
| fn reserve_tokenspeed_shm_file(file: &std::fs::File, nbytes: usize) -> std::io::Result<()> { | |
| Ok(rustix::fs::fallocate( | |
| file, | |
| FallocateFlags::empty(), | |
| 0, | |
| nbytes as u64, | |
| )?) | |
| } | |
| #[cfg(target_os = "linux")] | |
| fn reserve_tokenspeed_shm_file(file: &std::fs::File, nbytes: usize) -> std::io::Result<()> { | |
| match rustix::fs::fallocate(file, FallocateFlags::empty(), 0, nbytes as u64) { | |
| Ok(()) => Ok(()), | |
| Err(err) if err == rustix::io::Errno::OPNOTSUPP || err == rustix::io::Errno::NOSYS => { | |
| file.set_len(nbytes as u64) | |
| } | |
| Err(err) => Err(err.into()), | |
| } | |
| } |
There was a problem hiding this comment.
@coderabbitat Check if addressed
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@model_gateway/src/routers/grpc/multimodal.rs`:
- Around line 1551-1581: Offload the TokenSpeed assembly work from the async
request-building path, since assemble_multimodal_data() is still being invoked
inline from the execute() methods in chat/request_building.rs and
messages/request_building.rs and can block a Tokio worker. Move the branch that
calls fill_f32_slice_as_u16_bytes into spawn_blocking or a dedicated CPU-bound
executor, keeping the async flow responsive while the rayon-backed serialization
runs. Use assemble_multimodal_data() and fill_f32_slice_as_u16_bytes() as the
main touchpoints when wiring the offload.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 681aca0c-f3b1-40dd-b544-cdd2e4103445
📒 Files selected for processing (3)
model_gateway/Cargo.tomlmodel_gateway/src/routers/grpc/multimodal.rsmodel_gateway/src/routers/grpc/proto_wrapper.rs
yechank-nvidia
changed the title
Signed-off-by: yechank-nvidia <161688079+yechank-nvidia@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@model_gateway/src/routers/grpc/multimodal.rs`:
- Around line 971-979: Move the TokenSpeed option resolution into the blocking
work so the `/dev/shm` probe is not done on the async path. Update the
GrpcClient::TokenSpeed branch in multimodal.rs to compute
tokenspeed_assembly_options inside the spawn_blocking closure together with
assemble_tokenspeed_with_options, and keep the
resolve_tokenspeed_shm_enabled/tokenspeed_shm_dev_writable call chain off the
runtime thread.
In `@model_gateway/src/routers/grpc/proto_wrapper.rs`:
- Around line 555-563: The fallocate handling in the proto wrapper should retry
interrupted syscalls instead of immediately falling back or returning an error.
Update the logic around the `rustix::fs::fallocate` call in `proto_wrapper.rs`
to use `rustix::io::retry_on_intr` (or equivalent retry handling) for
`Errno::INTR`, while keeping the existing `OPNOTSUPP` and `NOSYS` fallback to
`file.set_len` and preserving the current error conversion path for other
failures.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: c4f5fc04-0680-4802-922d-80d96c1dd7bf
📒 Files selected for processing (4)
model_gateway/src/routers/grpc/multimodal.rsmodel_gateway/src/routers/grpc/proto_wrapper.rsmodel_gateway/src/routers/grpc/regular/stages/chat/request_building.rsmodel_gateway/src/routers/grpc/regular/stages/messages/request_building.rs
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6f5d9d8bbc
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Signed-off-by: yechank-nvidia <161688079+yechank-nvidia@users.noreply.github.com>
3 participants
Description
Problem
Qwen image and video preprocessing spent substantial CPU time on repeated buffer materialization, resize setup, pixel indexing, and serial frame processing. The overhead became more pronounced for videos and concurrent requests.
Solution
Optimize the existing Qwen preprocessing path while preserving its public API, output layout, and preprocessing semantics.
The implementation reuses a shared worker pool, parallelizes independent temporal groups, specializes PIL-compatible RGB resize kernels, and reduces indexing and allocation overhead in resize and patchification.
Changes
Performance
Test Plan
cargo +nightly fmt --check
cargo test -p llm-multimodal qwen
cargo test -p llm-multimodal --test qwen_preprocess_golden
cargo clippy -p llm-multimodal --all-targets -- -D warnings
Checklist
cargo +nightly fmtpassescargo clippy --all-targets --all-features -- -D warningspassesSummary by CodeRabbit