Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update Zstd decompression for "unknown decompressed size" when streaming API was used for compression #116

Open
3 tasks
mkitti opened this issue May 18, 2024 · 4 comments
Assignees
Labels
Filter - ZSTD Priority - 1. High 🔼 These are important issues that should be resolved in the next release Type - Improvement

Comments

@mkitti
Copy link

mkitti commented May 18, 2024

Introduction

The Zstandard plugin for HDF5 should be modified to allow for an unknown decompressed size in the frame header.

Currently, the Zstd decompression scheme, following from the original implemention, uses ZSTD_getDecompressedSize to obtain the size of the decompressed buffer. The returned value is not validated and passed directly to malloc.

size_t decompSize = ZSTD_getDecompressedSize(*buf, origSize);
if (NULL == (outbuf = malloc(decompSize)))

ZSTD_getDecompressedSize returns 0 if the decompressed size is empty, unknown, or an error has occured. If malloc is asked to allocate 0 bytes, it will return NULL, resulting in returning an error condition. This is an incorrect result if the decompressed size is actually empty or unknown and there is no actual error. ZSTD_getDecompressedSize is obsolete.

ZSTD_getFrameContentSize should replace the use of ZSTD_getDecompressedSize. ZSTD_getFrameContentSize distinguishes between empty, unknown, or an error. The unknown or error states are indicated by a return value of ZSTD_CONTENTSIZE_UNKNOWN or ZSTD_CONTENTSIZE_ERROR, respectively.

The unknown decompression state is common. This occurs when the compression is done via the streaming API via ZSTD_compressStream or ZSTD_compressStream2. ZSTD_compressStream2 in particular only stores the frame size when either ZSTD_e_end is provided on the initial call or ZSTD_CCtx_setPledgedSrcSize is used.

Tasks

  • Use ZSTD_getFrameContentSize instead of the obsolete ZSTD_getDecompressedSize to correctly distinguish between empty, unknown, or error states when determining the decompressed size.
  • Recognize the unknown size state by checking the return value of ZSTD_getFrameContentSize against ZSTD_CONTENTSIZE_UNKNOWN
  • Address unknown size state by growing buffer if needed or using stream decompression API via ZSTD_decompressStream
    • The HDF5 library should know the expected number of bytes for a chunk. We should not be relying on the filter to figure this out.

References

[1] https://facebook.github.io/zstd/zstd_manual.html

@derobins derobins added this to the HDF5 1.14.5 Release milestone Aug 15, 2024
@derobins derobins added Type - Improvement Priority - 1. High 🔼 These are important issues that should be resolved in the next release labels Aug 15, 2024
@byrnHDF
Copy link
Collaborator

byrnHDF commented Nov 18, 2024

ZSTD_getFrameContentSize change has been implemented - but does not check for UNKNOWN.

@mkitti
Copy link
Author

mkitti commented Nov 18, 2024

@byrnHDF If it is helpful, I recently implemented ZSTD_getFrameContentSize with a check for ZSTD_CONTENTSIZE_UNKNOWN in C++ for numcodecs.js via WASM. The code is mostly just plain C though:

https://github.com/manzt/numcodecs.js/blob/fa8a86065ce42ca024ecee8d376a440bdd304ac1/codecs/zstd/zstd_codec.cpp#L52-L128

@byrnHDF
Copy link
Collaborator

byrnHDF commented Nov 18, 2024

That does help to see the logic. However, wouldn't this only be a problem if someone created an hdf5 file outside of the hdf5 library? Because the HDF5 filter wouldn't be using the stream version?

@mkitti
Copy link
Author

mkitti commented Nov 18, 2024

The HDF5 C library does offer multiple APIs to read and write raw chunks. In particular, one could write a raw chunk via H5Dwrite_chunk. In this case, a 3rd party could employ Zstandard compression, perhaps even in parallel across multiple threads, and then use H5Dwrite_chunk to write the chunks using a single thread.

There are multiple instances where this approach is being taken by those interested in accelerated compression:

  1. @FrancescAlted for example has presented about using a multithreaded parallel approach for Blosc2:
    https://www.hdfgroup.org/wp-content/uploads/2022/05/Blosc2-and-HDF5-European-HUG2022.pdf
  2. @ImarisRyder has used this approach in ImarisWriter for GZIP and LZ4 compression:
    https://github.com/imaris/ImarisWriter/blob/b128e6e7d1a147261e9d5caf24ebc6b5c9c63779/writer/bpWriterHDF5.cxx#L541
  3. The Julia package JLD2.jl writes HDF5 files independently of the C library, including Zstandard compression:
    Add Zstd compression JuliaIO/JLD2.jl#560

Thus, we cannot assume that all chunks in a HDF5 container would have been compressed by the compression routine present in this repository. It is possible that streaming compression was used to encode a chunk, and the decompressed size was not encoded in the frame header.

I have specifically encountered this specific issue in real world data when working with with Zarr datasets or N5 datasets. I anticipate that this may occur with HDF5 datasets at some point, and I hope that this reference implementation would be robust enough to deal with any Zstandard compressed data given its increasing ubiquity.

By the way, here are some additional examples in the Zstandard repository itself:
https://github.com/facebook/zstd/blob/dev/examples/streaming_memory_usage.c
https://github.com/facebook/zstd/blob/dev/examples/streaming_decompression.c

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Filter - ZSTD Priority - 1. High 🔼 These are important issues that should be resolved in the next release Type - Improvement
Projects
None yet
Development

No branches or pull requests

3 participants