feat: add zxc compression support#362
Open
hellobertrand wants to merge 7 commits into
Open
Conversation
Open
mhx
reviewed
Apr 18, 2026
Owner
There was a problem hiding this comment.
Thanks for your PR, @hellobertrand!
This already seems to be in pretty good shape and I'm happy to integrate it under a few conditions:
- It should be rock-solid on all platforms supported by DwarFS and the format should be completely platform-independent.
- The decompression code must guarantee to always be able to read any compressed blobs generated with v0.10.0 or later. (I'm sure that's the case.)
- The code size of the (compressed) statically linked binaries should not increase significantly. I'm assuming this also won't be the case. But integrating brotli was a very bad idea in this regard due to its compiled-in dictionaries.
It is quite possible that this PR is going to stay unmerged for a while. That is mainly because I've made a whole bunch of changes to the CI config in my work branch and I'll need to update those configs to also build cross-compiled versions of zxc before giving this a full spin. Just saying that if this hangs around for a few weeks, it's not because I'm not interested. :)
Owner
|
One question regarding building the Disabling |
Owner
|
There's probably still a bit of work to do :) That's on s390x. Can you check that |
Author
|
Thanks for the thorough review, @mhx! Really appreciate you taking the time, and I'm glad to hear you're open to integrating zxc into DwarFS. I've addressed all three points in the updated commit:
Regarding the s390x SEGFAULT, thank you for catching that, this is very valuable. The crash is in Regarding the static build config you mentioned: That looks correct to me. Disabling No rush on the merge at all; take whatever time you need. I'm just happy that zxc might find a home in DwarFS. Thanks again! 🙏 |
Owner
FWIW, I saw the same crash on ppc64 and ppc64le, so I'm not entirely sure it's a big-endian only thing. Didn't have time to look into it, though.
Cool, thanks!
Yeah, it's always a bit of a chicken-and-egg problem. You don't really get all bugs ironed out unless you have some exposure. And unless you have that exposure and the serious bugs are ironed out, it's hard to get something integrated. |
mhx
reviewed
Apr 18, 2026
hellobertrand
force-pushed
the
zxc-compressor
branch
from
4df66e8 to
0616144
Compare
Author
|
Thanks for the detailed report. I investigated and I think I have found the root cause and it's not endianness-related. The crash occurs on all Linux architectures (s390x, ppc64, ppc64le) because it's a heap overflow detected by glibc's malloc. The previous code called auto const csize = ::zxc_compress_block(cctx_, data.data(), data.size(),
compressed.data() + size_size,
compressed.size() - size_size, nullptr);With When DwarFS sends a block larger than 256 KB (e.g. 4 MB with FixNow we pass explicit opts with zxc_compress_opts_t copts{};
copts.level = level_;
copts.block_size = std::max(
std::bit_ceil(data.size()),
static_cast<size_t>(ZXC_BLOCK_SIZE_MIN));
copts.checksum_enabled = 0;
|
Author
|
I've also addressed this upstream for the upcoming zxc release to improve the API's safety and ease of use. The block_size parameter in |
Owner
|
I made a small commit locally to add That gave me a whole bunch of errors during decompression. :) They all look pretty much the same ( |
Owner
|
Here's another problem (playing with Sometimes this: |
Owner
|
Here's another trace of the double-free error, this time without the |
Owner
|
Yeah, this felt like some kind of race condition given how non-deterministic it was. And thread sanitizer triggered literally within milliseconds: |
Owner
|
I was able to build an image by setting the number of compression threads to 1, which confirms the race: However, trying to do a checksum over all files in the image... ...and tons of error after that. This is likely the same issue as the one from the unit tests. |
Author
|
Thanks for the detailed reports @mhx: all three issues are now fixed: Bug 1 & 3: Thread-safety / heap corruption in compressor: The shared Bug 2: ZXC_ERROR_OVERFLOW in decompressor: Tests: Added |
Owner
|
Tried with all the latest commits; The more blocks there are, the more errors I'm seeing (the above is with |
Author
Notes on libzxc gaps found while integratingTwo things on the libzxc side would let this PR be cleaner. Both are additive and I'll open a companion PR upstream. 1. Missing
|
Contributor
I switched my DwFS submodule to this PR and added zxc to the project just to test*. +50 Kb to the exe (this includes changes from this PR + zxc itself), everything is linked statically with ClangLTO (from DwarFS, common, compressor, and reader libraries are linked, I started using DwFS to store game assets). That's a very small increase I'd say. BTW once zxc is here, maybe it would be worth to reconsider the default DwFS compression presets and put zxc there somewhere (I think most people just use the default presets). * to test how it builds, links and weights, haven't tried compressing assets with zxc yet (as last time I checked this thread there still were issues) |
Author
|
Focused test scope (all green):
Heads-up: decompression goes through an internal scratch buffer plus a I haven't benchmarked yet, but I expect the extra allocation + memcpy per block to show up on decompression-heavy workloads. Compression is unaffected. The clean fix is a bounds-checked |
Owner
That's pretty much what I saw: #362 (comment)
And because most people just use the default presets, |
Owner
I'm not planning to push out a new DwarFS release in the near future, so I think we can wait until |
Contributor
Yep, I agree that would be a better option. |
Owner
The people who want to use That doesn't mean |
Add zxc as a new compression backend (type ID 8), using the block-level API (zxc_compress_block / zxc_decompress_block) for minimal framing overhead. Uncompressed size is stored as a little-endian uint32 prefix. Features: - Compression level configurable via -C zxc:level=N (runtime range from zxc_min_level()..zxc_max_level()) - Internal block size configurable via -C zxc:block_size=N (4KB..2MB, default 256KB) - Reusable compression/decompression contexts (zxc_create_cctx/dctx) - Runtime library version reporting via zxc_version_string() - Defensive bounds check on decompression input Build integration: - Optional via TRY_ENABLE_ZXC (ON by default) - Detected via pkg-config (libzxc >= 0.10.0) - Available through vcpkg (zxc >= 0.10.0)
hellobertrand
force-pushed
the
zxc-compressor
branch
from
ae57f61 to
eed27bd
Compare
…ion buffer - Use varint encoding for the uncompressed size prefix instead of a fixed uint32, saving space on smaller blocks. - Remove the configurable internal block size in favor of a dynamic value based on input data size. - Implement a temporary scratch buffer for decompression to safely accommodate zxc's speculative wild-copy writes, which require tail padding beyond the exact uncompressed size. - Move compression context creation to the compress call to simplify state management.
Adds zxc compression levels 1 and 3 to the list of compression options used for testing when libzxc support is enabled.
hellobertrand
force-pushed
the
zxc-compressor
branch
from
eed27bd to
d64d171
Compare
hellobertrand
force-pushed
the
zxc-compressor
branch
from
d64d171 to
bea2ecc
Compare
Contributor
|
@hellobertrand, the last commit doesn't build -- there's no |
hellobertrand
force-pushed
the
zxc-compressor
branch
from
bea2ecc to
e00af19
Compare
Author
@solbjorn My bad! Forgot to push the local changes. Fixed. |
hellobertrand
force-pushed
the
zxc-compressor
branch
2 times, most recently
from
6ad613d to
3b8561e
Compare
hellobertrand
commented
Apr 25, 2026
| pkg_check_modules(LIBBROTLIENC IMPORTED_TARGET libbrotlienc>=${LIBBROTLI_REQUIRED_VERSION}) | ||
| endif() | ||
| if(TRY_ENABLE_ZXC) | ||
| pkg_check_modules(LIBZXC IMPORTED_TARGET libzxc>=0.10.0) |
Author
There was a problem hiding this comment.
bump to 0.10.1 when published
hellobertrand
commented
Apr 25, 2026
| {"name": "zxc", "version>=": "0.10.0"} | ||
| ], | ||
| "overrides": [ | ||
| {"name": "zxc", "version": "0.10.0"} |
Author
There was a problem hiding this comment.
bump to 0.10.1 when published
hellobertrand
force-pushed
the
zxc-compressor
branch
from
3b8561e to
d46f291
Compare
hellobertrand
force-pushed
the
zxc-compressor
branch
from
d46f291 to
790d741
Compare
Contributor
|
Minor: when I ran clang-format locally before creating my PR (my local work tree also contains changes from this PR), I noticed that it made a couple changes in What it did is: diff --git a/src/compression_registry.h b/src/compression_registry.h
index 8858b4c05102..d79ade3f2869 100644
--- a/src/compression_registry.h
+++ b/src/compression_registry.h
@@ -40,9 +40,9 @@
#include <range/v3/view/map.hpp>
#include <dwarfs/config.h>
-#include <dwarfs/library_dependencies.h>
#include <dwarfs/detail/compression_registry.h>
#include <dwarfs/fstypes.h>
+#include <dwarfs/library_dependencies.h>
namespace dwarfs::detail {
@@ -75,12 +75,11 @@ void compression_registry<FactoryT, InfoT>::for_each_algorithm(
template <typename FactoryT, typename InfoT>
void compression_registry<FactoryT, InfoT>::add_library_dependencies(
library_dependencies& deps) const {
- this->for_each_algorithm(
- [&](compression_type, InfoT const& info) {
- for (auto const& lib : info.library_dependencies()) {
- deps.add_library(lib);
- }
- });
+ this->for_each_algorithm([&](compression_type, InfoT const& info) {
+ for (auto const& lib : info.library_dependencies()) {
+ deps.add_library(lib);
+ }
+ });
}
template <typename FactoryT, typename InfoT> |
Owner
|
Contributor
I have 21 in WSL and 22 on Windows, rechecked, both give me the same diff I pasted above. Anyway, not related directly to ZXC, just a suggestion (when I create a PR, I usually run clang-format on every file I touched, so I noticed this one). |
Owner
Oh, my bad! The pattern I've been using for the |
Define LIBZXC_REQUIRED_VERSION alongside other library versions and use it in the pkg_check_modules call for consistency.
Previously, the compression context (cctx) memory estimation did not consider the active compression level. This change ensures the `zxc_estimate_cctx_size` call includes the level for a more accurate memory usage prediction.
Contributor
|
Needs updating since recent commits changed the |
Author
fixed |
Contributor
|
New ZXC version is out, the API is there -- shouldn't this be converted from WIP to a proper PR and considered for merging? |
|
Author
|
@solbjorn Yes, agreed. zxc 0.11.0 is now published on vcpkg, so the build constraint resolves cleanly. The patch has been validated end-to-end:
Marking as ready for review. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adds zxc as a new compression backend (type ID 8), available via
-C zxc[:level=N].Design
Uses the buffer-level API (
zxc_compress/zxc_decompress)Runtime metadata via
zxc_version_string(),zxc_min/max/default_level()Options
level: compression level (
zxc_min_level()..zxc_max_level())Build
Optional via
TRY_ENABLE_ZXC(ON by default)Detected via pkg-config (
libzxc >= 0.11.0)Available through vcpkg with a version override (baseline has 0.7.3, override pins 0.11.0)
Requires zxc ≥ 0.11.0 for the block-level API and runtime metadata functions.