Skip to content

perf(multimodal): parallelize Qwen VL batch preprocessing with rayon - #927

Closed
slin1237 wants to merge 2 commits into
mainfrom
perf/qwen-vl-rayon-parallel
Closed

slin1237 wants to merge 2 commits into
mainfrom
perf/qwen-vl-rayon-parallel

Conversation

@slin1237

@slin1237 slin1237 commented Mar 26, 2026 •

Copy link
Copy Markdown
Member

Summary

  • Add rayon dependency to llm-multimodal crate
  • Refactor the sequential image loop in QwenVLProcessorBase::preprocess to use rayon::par_iter(), processing each image's resize/normalize/patchify pipeline in parallel
  • Results are collected and merged sequentially to preserve image order

Test plan

  • cargo test -p llm-multimodal -- all 135 unit tests pass
  • cargo test -p llm-multimodal -- vision_golden -- all 81 golden tests pass (bit-exact output preserved)

Summary by CodeRabbit

  • Refactor
    • Improved image preprocessing performance in the multimodal library by introducing parallel processing and more efficient per-image handling, reducing latency for multi-image workloads and improving throughput.

@slin1237
slin1237 requested a review from CatherineSue as a code owner March 26, 2026 18:13
@coderabbitai

coderabbitai Bot commented Mar 26, 2026 •

Copy link
Copy Markdown

Warning

Rate limit exceeded

@slin1237 has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 7 minutes and 25 seconds before requesting another review.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 611fdb88-bc14-4e49-8b10-ca1d02e50740

📥 Commits

Reviewing files that changed from the base of the PR and between ea9de7c and f46ebff.

📒 Files selected for processing (2)
  • crates/multimodal/Cargo.toml
  • crates/multimodal/src/vision/processors/qwen_vl_base.rs
📝 Walkthrough

Walkthrough

Refactored qwen_vl_base.rs::preprocess() to parallelize per-image processing with rayon when multiple images are provided; added rayon = "1.10" to crates/multimodal/Cargo.toml. Metadata and patch buffers are produced per-image and merged to preserve ordering.

Changes

Cohort / File(s) Summary
Dependency Addition
crates/multimodal/Cargo.toml
Added rayon = "1.10" to [dependencies].
Image Preprocessing Parallelization
crates/multimodal/src/vision/processors/qwen_vl_base.rs
Refactored preprocess() to use rayon::prelude::* and par_iter() when images.len() > 1; per-image resize/normalize/patchify is done in parallel into local buffers, then results (patches, num_patches, grid_thw, tokens) are merged sequentially; preserved original sequential path for single-image case and unified do_resize/do_normalize flags.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

Suggested labels

multimodal

Suggested reviewers

  • key4ng

Poem

🐰 I hopped through code where images lay,
Threads stitched patches, bright as day,
Per-image baskets, neat and true,
Merged in order — one-two-two,
Hooray for faster vision play! 🎨

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately and concisely describes the main change: adding parallel processing to Qwen VL batch preprocessing using rayon.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch perf/qwen-vl-rayon-parallel

Comment @coderabbitai help to get the list of available commands and usage tips.

@mergify

mergify Bot commented Mar 26, 2026

Copy link
Copy Markdown
Contributor

Hi @slin1237, the DCO sign-off check has failed. All commits must include a Signed-off-by line.

To fix existing commits:

# Sign off the last N commits (replace N with the number of unsigned commits)
git rebase HEAD~N --signoff
git push --force-with-lease

To sign off future commits automatically:

  • Use git commit -s every time, or
  • VSCode: enable Git: Always Sign Off in Settings
  • PyCharm: enable Sign-off commit in the Commit tool window

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 3a04edd14c

ℹ️ About Codex in GitHub

Codex has been enabled to automatically 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 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment on lines +377 to +381
.collect::<Result<Vec<_>, TransformError>>()?;

// Merge results sequentially to preserve image order
let total_patch_floats: usize = per_image_results.iter().map(|(p, _, _, _)| p.len()).sum();
let mut all_patches: Vec<f32> = Vec::with_capacity(total_patch_floats);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Avoid duplicating full patch buffers in preprocess

Collecting per_image_results stores every image's local_patches in memory at once, and then Vec::with_capacity(total_patch_floats) allocates a second full buffer before merge. For large batches/high-resolution images this roughly doubles peak memory versus the previous streaming append path and can trigger OOMs in production inference workers. This regression is introduced by the parallel refactor; consider a flattening strategy that consumes per-image vectors without preallocating a second full-sized buffer (or writes into final storage by offset).

Useful? React with 👍 / 👎.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces parallel image preprocessing in the QwenVLProcessorBase by integrating the rayon library. The implementation refactors the resizing, normalization, and patchification steps to execute in parallel, followed by a sequential aggregation phase to maintain the original image order. A recommendation was made to refactor the final result collection into a more functional fold operation to enhance code clarity and follow idiomatic Rust patterns.

Comment on lines 381 to 391
let mut all_patches: Vec<f32> = Vec::with_capacity(total_patch_floats);
let mut patches_per_image: Vec<i64> = Vec::with_capacity(images.len());
let mut grid_thw_data = Vec::with_capacity(images.len() * 3);
let mut num_img_tokens = Vec::with_capacity(images.len());

for image in images {
let (w, h) = image.dimensions();
let (target_h, target_w) = self.smart_resize(h as usize, w as usize)?;

// Resize to the image's own target size (skip if dimensions match)
let (tw32, th32) = (target_w as u32, target_h as u32);
let needs_resize = config.do_resize.unwrap_or(true) && (w != tw32 || h != th32);
let resized;
let img_ref = if needs_resize {
resized = resize(image, tw32, th32, filter);
&resized
} else {
image
};

// Grid dimensions based on the target size
let (grid_t, grid_h, grid_w) = self.calculate_grid_thw(target_h, target_w, 1);
grid_thw_data.push(grid_t as i64);
grid_thw_data.push(grid_h as i64);
grid_thw_data.push(grid_w as i64);

let num_patches = grid_t * grid_h * grid_w;
let tokens = self.calculate_tokens_from_grid(grid_t, grid_h, grid_w);
num_img_tokens.push(tokens);

// Convert to tensor [C, H, W] and normalize in one fused pass
let tensor = if config.do_normalize.unwrap_or(true) {
to_tensor_and_normalize(img_ref, &mean, &std)
} else {
to_tensor(img_ref)
};

// Patchify directly into all_patches to avoid intermediate Vec + copy
self.patchify_into(&tensor, grid_t, grid_h, grid_w, &mut all_patches)?;
for (local_patches, num_patches, grid_thw, tokens) in per_image_results {
all_patches.extend(local_patches);
patches_per_image.push(num_patches as i64);
grid_thw_data.extend_from_slice(&grid_thw);
num_img_tokens.push(tokens);
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

For improved code clarity and to follow a more functional style, you can replace the multiple mutable Vec declarations and the subsequent for loop with a single Iterator::fold operation. This consolidates the result aggregation logic into one expression, which can make the data flow easier to follow.

        let (all_patches, patches_per_image, grid_thw_data, num_img_tokens) =
            per_image_results.into_iter().fold(
                (
                    Vec::with_capacity(total_patch_floats),
                    Vec::with_capacity(images.len()),
                    Vec::with_capacity(images.len() * 3),
                    Vec::with_capacity(images.len()),
                ),
                |(mut all_patches, mut patches_per_image, mut grid_thw_data, mut num_img_tokens),
                 (local_patches, num_patches, grid_thw, tokens)| {
                    all_patches.extend(local_patches);
                    patches_per_image.push(num_patches as i64);
                    grid_thw_data.extend_from_slice(&grid_thw);
                    num_img_tokens.push(tokens);
                    (all_patches, patches_per_image, grid_thw_data, num_img_tokens)
                },
            );

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@crates/multimodal/src/vision/processors/qwen_vl_base.rs`:
- Around line 372-381: The parallel path currently creates and retains
per_image_results (holding local_patches) and then allocates all_patches and
copies every float again, doubling peak memory and incurring a full copy;
instead compute per-image patch counts/offsets first, preallocate a single
all_patches Vec<f32> of the total size, and have each parallel task call a
variant of patchify_into that writes directly into its disjoint slice of that
preallocated buffer (use per-image index/offset from the enumerated parallel
iterator); update or overload patchify_into (or add patchify_into_buffer) to
accept &mut [f32] plus the write offset so threads write in-place into
all_patches (preserving image order by using the same indexing used for
calculating offsets) and eliminate keeping local_patches/per_image_results.
- Around line 345-373: The code currently keeps img_ref at the original image
when do_resize is false even if smart_resize computed a different target size,
causing patchify_into/calculate_grid_thw to see inconsistent dimensions; update
the branch around needs_resize (and where img_ref is chosen) to detect the case
where !do_resize but (w != tw32 || h != th32) and immediately return a
recoverable error (use an existing TransformError variant or add one, e.g.,
TransformError::InvalidImageDimensions) instead of proceeding, and ensure any
places using expect/unreachable switch to ok_or(...) where appropriate to avoid
panics (refer to smart_resize, calculate_grid_thw, patchify_into,
to_tensor/to_tensor_and_normalize, and the do_resize flag).
🪄 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: bb5a19b7-e16f-4971-b172-16240cc0ddbe

📥 Commits

Reviewing files that changed from the base of the PR and between cb8407f and 3a04edd.

📒 Files selected for processing (2)
  • crates/multimodal/Cargo.toml
  • crates/multimodal/src/vision/processors/qwen_vl_base.rs

Comment on lines +345 to +373
let (w, h) = image.dimensions();
let (target_h, target_w) = self.smart_resize(h as usize, w as usize)?;

// Resize to the image's own target size (skip if dimensions match)
let (tw32, th32) = (target_w as u32, target_h as u32);
let needs_resize = do_resize && (w != tw32 || h != th32);
let resized;
let img_ref = if needs_resize {
resized = resize(image, tw32, th32, filter);
&resized
} else {
image
};

// Grid dimensions based on the target size
let (grid_t, grid_h, grid_w) = self.calculate_grid_thw(target_h, target_w, 1);
let num_patches = grid_t * grid_h * grid_w;
let tokens = self.calculate_tokens_from_grid(grid_t, grid_h, grid_w);

// Convert to tensor [C, H, W] and normalize in one fused pass
let tensor = if do_normalize {
to_tensor_and_normalize(img_ref, &mean, &std)
} else {
to_tensor(img_ref)
};

// Patchify into a local buffer
let mut local_patches = Vec::new();
self.patchify_into(&tensor, grid_t, grid_h, grid_w, &mut local_patches)?;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Return an error when do_resize is false but the image still needs resizing.

smart_resize() always computes target_h/target_w, but this branch keeps img_ref at the original size when resizing is disabled. The later calculate_grid_thw(target_*, ...) / patchify_into(...) path then sees inconsistent tensor and grid dimensions, which can silently crop oversized inputs and panic on undersized ones.

🛡️ Minimal fix
                 let (tw32, th32) = (target_w as u32, target_h as u32);
-                let needs_resize = do_resize && (w != tw32 || h != th32);
+                let needs_resize = w != tw32 || h != th32;
+                if !do_resize && needs_resize {
+                    return Err(TransformError::InvalidShape {
+                        expected: format!(
+                            "dimensions already match smart_resize output ({target_h}x{target_w}) when do_resize=false"
+                        ),
+                        actual: vec![h as usize, w as usize],
+                    });
+                }
                 let resized;
-                let img_ref = if needs_resize {
+                let img_ref = if do_resize && needs_resize {
                     resized = resize(image, tw32, th32, filter);
                     &resized
                 } else {
                     image
                 };
Based on learnings, prefer returning a recoverable error via ok_or(TransformError::EmptyBatch)? over using expect/unreachable, even when a prior non-empty check exists. This follows the repo’s “avoid panics in production code” guideline for PR `#489` and similar lint-only efforts.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@crates/multimodal/src/vision/processors/qwen_vl_base.rs` around lines 345 -
373, The code currently keeps img_ref at the original image when do_resize is
false even if smart_resize computed a different target size, causing
patchify_into/calculate_grid_thw to see inconsistent dimensions; update the
branch around needs_resize (and where img_ref is chosen) to detect the case
where !do_resize but (w != tw32 || h != th32) and immediately return a
recoverable error (use an existing TransformError variant or add one, e.g.,
TransformError::InvalidImageDimensions) instead of proceeding, and ensure any
places using expect/unreachable switch to ok_or(...) where appropriate to avoid
panics (refer to smart_resize, calculate_grid_thw, patchify_into,
to_tensor/to_tensor_and_normalize, and the do_resize flag).

Comment on lines +372 to +381
let mut local_patches = Vec::new();
self.patchify_into(&tensor, grid_t, grid_h, grid_w, &mut local_patches)?;

Ok((local_patches, num_patches, [grid_t as i64, grid_h as i64, grid_w as i64], tokens))
})
.sum();
let mut all_patches: Vec<f32> = Vec::with_capacity(estimated_total);
.collect::<Result<Vec<_>, TransformError>>()?;

