Skip to content

perf(postprocess): read every image's mask target size once per batch - #1369

Merged
Borda merged 3 commits into
roboflow:developfrom
JESUSROYETH:perf/postprocess-prefetch-target-sizes-20260818
Aug 18, 2026
Merged

perf(postprocess): read every image's mask target size once per batch#1369
Borda merged 3 commits into
roboflow:developfrom
JESUSROYETH:perf/postprocess-prefetch-target-sizes-20260818

Conversation

@JESUSROYETH

Copy link
Copy Markdown
Contributor

PostProcess._postprocess_masks calls target_sizes[i].tolist() inside the per-image loop, but only on the path where upsample_masks_to_image_size=True. When target_sizes is on CUDA, .tolist() forces a device-to-host sync, so a batch of B images pays B separate syncs in the middle of the masks work instead of one.

This reads the whole [B, 2] tensor once, before the loop, and only when it's actually going to be used (upsampling requested and the batch isn't empty), then indexes the resulting host-side list per image:

# before
h, w = target_sizes[i].tolist()

# after (read once, outside the loop)
target_sizes_list = target_sizes.tolist() if upsample_masks_to_image_size and out_masks.shape[0] else []
...
h, w = target_sizes_list[i]

The native-resolution path and the empty-batch path already skipped this read and still do — I added a test that checks that directly instead of trusting it by inspection.

COCOEvalCallback._convert_targets (training/callbacks/coco_eval.py) has the identical shape: h, w = t["orig_size"].tolist() inside a per-target loop over the batch's GT dicts. It runs on every validation batch during training (and every training batch too, when compute_train_metrics=True), so it pays the same per-image sync — on a hotter path than inference-only mask postprocessing. Same fix: stack every target's orig_size once before the loop, following the pattern this file's EMA branch already uses a few lines above it (torch.stack([t["orig_size"] for t in outputs["targets"]])). I didn't re-run today's perf benchmark for this path — it's a Lightning callback, not the standalone microbenchmark harness _postprocess_masks has — so this is the same correctness/sync-removal change, not a separately measured number.

On an RTX 4060, postprocess-only (101 rounds, 3 separate processes, score_threshold=0.5 matching predict()'s default so the per-image nonzero() sync from score filtering runs in both baseline and candidate): 6.49–7.66% median reduction at batch 8, 9.69–10.31% at batch 16, gap growing with batch, which is what you'd expect from removing B-1 mid-loop syncs. I also ran a CPU-only control over the same 101 rounds and it shows overlapping ranges with two sign flips out of six, so this isn't just fewer Python list allocations — it tracks the sync removal.

I have to be honest about end-to-end: full forward + postprocess on this same GPU came back neutral, −0.10% to 0.53% at batch 8 and −0.03% to 0.22% at batch 16, inside noise. I'm not claiming an end-to-end win here — the postprocess-only number is the real claim, and on a smaller/laptop GPU the sync cost is apparently not the bottleneck end-to-end. It might be a different story on a GPU where host syncs cost more relative to the forward pass, but I didn't measure that today so I won't assert it.

Equivalence: ran the baseline twice against itself and once against the candidate on 16 real COCO val2017 images, boxes matched by IoU + assignment. Boxes, scores, labels, and masks came back byte-identical across all three runs.

Changes

  • postprocess.py: prefetch target_sizes.tolist() once per batch instead of once per image.
  • training/callbacks/coco_eval.py: same prefetch for _convert_targets's per-target orig_size.tolist().
  • test_postprocess.py: new parametrized test asserting the exact tolist() call shapes for the upsampled, native-resolution, and empty-batch cases, plus a @pytest.mark.gpu variant that runs the same call-count assertion with every input tensor actually on CUDA (the real site of the device-to-host sync). Confirmed it fails first against the current code (1 failed, wrong call shapes), then passes with the fix (25 passed).
  • test_coco_eval_callback.py: same shape of test for _convert_targets, parametrized over a multi-target batch and an empty one, plus its own @pytest.mark.gpu CUDA variant. Also confirmed failing first (assert [(2,), (2,), (2,)] == [(3, 2)]), then passing.
  • CHANGELOG.md: added a ### Changed entry under [Unreleased].

Validation

  • Both targeted test files: 154 passed (25 in test_postprocess.py, 129 in test_coco_eval_callback.py).
  • pre-commit run --all-files: all 19 hooks clean, including mypy --strict on both touched modules.
  • Full uv run --no-sync pytest src/ tests/ -n 2 -m "not gpu and not coco17 and not e2e_coreml and not e2e_executorch and not e2e_roboflow and not xla and not tpu" --ignore=tests/run_smoke_all_models.py --ignore=tests/legacy/test_checkpoint_compat.py --timeout=420 (the exact command ci-tests-cpu.yml runs): 3981 passed, 78 skipped, 0 failed.
  • The -m gpu tests in both touched test files (real RTX 4060): 4 passed — the 2 pre-existing ones plus the 2 new CUDA-specific regression tests added by this PR.
  • Patch applies cleanly on current develop.

I checked for an existing PR on this — only hit was #921 (OBB support), which still has the per-image read and doesn't touch this. This is independent of #1367, which is training-time (criterion.py), not this postprocess path.

Small, narrow perf change, but flagged as real vs. neutral where it actually is .. hope this helps!

@codecov

codecov Bot commented Aug 18, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 85%. Comparing base (701e7b3) to head (0092221).

Additional details and impacted files
@@           Coverage Diff           @@
##           develop   #1369   +/-   ##
=======================================
- Coverage       86%     85%   -1%     
=======================================
  Files          111     111           
  Lines        14290   14292    +2     
=======================================
- Hits         12261   12183   -78     
- Misses        2029    2109   +80     
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@Borda
Borda requested a balanced review from Copilot August 18, 2026 20:16
@Borda Borda added the enhancement New feature or request label Aug 18, 2026

Copilot AI 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.

Pull request overview

Optimizes CUDA mask postprocessing and COCO target conversion by consolidating per-image device-to-host reads into one per batch.

Changes:

  • Batch-converts mask target sizes and COCO original sizes.
  • Adds CPU, empty-batch, native-resolution, and CUDA regression tests.
  • Documents the performance change.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.

Show a summary per file
File Description
src/rfdetr/models/postprocess.py Reads all mask resize targets once per batch.
src/rfdetr/training/callbacks/coco_eval.py Batches orig_size conversion during evaluation.
tests/models/test_postprocess.py Tests synchronization-sensitive conversion behavior.
tests/training/callbacks/test_coco_eval_callback.py Tests batched target-size reads.
CHANGELOG.md Records the optimization.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Borda and others added 2 commits August 18, 2026 23:02
Changes:
- Add heterogeneous non-square mask target sizes to postprocess regression coverage, asserting each image keeps its own output shape and interpolated mask geometry after batched conversion.
- Add heterogeneous COCO orig_size coverage, asserting per-image labels and exact absolute box coordinates remain paired with the correct target row.

Impact:
- Prevents future row-order and H/W-transposition regressions in the batched device-to-host metadata optimization.
- Keeps the production implementation unchanged while making the synchronization optimization's positional contract executable.

Verification:
- uv run --no-sync pytest tests/models/test_postprocess.py tests/training/callbacks/test_coco_eval_callback.py -n 1 -m "not gpu" --timeout=240 — 152 passed.
- uv run --no-sync ruff check ... — passed.
- uv run --no-sync ruff format --check ... — passed.
- uv run --no-sync mypy --strict src/rfdetr/models/postprocess.py src/rfdetr/training/callbacks/coco_eval.py — passed.
- pre-commit run --all-files — passed.
- git diff --check — passed.

Residual limits:
- CUDA tests were skipped on the CPU-only host.
- Full CPU suite had six unrelated CoreML/ONNX/environment failures, including missing onnxscript.
- Benchmark reproducibility and HIGH_RISK specialist provenance remain deferred in the remediation report.

---

Co-authored-by: Codex <codex@openai.com>
@Borda
Borda merged commit 48d6777 into roboflow:develop Aug 18, 2026
41 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants