Wait for the caller's stream before the nvJPEG hardware decode writes - #1634
Wait for the caller's stream before the nvJPEG hardware decode writes#1634karkuspeter wants to merge 1 commit into
Conversation
nvJPEG's hardware engine can write its destination buffers before work that was already queued on the stream it was given has run. Those destinations come from the caching allocator, which hands out a block as soon as it is freed on the stream that owns it, so the decoder can be handed memory that queued kernels are still reading and silently corrupt them. Host-synchronize the stream before the batched hardware decode, which costs no wall time because decode_images() already synchronizes it before returning, and add a regression test.
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/meta-pytorch/torchcodec/1634
Note: Links to docs will display an error until the docs builds have been completed. This comment was automatically generated by Dr. CI and updates every 15 minutes. |
|
Hi @karkuspeter! Thank you for your pull request and welcome to our community. Action RequiredIn order to merge any pull request (code, docs, etc.), we require contributors to sign our Contributor License Agreement, and we don't seem to have one on file for you. ProcessIn order for us to review and merge your suggested changes, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA. Once the CLA is signed, our tooling will perform checks and validations. Afterwards, the pull request will be tagged with If you have received this in error or have any questions, please contact us at cla@meta.com. Thanks! |
|
Thanks @karkuspeter I can reproduce on an H100 (not on an A100, similar to yours). I'll get to this eventually. In the mean time, out of curiosity is that a bug that was reported to NVJPEG? |
|
Thanks @NicolasHug great to hear you were able to reproduce. I am preparing a bugreport / fix for nvjpeg as well |
decode_jpeg(..., device="cuda")can silently corrupt unrelated GPU memory.nvJPEG's hardware engine does not honour the stream it is given: with work already queued on that
stream,
nvjpegDecodeBatchedwrites its destination buffers before that work has run. Thosedestinations come from PyTorch's caching allocator, which hands out a block as soon as it is freed on
the stream that owns it — so the decoder can be handed memory that queued kernels are still reading,
and overwrite it. The caller sees wrong numbers, NaNs, or a device-side assert in code that never
touched the decoder.
The wrapper here is not at fault; it submits on the caller's stream, which is the right thing to do.
The fix belongs in nvJPEG, and until it lands the decoder has to defend itself: this adds one
cudaStreamSynchronize(stream)indecode_batched_hardware, after the outputs are allocated andbefore the first
nvjpegDecodeBatched. It costs no wall time today, becausedecode_imagesalreadyhost-synchronizes that same stream before returning, so the wait moves earlier rather than being
added — median 1920x1080 decode over 200 calls is 1.186 ms with the barrier and 1.183 ms without. The
software path is untouched.
Reproducing it
repro.pybelow needs only torch and torchcodec, on a GPU with an nvJPEG hardware engine. Eachiteration fills four tensors and synchronizes so they are resident, queues ~100 ms of work followed by
one reduction per tensor, frees the tensors while those reductions are still queued, then decodes into
what the allocator just took back. The sums are exact integers, so they are only wrong if the decode
wrote early.
H100 80GB HBM3, driver 575.57.08, torch 2.13.0+cu129, torchcodec v0.16.0 built against the CUDA 12.8
toolkit (nvJPEG 12.3.5); the released 0.16.0+cu129 wheel (nvJPEG 12.4.0.76) behaves the same.
repro.pyWhy this is nvJPEG and not the wrapper
The program below involves no framework, no allocator and no aliasing: one
cudaMalloc'd destinationis reused for the whole run, and its only other user is a checksum kernel on the same stream. Each
iteration zeroes the buffer and synchronizes, queues a ~100 ms spin kernel and then the checksum, then
decodes into that buffer on that stream. The checksum must be zero.
nvjpegDecodeBatchednvjpegDecodeBatchedcudaStreamSynchronizenvjpegDecodeBatchedcudaStreamWaitEventon a second streamnvjpegDecodeBatchednvjpegDecodenvjpegDecodeBatchedThis contradicts the documented contract, that
nvjpegDecodeBatched"is asynchronous with respect tothe host. All GPU tasks for this function will be submitted to the provided stream", together with
in-order execution within a stream. Reproduced on H100 with nvJPEG 12.3.5, 12.4.0.76, 13.0.1 and
13.2.1.
Rows three and four are why the barrier has to be a host wait. A device-side dependency does not gate
the write: submitting the decode on a second stream that holds a pending
cudaStreamWaitEventon thebusy stream is out of order in all 100 iterations, while waiting for that same event on the host first
is clean in all 100. The write happens when the call is made. The host-side call times say the same
thing from the other end — with ~100 ms queued, the hardware call returns in 0.1 ms and writes anyway,
while the software backend's call blocks the host for ~76 ms, so the software path is ordered because
it waits, not because it takes a device-side dependency. NVIDIA's DALI carries the same workaround for
the same class of problem in nvImageCodec (NVIDIA/DALI#5408).
nvjpeg_hw_stream_order.cu, built withnvcc -O2 -std=c++17 nvjpeg_hw_stream_order.cu -o nvjpeg_hw_stream_order -lnvjpegTest
TestImageDecoder::test_cuda_jpeg_waits_for_callers_stream, the same shape as the reproducer. Itskips itself if the allocator never reused a freed block, so it cannot quietly become a test of
nothing.
Built from v0.16.0 with and without the barrier, same tree and same build configuration: the test
passes 5 runs out of 5 with it and fails 3 out of 3 without it.
-k "jpeg or Jpeg"is otherwiseunchanged at 121 passed, with one pre-existing failure either way (
test_cuda_jpeg_errorsexpectsnvjpegDecode failed:for corrupt input, which this GPU's hardware engine does not produce).Scope
The hardware batched path only; the software path is unaffected, which the
nvjpegDecodeandsoftware-backend rows above show. An A100 on driver 535.129.03 was clean over 300 iterations while two
H100 machines reproduce, so architecture and driver version move together across the hardware I have
and I cannot say which one matters. The test passes vacuously on unaffected GPUs.
Related
torchvision.io.decode_jpeg. That decoder has anadditional defect of its own — it decodes on a private stream that is never ordered against the
caller's, so it corrupts on nvJPEG's software path too. This wrapper does not have that problem.
EncodeJpegCuda.cppfollows the same caller's-stream-plus-host-sync design as decode, and whethernvJPEG's encode path has the same defect is untested.