// Merge results sequentially to preserve image order
let total_patch_floats: usize = per_image_results.iter().map(|(p, _, _, _)| p.len()).sum();
let mut all_patches: Vec<f32> = Vec::with_capacity(total_patch_floats);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

This parallel path now materializes the patch buffer twice.

Collecting per_image_results keeps every local_patches alive until the merge, and the merge then allocates all_patches and copies every float again. For max-sized inputs that is tens of MiB per image, so large batches now pay roughly 2× peak patch-buffer memory plus an extra full copy. Please consider precomputing per-image lengths/offsets and patchifying directly into disjoint slices of one preallocated output buffer instead.

Also applies to: 386-387

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@crates/multimodal/src/vision/processors/qwen_vl_base.rs` around lines 372 -
381, The parallel path currently creates and retains per_image_results (holding
local_patches) and then allocates all_patches and copies every float again,
doubling peak memory and incurring a full copy; instead compute per-image patch
counts/offsets first, preallocate a single all_patches Vec<f32> of the total
size, and have each parallel task call a variant of patchify_into that writes
directly into its disjoint slice of that preallocated buffer (use per-image
index/offset from the enumerated parallel iterator); update or overload
patchify_into (or add patchify_into_buffer) to accept &mut [f32] plus the write
offset so threads write in-place into all_patches (preserving image order by
using the same indexing used for calculating offsets) and eliminate keeping
local_patches/per_image_results.

@github-actions github-actions Bot added the dependencies Dependency updates label Mar 26, 2026

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@crates/multimodal/src/vision/processors/qwen_vl_base.rs`:
- Around line 413-420: The conditional in the sequential path that sets img_ref
(using do_resize, needs_resize, resized, resize()) has the same bug as the
parallel branch: when do_resize is false but the incoming image size differs
from smart_resize() output, downstream patchify_into() gets mismatched
tensor/grid sizes; update the sequential branch to compute the target size (as
smart_resize does), and always ensure the image is resized to that target when
dimensions differ (i.e., replace the current do_resize-only check with a
size-comparison check like in the parallel fix), so that img_ref passed to
patchify_into() is guaranteed to match the expected grid dimensions.
- Around line 341-443: The image-processing logic is duplicated between the
parallel branch and the sequential branch; extract it into a helper (e.g., fn
process_single_image(&self, image: &DynamicImage, do_resize: bool, do_normalize:
bool, filter: FilterType, mean: &[f64;3], std: &[f64;3]) -> Result<(Vec<f32>,
usize, [i64;3], usize), TransformError>) that calls self.smart_resize, applies
the do_resize check + resize, calls self.calculate_grid_thw,
self.calculate_tokens_from_grid, chooses to_tensor_or to_tensor_and_normalize,
and (optionally) invokes self.patchify_into or returns the patch buffer so
callers can decide zero-copy; then replace the duplicated blocks in the parallel
path (where per_image_results are built) and the sequential loop (where you
currently call self.patchify_into directly) to call this helper so behavior
(including the do_resize logic) is consistent.
🪄 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: f4c2d80f-a1fe-4e07-bfb4-5c5649be03fa

📥 Commits

Reviewing files that changed from the base of the PR and between 3a04edd and ea9de7c.

📒 Files selected for processing (1)
  • crates/multimodal/src/vision/processors/qwen_vl_base.rs

Comment on lines +341 to +443
let (all_patches, patches_per_image, grid_thw_data, num_img_tokens) = if images.len() > 1 {
// Parallel path: process each image independently, then merge
let per_image_results: Vec<_> = images
.par_iter()
.map(|image| {
let (w, h) = image.dimensions();
let (target_h, target_w) = self.smart_resize(h as usize, w as usize)?;

let (tw32, th32) = (target_w as u32, target_h as u32);
let needs_resize = do_resize && (w != tw32 || h != th32);
let resized;
let img_ref = if needs_resize {
resized = resize(image, tw32, th32, filter);
&resized
} else {
image
};

let (grid_t, grid_h, grid_w) = self.calculate_grid_thw(target_h, target_w, 1);
let num_patches = grid_t * grid_h * grid_w;
let tokens = self.calculate_tokens_from_grid(grid_t, grid_h, grid_w);

let tensor = if do_normalize {
to_tensor_and_normalize(img_ref, &mean, &std)
} else {
to_tensor(img_ref)
};

let mut local_patches = Vec::new();
self.patchify_into(&tensor, grid_t, grid_h, grid_w, &mut local_patches)?;

Ok((local_patches, num_patches, [grid_t as i64, grid_h as i64, grid_w as i64], tokens))
})
.collect::<Result<Vec<_>, TransformError>>()?;

let total_patch_floats: usize =
per_image_results.iter().map(|(p, _, _, _)| p.len()).sum();
let mut all_patches: Vec<f32> = Vec::with_capacity(total_patch_floats);
let mut patches_per_image: Vec<i64> = Vec::with_capacity(images.len());
let mut grid_thw_data = Vec::with_capacity(images.len() * 3);
let mut num_img_tokens = Vec::with_capacity(images.len());

for (local_patches, num_patches, grid_thw, tokens) in per_image_results {
all_patches.extend(local_patches);
patches_per_image.push(num_patches as i64);
grid_thw_data.extend_from_slice(&grid_thw);
num_img_tokens.push(tokens);
}

(all_patches, patches_per_image, grid_thw_data, num_img_tokens)
} else {
// Sequential path: patchify directly into output buffer (zero-copy)
let estimated_total: usize = images
.iter()
.map(|img| {
let (w, h) = img.dimensions();
(w as usize * h as usize)
/ (self.config.merge_size * self.config.merge_size)
* patch_features
/ (patch_size * patch_size)
})
.sum();
let mut all_patches: Vec<f32> = Vec::with_capacity(estimated_total);
let mut patches_per_image: Vec<i64> = Vec::with_capacity(images.len());
let mut grid_thw_data = Vec::with_capacity(images.len() * 3);
let mut num_img_tokens = Vec::with_capacity(images.len());

for image in images {
let (w, h) = image.dimensions();
let (target_h, target_w) = self.smart_resize(h as usize, w as usize)?;

let (tw32, th32) = (target_w as u32, target_h as u32);
let needs_resize = do_resize && (w != tw32 || h != th32);
let resized;
let img_ref = if needs_resize {
resized = resize(image, tw32, th32, filter);
&resized
} else {
image
};

let (grid_t, grid_h, grid_w) = self.calculate_grid_thw(target_h, target_w, 1);
grid_thw_data.push(grid_t as i64);
grid_thw_data.push(grid_h as i64);
grid_thw_data.push(grid_w as i64);

let num_patches = grid_t * grid_h * grid_w;
let tokens = self.calculate_tokens_from_grid(grid_t, grid_h, grid_w);
num_img_tokens.push(tokens);

let tensor = if do_normalize {
to_tensor_and_normalize(img_ref, &mean, &std)
} else {
to_tensor(img_ref)
};

// Patchify directly into all_patches to avoid intermediate Vec + copy
self.patchify_into(&tensor, grid_t, grid_h, grid_w, &mut all_patches)?;
patches_per_image.push(num_patches as i64);
}

(all_patches, patches_per_image, grid_thw_data, num_img_tokens)
};

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial

Consider extracting shared image-processing logic to reduce duplication.

The per-image processing logic (smart_resize → conditional resize → grid calculation → tensor conversion) is nearly identical between the parallel path (lines 346-372) and sequential path (lines 408-439). A helper method could reduce duplication and ensure fixes (like the do_resize issue) are applied consistently.

♻️ Sketch of a possible helper
fn process_single_image(
    &self,
    image: &DynamicImage,
    do_resize: bool,
    do_normalize: bool,
    filter: FilterType,
    mean: &[f64; 3],
    std: &[f64; 3],
) -> Result<(Array3<f32>, usize, usize, usize, [i64; 3], usize), TransformError> {
    let (w, h) = image.dimensions();
    let (target_h, target_w) = self.smart_resize(h as usize, w as usize)?;
    
    // ... resize/normalize/grid logic ...
    
    Ok((tensor, grid_t, grid_h, grid_w, grid_thw, tokens))
}

Then both paths call this helper—the parallel path collects and merges, while the sequential path patchifies directly.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@crates/multimodal/src/vision/processors/qwen_vl_base.rs` around lines 341 -
443, The image-processing logic is duplicated between the parallel branch and
the sequential branch; extract it into a helper (e.g., fn
process_single_image(&self, image: &DynamicImage, do_resize: bool, do_normalize:
bool, filter: FilterType, mean: &[f64;3], std: &[f64;3]) -> Result<(Vec<f32>,
usize, [i64;3], usize), TransformError>) that calls self.smart_resize, applies
the do_resize check + resize, calls self.calculate_grid_thw,
self.calculate_tokens_from_grid, chooses to_tensor_or to_tensor_and_normalize,
and (optionally) invokes self.patchify_into or returns the patch buffer so
callers can decide zero-copy; then replace the duplicated blocks in the parallel
path (where per_image_results are built) and the sequential loop (where you
currently call self.patchify_into directly) to call this helper so behavior
(including the do_resize logic) is consistent.

Comment on lines +413 to +420
let needs_resize = do_resize && (w != tw32 || h != th32);
let resized;
let img_ref = if needs_resize {
resized = resize(image, tw32, th32, filter);
&resized
} else {
image
};

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Same do_resize=false dimension mismatch applies here.

The sequential path has the identical logic issue flagged for the parallel path: when do_resize=false but the image dimensions don't match smart_resize() output, the tensor dimensions will be inconsistent with the grid passed to patchify_into().

When addressing the fix proposed in the earlier review comment, apply it to both paths.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@crates/multimodal/src/vision/processors/qwen_vl_base.rs` around lines 413 -
420, The conditional in the sequential path that sets img_ref (using do_resize,
needs_resize, resized, resize()) has the same bug as the parallel branch: when
do_resize is false but the incoming image size differs from smart_resize()
output, downstream patchify_into() gets mismatched tensor/grid sizes; update the
sequential branch to compute the target size (as smart_resize does), and always
ensure the image is resized to that target when dimensions differ (i.e., replace
the current do_resize-only check with a size-comparison check like in the
parallel fix), so that img_ref passed to patchify_into() is guaranteed to match
the expected grid dimensions.

Signed-off-by: Simo Lin <linsimo.mark@gmail.com>
…d thread pool overhead

Signed-off-by: Simo Lin <linsimo.mark@gmail.com>
@slin1237
slin1237 force-pushed the perf/qwen-vl-rayon-parallel branch from ea9de7c to f46ebff Compare March 26, 2026 20:51
@slin1237

Copy link
Copy Markdown
Member Author

Benchmark Results

Baseline (main) vs this PR (with batch-size threshold fix):

Benchmark Baseline This PR Delta
qwen3_vl 224x224 (single) 423.33 µs 397 µs -6.2%
qwen3_vl 640x480 (single) 1.564 ms 1.47 ms -6.0%
qwen3_vl 1024x768 (single) 4.198 ms 3.88 ms -7.6%
qwen3_vl 1920x1080 (single) 31.83 ms 31.7 ms -0.4%
qwen3_vl batch3 18.88 ms 4.93 ms -73.9%
qwen3_vl batch5 28.66 ms 6.94 ms -75.8%
qwen3_vl batch10 61.57 ms 57.8 ms -6.1%

Single images use the original zero-copy sequential path (no rayon overhead). Batches >1 use par_iter() for parallel processing. Batch3/5 see ~74-76% improvement from parallelization across CPU cores.

Note: Initial implementation (without threshold) caused 5.3x regression on single images. Fixed by gating par_iter on images.len() > 1.

@slin1237

Copy link
Copy Markdown
Member Author

MMMU Validation — Qwen3-VL-8B

Baseline Rayon PR Delta
MMMU accuracy 51.33% 51.67% +0.34pp (noise)
Total elapsed time 531.6s 564.7s +6.2%
Avg speed 49.5 tok/s 46.6 tok/s -5.9%

No accuracy degradation. The slight latency increase is expected since MMMU sends single images sequentially (rayon parallelism only activates for batch > 1). The real-world benefit is in multi-image batch scenarios where batch3/5 see 74-76% speedup.

@slin1237 slin1237 closed this Mar 26, 2026
@slin1237
slin1237 deleted the perf/qwen-vl-rayon-parallel branch March 26, 2026 22:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

dependencies Dependency updates

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant