perf(training): pack per-sample targets across the DataLoader boundary - #1399
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #1399 +/- ##
========================================
Coverage 86% 86%
========================================
Files 112 113 +1
Lines 14671 14791 +120
========================================
+ Hits 12629 12748 +119
- Misses 2042 2043 +1 🚀 New features to boost your workflow:
|
Could a good follow-up to try in two separate PRs - segm & kp |
There was a problem hiding this comment.
Pull request overview
Adds opt-in target packing to reduce DataLoader IPC overhead while preserving downstream target semantics.
Changes:
- Adds
PackedTargets, lossless packing, pinning, and device transfer. - Wires packing through
TrainConfigandRFDETRDataModule. - Adds tests and user documentation.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
src/rfdetr/utilities/tensors.py |
Implements target packing and materialization. |
src/rfdetr/utilities/__init__.py |
Exports the new public utilities. |
src/rfdetr/training/module_data.py |
Integrates packing into DataLoaders and transfers. |
src/rfdetr/config.py |
Adds the opt-in configuration field. |
tests/utilities/test_tensors.py |
Tests packing behavior and fidelity. |
tests/training/test_module_data.py |
Tests DataModule integration. |
docs/learn/train/training-parameters.md |
Documents the configuration option. |
docs/learn/train/customization.md |
Documents transfer-hook behavior. |
CHANGELOG.md |
Records the feature. |
Suppressed comments (1)
docs/learn/train/training-parameters.md:358
- This statement mentions only
train_dataloader(), but the configured collate function is also passed to validation, test, and predict loaders. Update the complete reference so callers know that enabling this option changes the direct output type of everyRFDETRDataModuleloader.
| `pack_targets` | bool | False | Concatenate each batch's target dicts into one tensor per field before crossing the DataLoader worker boundary. `train_dataloader()` yields `PackedTargets` instead of a tuple of dicts whenever a batch packs losslessly. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
I think we already have preferable use of Kornia on the GPU, so if we provide meaningful gain, I would be fine to make a bonus for GPU augmentation, but if the user, for whatever reason, insists on using Albumentations, we won't use it... |
This may be addressed by #1391 |
Changes: - Reject gradient-tracked or mixed-device target fields before concatenation, preserving the original tuple fallback instead of creating an unserializable tensor or raising from `torch.cat`. - Enable target packing by default across train, validation, test, and predict loaders while preserving the explicit `pack_targets=False` opt-out and plain-list targets after device transfer. - Add identity regressions and a real one-worker DataLoader traversal for packed reconstruction and gradient-bearing fallback; annotate the touched test helper. - Align loader and transfer docstrings, training documentation, and the changelog with the default-on contract, fallback behavior, and unmeasured performance boundary. Impact: - Custom gradient-bearing and mixed-device targets retain the existing unpacked path safely, while ordinary losslessly packable batches reduce worker handoff objects by default. - Direct DataLoader consumers may now receive `PackedTargets` by default; model hooks still receive independently materialized target dictionaries, and callers can opt out. - Maintainers have explicit follow-up criteria for representative detection and segmentation memory and transfer measurements. Verification: - `pre-commit run --all-files` passed, including configured formatting and mypy hooks. - `pytest tests/utilities/test_tensors.py tests/training/test_module_data.py -n 1 -m 'not gpu' --timeout=240` passed: 218 passed, 2 skipped. - `mkdocs build --strict` passed; independent QA review passed; `git diff --check` passed. Residual limits: - CUDA, DDP, and representative detection/segmentation host-device memory and transfer benchmarks were not run locally. - The author performance benchmark was not independently rerun on its stated hardware and dataset. --- Co-authored-by: Codex <codex@openai.com>
Follow-up to the hand-off ceiling found while measuring #1392. I open it separately because it is not about WebDataset — it sits downstream of the dataset and applies the same way to the existing loose-file loader. It doesn't compete with #1396, it composes with it: the change is in
collate_fnandtransfer_batch_to_device, so it doesn't care where the bytes come from, and it works the same for tar shards. Everything below is measured ondevelop, with no WebDataset in the tree.TL;DR — Every tensor a DataLoader worker returns gets moved into its own shared-memory segment and handed to the parent as a file descriptor. rf-detr returns 7 tensors per detection sample, so a batch of 16 crosses as 114 objects. Of those, 112 carry 3,592 bytes in total — 0.005% of a 65.81 MB batch — but they pay most of the per-object cost anyway. Concatenating each field into one tensor takes that down to 9 objects, same payload: 1.25x-1.67x loader throughput depending on worker count, and it removes a file-descriptor exhaustion crash the current loader hits at 16+ workers. Output is bit-identical and COCO metrics don't move, to ten decimal places. Off by default.
The cost, isolated
Only the collate output changes here, everything upstream stays the same. 16 workers, unpinned, three repetitions, median:
count)Shipping the targets as 112 separate objects costs 19.5 ms per batch — 128x what the packed form costs, and 7x the cost of the entire 65.81 MB image payload. So the cost is per object, not per byte. This ablation uses a standalone packer, so the only thing that varies is the collate output — the throughput table below uses the collate this PR actually ships.
Loader throughput, as shipped
COCO
train2017,RFDETRNano, resolution 576,batch_size=16, oneg2-standard-32(32 vCPU, L4).make_collate_fn(pack=True)against today's collate, 300 timed batches after 20 warm-up, three repetitions, median[min-max]:At 8 workers unpinned there is no gain: the ranges overlap, and the workers, not the hand-off, are the constraint there. Every other cell is non-overlapping. Pinned throughput dips from 365.6 at 16 workers to 337.0 at 32 — my guess is the single pin-memory thread turns into the serialiser once enough workers feed it. That would also explain why the unpinned column keeps scaling over the same step (443.9 → 465.9), but I did not instrument that thread to prove it.
It also removes a crash
At the stock soft limit of 1024 file descriptors, the current loader dies at 16 and 32 workers when
pin_memory=False:in
rebuild_storage_fd→recvfds. One descriptor per tensor, 114 per batch, several batches in flight. Reproduced 6 out of 6 attempts (three at each worker count), and the packed path completed every time in the same setup. So the unpinned rows above were measured withulimit -n 65536, otherwise both arms could not even run. This is also whyset_sharing_strategy("file_system")helps — it is the usual workaround for this exact failure — and packing gets there without touching the strategy. The pinned path is not affected: the same setup atulimit -n 1024withpin_memory=Trueruns clean (0ancdataerrors). So this is a hazard of the unpinned loader, not of high worker counts in general.pin_memory
PackedTargetsimplementspin_memory()and on purpose does not subclasscollections.abc.Sequence. PyTorch's pin-memory worker checks that ABC before it looks for apin_memorymethod (torch/utils/data/_utils/pin_memory.py), so a Sequence subclass gets taken apart and pinned tensor by tensor — which rebuilds, in the main process, exactly what packing was meant to avoid, in the default CUDA setup. As written, the batch stays packed through pinning, andtransfer_batch_to_devicemoves one tensor per field. There is a regression test for this, because the coupling is not obvious just from reading the class.Against
set_sharing_strategy("file_system"), which you asked aboutSame harness, 16 workers,
batch_size=16, three repetitions, median:file_descriptor(default)file_systemPinned, which is the CUDA default, packing is the better single change — 365.6 against 353.9 for the strategy switch — and the two together reach 417.9, 1.91x over the untouched baseline.
Unpinned, the answer flips, and I'd rather say so here than have it found in review:
file_systemalone reaches 488.3, better than packing onfile_descriptor(443.9), and packing on top offile_systemdrops to 427.6, 0.88x. For an unpinned loader the one-line strategy switch is the better change, and this patch should not be stacked on it. My guess isfile_systemalready removes the descriptor round-trip that packing exists to amortise, leaving only its bookkeeping — but I did not check that, so take it as an observation, not a mechanism.The saving is per-object, so it scales with batch size
Same worker count and pinning, varying only
batch_size:The per-object cost stays flat across a 4x range of batch size, and that's what makes this structural instead of a tuning artifact: a batch carries
7 x batch_sizetarget tensors no matter how many annotations each image holds, so the object count removed from the worker-to-main boundary follows from batch size alone. This run only sweeps batch size — annotation density, image content and dataset size were not varied. Packing's own compute (torch.cat, and.clone()inas_list()) does scale with the total number of boxes, just several orders below the ~19.5 ms/batch IPC saving measured above, at COCO's typical annotation density. What changes with upstream work — bigger images, heavier augmentation — is the ratio, because that grows the denominator.The operational effect: fewer workers needed
Eight packed workers beat thirty-two unpacked ones in the pinned default, and sixteen beat thirty-two unpinned. It's the same lock contention and thread thrashing from #1392, just showing up as free cores this time instead of extra throughput.
End to end, this changes nothing
A
RFDETRNanofine-tune (3 epochs, 2,000 images,batch_size=8, 8 workers, two repetitions):Identical within noise, and that's the expected result: this run only consumes about 24 img/s while the loader already supplies 220+, so the loader was never the bottleneck and making it faster can't help here. What this table is really showing is that there is no regression. The change pays off where the loader is the bottleneck — high worker counts, object storage, several ranks sharing a host's cores — and in removing the descriptor crash.
Correctness
RFDETRNanocheckpoint evaluated twice over COCOval, differing only inpack_targets: all 7 metrics identical to ten decimal places. Atresolution=576(whatrepro/eval_ab.pypasses):mAP@50:95 = 0.5181497931,mAP@50 = 0.7134777308,F1 = 0.6934525371. The absolute level depends on the eval resolution, so the same comparison run atevaluate()'s own default of 384 lands atmAP@50:95 = 0.4801311492919922— also exact, every metric differing by 0.0. So parity holds at both resolutions, but quote the number together with the resolution or the two won't match.build_trainer()setsdeterministic=Falsefor evaluation, same as for training, so nothing in the trainer path forces bit-exact reproducibility — the exact 0.0 diff above is what I observed running fixed weights through it twice, not a guarantee from the code.RFDETRDataModule: 25 batches, 200 images, 1,400 target tensors compared field by field aftertransfer_batch_to_device— 0 value, 0 dtype and 0 shape mismatches. Object count confirmed 58 → 9 atbatch_size=8.torch.catpromotes across dtypes instead of failing, and COCO hits that case: a sample with no annotations buildsarea/iscrowdfrom an empty list and gets float32, where a populated sample gets int64. Packing a field like that would silently change its dtype and can lose integer precision (2**54+1does not survive float32). The packer catches it and returns the batch untouched.reshape(-1)to preserve, sotorch.catraisesRuntimeErrorinstead of falling back on its own. A quantized value is trickier: it can sharetorch.dtypewith another sample while using a different(scale, zero_point), and in that casetorch.catsilently requantizes it to the first sample's parameters instead of failing — so dtype equality alone does not catch either case. Neither is reachable through this repo's own COCO/YOLO loaders, butpack_targetsis documented to fall back for any custom dataset it cannot pack losslessly. Both are now covered by that guard and a regression test.TrainConfig.pack_targetsis tested throughRFDETRDataModule, not only throughpack_targets()directly.train_dataloader().collate_fnis asserted to return aPackedTargetswhen the config sets it, and the original tuple of dicts when it does not — that covers the wiring between the config field and the DataLoader it configures.PackedTargets/pack_targetsare exported fromrfdetr.utilities, matchingNestedTensor/nested_tensor_from_tensor_list— the same public surface the image half of the collated batch already has, so a caller doingisinstance(targets, PackedTargets)againsttrain_dataloader()'s output doesn't need to reach into the private module. Thetransfer_batch_to_devicerow indocs/learn/train/customization.md's lifecycle-hooks table now also names the packed-targets branch.transfer_batch_to_device, and reverting theas_listcopy. Each mutation fails exactly the test that claims to cover it, and the suite goes back to green on revert.ci-tests-cpu.ymlruns it, on this branch and on pristinedevelop: 2 failures on both, the same two, 4312 passed here against 4296 — the difference is the tests this branch adds.mypy src/rfdetr/produces byte-identical output to pristinedevelop.Not covered
transfer_batch_to_devicealways materialises the packed batch back into a plain list (as_list(), one.clone()per sample) before handing it off, so the packed and materialised copies of a field sit in memory at the same time for the duration of that call. For a smallboxes/labelsfield this is negligible, but for a segmentationmasksfield (bytes scale with instances × resolution²) it is a real, unmeasured transient doubling of that field's memory. A mask field would also makeas_list()hold both the packed buffer and the materialised views at once. The obvious fix is to release each packed field as it gets materialised, which keeps memory bounded, but that's a follow-up, not implemented here.ptrace_scope), so the internal split stays unmeasured.Where the remaining headroom is
After packing, the boundary is 2.79 ms of a 31.7 ms batch (8.8%). That bounds the structural follow-ups: a pre-allocated buffer pool, or packing the image into the same buffer, can only attack that 2.79 ms — and that means replacing the loader's IPC, not just changing a
collate_fn. Cutting image bytes does not stack —uint8on top of packing measured 0.983x, with overlapping ranges. The only lever that would reach the upstream 28.9 ms is moving the boundary itself, crossing compressed bytes and decoding on GPU — but that conflicts with keeping Albumentations usable, since it works on the CPU-decoded image.Default
TrainConfig.pack_targetsdefaults toFalse, so nothing changes unless you ask for it. My proposal is to default it toTruefor the pinned path, which is the CUDA default and where it's faster at every worker count I measured, and composes withfile_system. I would not turn it on for an unpinned loader already onfile_system, where it measured 0.88x. At 16 or more workers the current path doesn't just run slower, it crashes on a system with an untouched descriptor limit, and the packed path is faster in five of six setups and no worse in the sixth. The cost is a visible API surface — a caller consumingdatamodule.train_dataloader()directly gets aPackedTargetsinstead of a tuple of dicts whenever a batch packs losslessly — which is why I did not flip it on my own. All six setups above arebatch_size=16withnum_workersin{8, 16, 32}.TrainConfig's own shipped defaults arebatch_size=4andnum_workers=2, a regime none of this throughput grid covers, and where neither the crash nor the gain is measured. If defaulting it on is too much for one PR, the smaller step is to warn whennum_workers >= 16withpin_memory=Falseandpack_targets=False, which is the exact combination that dies. The full suite above ran with it on, so the evidence for the flip exists either way — the call is yours.Happy to re-run any of this on your 48-core box if you want the numbers from the machine that motivated #1392 .. the scenarios where this pays off end to end (many ranks, constrained cores, object storage) are argued from the mechanism here, and your machine is the one that would actually settle it.