Skip to content

Bump up nvComp version to 5.3.0.16 - #6430

Open
JanuszL wants to merge 5 commits into
NVIDIA:mainfrom
JanuszL:bump_up_nvcomp_version
Open

Bump up nvComp version to 5.3.0.16#6430
JanuszL wants to merge 5 commits into
NVIDIA:mainfrom
JanuszL:bump_up_nvcomp_version

Conversation

@JanuszL

@JanuszL JanuszL commented Jul 23, 2026

Copy link
Copy Markdown
Contributor

Category:

Bug fix

Description:

  • Bumps nvCOMP to 5.3.0.16.
  • Fixes test_dali_variable_batch_size.test_inflate, which compressed int64
    samples while relying on the Inflate default output type of uint8.
  • Adds the optional check_output_size argument to decoders.inflate.
    When enabled, it validates the decoded LZ4 size before decompression and
    raises an error if the requested output buffer is too small.

Additional information:

Affected modules and functionalities:

  • nvCOMP dependency definitions and CUDA Dockerfiles
  • GPU LZ4 Inflate operator
  • Inflate operator tests

Key points relevant for the review:

  • check_output_size is disabled by default to preserve existing behavior and
    avoid an additional GPU stream synchronization.
  • When enabled, it prevents calling nvCOMP with an output buffer that is
    smaller than the compressed payload expands to.

Tests:

  • Existing tests apply
    • operator_1.test_inflate (141 tests)
    • test_dali_variable_batch_size.test_inflate
  • New tests added
    • Python tests
      • test_rejects_insufficient_output_buffer
  • GTests
  • Benchmark
  • Other

Checklist

Documentation

  • Existing documentation applies
  • Documentation updated
    • Docstring
  • RST
  • Jupyter
  • Other

DALI team only

Requirements

  • Implements new requirements
  • Affects existing requirements
  • N/A

REQ IDs: N/A

JIRA TASK: N/A

JanuszL added 2 commits July 22, 2026 16:57
Signed-off-by: Janusz Lisiecki <jlisiecki@nvidia.com>
Signed-off-by: Janusz Lisiecki <jlisiecki@nvidia.com>
Comment on lines +57 to +62
.AddOptionalArg<bool>(inflate::checkOutputSizeArgName,
R"code(If True, validates before decompression that the requested output
buffers are large enough for the compressed data.

This validation synchronizes the GPU stream and is disabled by default.)code",
false)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@stiepan did we have a discussion about this unsafe behavior and potential switch for that or I mixing things?

@greptile-apps

greptile-apps Bot commented Jul 23, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR bumps nvCOMP from 5.2.0.x to 5.3.0.16 across CMake, Dockerfiles, and the stub generator, and adds an optional check_output_size argument to decoders.inflate that pre-validates output buffer sizes before GPU LZ4 decompression. It also fixes a latent bug in the variable-batch-size test where int64 samples were compressed but inflated with the default uint8 output type.

  • check_output_size flag calls nvcompBatchedLZ4GetDecompressSizeAsync, copies results to host, synchronises the stream, and raises a descriptive error if any chunk's output buffer is too small; the feature is opt-in to avoid the extra synchronisation in normal paths.
  • Variable-batch-size test fix adds dtype=types.INT64 to match the int64 test samples, resolving the dtype mismatch that caused test_inflate to fail.
  • Two new Python tests cover both the rejection path and the positive path for check_output_size=True.

Confidence Score: 4/5

Safe to merge after the docstring is corrected; the GPU decompression logic and stream ordering are sound.

The new check_output_size path has correct stream ordering and actionable error messages. One user-facing docstring says compressed data where it should say decompressed data, which would mislead users about what the flag actually checks. All other changes are straightforward.

dali/operators/decoder/inflate/inflate.cc - the check_output_size docstring needs compressed corrected to decompressed.

Important Files Changed

Filename Overview
dali/operators/decoder/inflate/inflate.cc Adds check_output_size optional bool arg to operator spec; docstring incorrectly says compressed data instead of decompressed data.
dali/operators/decoder/inflate/inflate_gpu.cc Implements check_output_size_ path: queries decoded sizes via nvcompBatchedLZ4GetDecompressSizeAsync, D2H copies, stream-syncs, then validates each chunk; logic and stream ordering are correct.
dali/operators/decoder/inflate/inflate_params.h Adds checkOutputSizeArgName constant; straightforward and consistent with existing arg-name declarations.
dali/test/python/operator_1/test_inflate.py Adds two new tests covering rejection and positive paths for check_output_size=True; both use real value comparisons via check_batch. Copyright year updated to 2026.
dali/test/python/test_dali_variable_batch_size.py Fixes test_inflate by adding dtype=types.INT64 to match the int64 test samples, resolving the silent type mismatch that caused the test to fail.
cmake/Dependencies.common.cmake Version pin updated from 5.2.0.13 to 5.3.0.16; straightforward one-line change.
internal_tools/stub_generator/nvcomp.json Adds nvcompBatchedLZ4GetDecompressSizeAsync stub entry; matches the new call site in inflate_gpu.cc.

Sequence Diagram

sequenceDiagram
    participant U as User Pipeline
    participant I as InflateOpGpuLZ4Impl
    participant nv as nvCOMP (GPU)
    participant H as Host (CPU)

    U->>I: RunImpl(ws)
    I->>I: SetupInChunks / SetupOutChunks
    I->>nv: ToContiguousGPU (in_sizes, in, out_sizes, out)

    alt "check_output_size == true"
        I->>nv: nvcompBatchedLZ4GetDecompressSizeAsync(stream)
        I->>H: cudaMemcpyAsync D2H decoded_sizes (stream)
        I->>I: cudaStreamSynchronize
        I->>I: "Validate each chunk decoded_size <= inflated_size"
        note over I,H: Raises DALI_ENFORCE if too small
    end

    I->>nv: nvcompBatchedLZ4DecompressGetTempSizeAsync
    I->>nv: nvcompBatchedLZ4DecompressAsync(stream)
    I->>nv: FillTheTails zero-pad oversized output
    nv-->>U: decompressed output tensor
Loading

Reviews (4): Last reviewed commit: "Add missing nvCOMP LZ4 dynamic stub" | Re-trigger Greptile

Comment thread dali/operators/decoder/inflate/inflate_gpu.cc
Comment thread dali/test/python/operator_1/test_inflate.py
@dali-automaton

Copy link
Copy Markdown
Collaborator

CI MESSAGE: [59212184]: BUILD STARTED

@dali-automaton

Copy link
Copy Markdown
Collaborator

CI MESSAGE: [59212200]: BUILD STARTED

Signed-off-by: Janusz Lisiecki <jlisiecki@nvidia.com>
@JanuszL

JanuszL commented Jul 23, 2026

Copy link
Copy Markdown
Contributor Author

Addressed all four Greptile findings in 901733d:

  • updated the modified Python test copyright year;
  • moved check_output_size_ into the GPU-only LZ4 implementation;
  • made the validation error identify both sample and chunk-within-sample;
  • added a successful check_output_size=True regression test.

Validated with operator_1.test_inflate and test_dali_variable_batch_size.test_inflate (143 tests total).

Signed-off-by: Janusz Lisiecki <jlisiecki@nvidia.com>
@dali-automaton

Copy link
Copy Markdown
Collaborator

CI MESSAGE: [59214762]: BUILD STARTED

@dali-automaton

Copy link
Copy Markdown
Collaborator

CI MESSAGE: [59214775]: BUILD STARTED

@dali-automaton

Copy link
Copy Markdown
Collaborator

CI MESSAGE: [59214762]: BUILD FAILED

@dali-automaton

Copy link
Copy Markdown
Collaborator

CI MESSAGE: [59214775]: BUILD FAILED

Signed-off-by: Janusz Lisiecki <jlisiecki@nvidia.com>
@dali-automaton

Copy link
Copy Markdown
Collaborator

CI MESSAGE: [59239054]: BUILD STARTED

@dali-automaton

Copy link
Copy Markdown
Collaborator

CI MESSAGE: [59239066]: BUILD STARTED

@dali-automaton

Copy link
Copy Markdown
Collaborator

CI MESSAGE: [59239066]: BUILD PASSED

@dali-automaton

Copy link
Copy Markdown
Collaborator

CI MESSAGE: [59239054]: BUILD PASSED

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants