[perf] Quantize frames to uint8 on-device before the post-decode D->H copy#1362
[perf] Quantize frames to uint8 on-device before the post-decode D->H copy#1362Mister-Raggs wants to merge 3 commits into
Conversation
Merge Protections🔴 1 of 1 protections blocking · waiting on 👀 reviews and 🤖 CI
🔴 PR merge requirementsWaiting for
This rule is failing.
|
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
This PR optimizes decoded-frame postprocessing by moving quantization/casting to uint8 onto the source device (typically CUDA) before transferring frames to CPU, while also clamping values to avoid wraparound artifacts.
Changes:
- Quantize/clamp video tensor on-device before CPU transfer to reduce transfer size and CPU work.
- Replace per-frame CPU scaling/casting loop with a list-comprehension over a pre-quantized tensor.
- Add explanatory comments about the performance rationale and overflow behavior.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| # smaller and moves the elementwise work onto the GPU. | ||
| # clamp_() also fixes a latent overflow bug: VAE output slightly | ||
| # outside [0, 1] wrapped mod 256 in the old unclamped cast. | ||
| src = output_batch.output |
| # outside [0, 1] wrapped mod 256 in the old unclamped cast. | ||
| src = output_batch.output | ||
| vid_u8 = (src * 255).clamp_(0, 255).to(torch.uint8) | ||
| vid_u8 = rearrange(vid_u8, "b c t h w -> t b c h w").cpu() |
| # the first op here blocked on the full fp32 video transfer | ||
| # (~0.3-1.4 GB) and ran a single-threaded per-frame CPU *255/cast | ||
| # loop. Casting to uint8 on-device first makes the transfer 4x | ||
| # smaller and moves the elementwise work onto the GPU. | ||
| # clamp_() also fixes a latent overflow bug: VAE output slightly | ||
| # outside [0, 1] wrapped mod 256 in the old unclamped cast. |
There was a problem hiding this comment.
Code Review
This pull request optimizes video post-processing by performing quantization and clamping on the GPU before transferring data to the CPU, which reduces the transfer size and fixes a potential overflow bug. Feedback suggests further improving performance by moving the grid layout and permutation operations to the GPU and using a single stacked transfer to minimize CPU bottlenecks.
| vid_u8 = rearrange(vid_u8, "b c t h w -> t b c h w").cpu() | ||
| frames = [ | ||
| torchvision.utils.make_grid(x, nrow=6).permute( | ||
| 1, 2, 0).squeeze(-1).contiguous().numpy() | ||
| for x in vid_u8 | ||
| ] |
There was a problem hiding this comment.
While quantizing on-device significantly reduces the transfer size, the subsequent make_grid and permute operations are still being performed on the CPU within a single-threaded loop. Since the tensors are already on the device (typically CUDA), moving these operations to the GPU and then performing a single stacked transfer can further reduce the bottleneck in the PostDecodeFrameProcessStage.
| vid_u8 = rearrange(vid_u8, "b c t h w -> t b c h w").cpu() | |
| frames = [ | |
| torchvision.utils.make_grid(x, nrow=6).permute( | |
| 1, 2, 0).squeeze(-1).contiguous().numpy() | |
| for x in vid_u8 | |
| ] | |
| vid_u8 = rearrange(vid_u8, "b c t h w -> t b c h w") | |
| # Perform grid layout and permutation on device to avoid CPU bottlenecks. | |
| # Stacking before .cpu() ensures a single efficient D->H transfer. | |
| grids = torch.stack([ | |
| torchvision.utils.make_grid(x, nrow=6).permute(1, 2, 0).squeeze(-1) | |
| for x in vid_u8 | |
| ]) | |
| frames = [f for f in grids.cpu().numpy()] |
…ed transfer size Address review comments on hao-ai-lab#1362: - Document that `samples` is just the pinned-CPU mirror of `output_batch.output` with no intervening preprocessing, so sourcing from `output_batch.output` is the same data (Copilot). - Replace the hardcoded ~0.3-1.4 GB estimate with a description of the scaling relationship; it varies with resolution/frames/batch/dtype and would rot (Copilot). - Note the SSIM-gated (not bit-exact) equivalence inline. No behavior change.
|
@Mister-Raggs Thanks for the patch! A/B results — Wan2.1-T2V-1.3B, H100×1, 720×1280×77f, 30 steps
Per-run PostDecode very stable: baseline 0.568 / 0.592 s, treatment 0.296 / 0.285 s. |
|
This PR has merge conflicts with the base branch. Please rebase: git fetch origin main
git rebase origin/main
# Resolve any conflicts, then:
git push --force-with-lease |
PostDecodeFrameProcessStage was charged ~6s/run (~25% of e2e on Cosmos 2.5, scaling with frames/resolution). Profiling (nsys + microbench) showed the stage's own compute is only ~0.5-1.9s; the rest is the non-blocking pinned-CPU samples.copy_(output) D->H of the full fp32 video (~0.3-1.4 GB) completing lazily and blocking the first postprocess op, plus a single-threaded per-frame CPU *255/cast loop. Cast to uint8 on the source device (typically CUDA) before the copy: the transfer becomes 4x smaller (fp32 -> uint8) and the elementwise work runs on the GPU. Microbench: 2.056s -> 0.034s (T=29), 2.304s -> 0.132s (T=125), 17-60x on the measurable cost. clamp_(0, 255) additionally fixes a latent overflow: VAE output slightly outside [0, 1] previously wrapped mod 256 in the unclamped (x * 255).to(uint8) cast. Output is not bit-identical to the old CPU cast (float->uint8 differs <=1 LSB between CPU and GPU on boundary pixels); gate via SSIM rather than exact equality. Scoped to the pixel-video path only; latent, audio-only, and return-samples paths are unchanged.
…ed transfer size Address review comments on hao-ai-lab#1362: - Document that `samples` is just the pinned-CPU mirror of `output_batch.output` with no intervening preprocessing, so sourcing from `output_batch.output` is the same data (Copilot). - Replace the hardcoded ~0.3-1.4 GB estimate with a description of the scaling relationship; it varies with resolution/frames/batch/dtype and would rot (Copilot). - Note the SSIM-gated (not bit-exact) equivalence inline. No behavior change.
The previous commit rebuilt the post-decode frames path to read `output_batch.output` directly via the GPU `vid_u8` cast, so `samples` is now consumed in exactly one place — the result dict's `samples` field, gated on `batch.return_frames`. When the caller doesn't ask for `samples`, the pinned ~50 MB fp32 alloc and its D->H copy (and the latent fallthrough `.cpu()`) are dead weight; the typical generate flow (save_video=True, return_frames=False) hits this on every call. Extend `skip_pixel_prealloc` to include `not return_frames` so the pinned buffer is allocated only when needed, and short-circuit the copy/`.cpu()` on the same condition. No effect when `return_frames=True` or for latent callers that read `samples`; correctness is unchanged (SSIM-gated, same as the parent commit). Removes the residual ~2 s the PR text already calls out for the "only saving to disk" case. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
c709711 to
3c3da4d
Compare
Summary
PostDecodeFrameProcessStagespends most of its time on a deferred fp32 device→host transfer mischarged to it, plus a single-threaded per-frame CPU*255/cast loop.Fix: cast the decoded video to
uint8on the source device (typically CUDA) before the host copy. The transfer becomes 4× smaller (fp32 → uint8) and the elementwise work runs on the GPU instead of a Python per-frame CPU loop.Mechanism proven and independently cross-validated. The per-path speedup (~−50–66%) is consistent; e2e impact scales with how much of total latency the post-decode path owns on a given model / resolution / GPU:
The post-decode path is ~25% of e2e on Cosmos/A100 but only ~0.3% on Wan/H100, so the same per-path halving lands very differently on e2e. Net-positive everywhere; magnitude is configuration-dependent. VAE and denoising stages are byte-untouched in both runs — the change is correctly scoped to the pixel path. Stacks with
enable_torch_compile(independently ~−14%; different pipeline stages).Root cause
samplesis a pinned-CPU buffer;samples.copy_(output_batch.output)is non-blocking, andgen_timeis logged immediately after it. The full fp32 video D→H (scales with resolution × frames × batch and can be large) therefore completes lazily and the first op in the post-decode block blocks on it — so a large transfer is attributed toPostDecodeFrameProcessStage. On top of that, the old code ran(x * 255).to(uint8)in a Python loop over frames on the CPU tensor.Evidence:
--trace-fork-before-exec): fragmented CPU tail + transfer, GPU idle during the stage.Correctness
Not bit-identical to the previous CPU cast:
float → uint8can differ by ≤1 LSB between CPU and GPU on boundary pixels, andclamp_(0, 255)additionally fixes a latent overflow (VAE output slightly outside[0, 1]previously wrapped mod 256 in the unclamped cast). Gated by SSIM rather than exact equality:MS-SSIM (repo
compute_video_ssim_torchvision, Cosmos 2.5, A100): 29/29 frames = 1.000000 exactly (verified all frames read and compared, mean = min = max = 1.0).Skip the dead fp32
samplescopy whenreturn_frames=False(commit 2)After commit 1, the post-decode frames path reads
output_batch.outputdirectly via the GPUvid_u8cast, sosamplesis consumed in exactly one place — the result dict'ssamplesfield, gated onbatch.return_frames("samples": samples if batch.return_frames else None). When the caller doesn't ask forsamples, the pinned ~50 MB fp32 alloc and its D→H copy (and the latent fallthrough.cpu()) are dead weight; the typical generate flow (save_video=True, return_frames=False) hits this on every call.Commit 2 extends
skip_pixel_preallocto includenot batch.return_framesand short-circuits the copy on the same condition — removing the residual ~2 s the PR text originally called out as a follow-up. Behavior is unchanged for callers withreturn_frames=True(they still receive the fp32samplestensor) or for the latent fallthrough whenreturn_frames=True.Further device-residency improvement — running
make_grid/permuteon-device with a single stacked transfer — is still a candidate follow-up, measured and SSIM-gated separately.Scope
Pixel-video path only.
output_type == "latent"and audio-only paths are byte-unchanged. Callers passingreturn_frames=Truestill receive the fp32samplestensor as before; commit 2 only skips work when nothing reads it.Test plan
return_frames=False): BASE (copy runs) vs CAND (copy skipped) = MS-SSIM mean/min/max = 1.000000 across all frames — bit-identical, confirming the skip is output-neutral