gix-pack: respect configured zlib compression levels instead of a hard-coded fastest level#2695
Conversation
…ded fastest level (GitoxideLabs#2024) The deflate stream used for pack entries, loose objects and thin-pack completion was hard-coded to the fastest compression level. That stemmed from `miniz_oxide` times where level 1 still compressed reasonably well - with the switch to the `zlib-rs` backend, whose level 1 is tuned for speed at the cost of ratio, pack entries grew by ~50%. Remove the hidden default entirely: `Compress`, `deflate::Write`, `output::Entry::from_data`, `input::Entry::from_data_obj`, `LookupRefDeltaObjectsIter` and `loose::Store` now take an explicit `Compression` level, while option structs default to what `git` uses so behaviour matches it out of the box: packs deflate with the zlib default (level 6, like `pack.compression`), loose objects with the fastest level (like `core.looseCompression`). The `core.compression`, `core.looseCompression` and `pack.compression` configuration keys are now understood and validated, with `-1` mapping to the zlib default just like `git`, and are respected when writing loose objects, creating packs and completing thin packs. The issue's reproduction drops from 29547 to 18724 bytes for a 72kB tree entry, back in line with what `git` produces.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 14ddfbf366
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "Codex (@codex) review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "Codex (@codex) address that feedback".
| .integer(Pack::COMPRESSION) | ||
| .or_else(|| config.integer(Core::COMPRESSION)) |
There was a problem hiding this comment.
Here Snapshot::integer() returns None both when the key is absent and when its value cannot be parsed as an integer. In a repo with pack.compression = bad (or a malformed core.compression fallback), gix free pack create silently falls through to the next/default level instead of reporting the invalid configuration, so the new knob can be ignored and packs are produced at the wrong level. Use try_integer() and pass the Result through the key conversion so malformed values are not treated as unset.
Useful? React with 👍 / 👎.
| self.loose_compression = | ||
| util::parse_loose_compression(config, self.lenient_config, self.filter_config_section)?; |
There was a problem hiding this comment.
After an in-memory config update such as repo.config_snapshot_mut().set_value(Core::LOOSE_COMPRESSION, ...).commit(), this only updates the cached value. The already-open gix_odb::Store captured its loose_compression in Store::at_opts(), and Repository::reread_values_and_clear_caches_replacing_config() only calls setup_objects() for cache-size changes, which does not replace or update the store; subsequent write_blob/write_object calls therefore keep using the old compression level until a full reload.
Useful? React with 👍 / 👎.
1 participant
Fixes #2024.
Problem
The deflate stream used for pack entries, loose objects and thin-pack completion was hard-coded to the fastest compression level (
DeflateConfig::best_speed()). That was fine inminiz_oxidetimes, whose level 1 follows stock zlib — butzlib-rs's level 1 is deliberately tuned for speed at the cost of ratio (as Folkert de Vries (@folkertdev) explained in the issue), so after the backend switch that the issue bisected to, pack entries grew by ~50%: the issue's 72kB tree compresses to 29,547 bytes onmain.Changes
This implements the full fix discussed in the issue — no hidden compression default at the lower levels, with configuration wired through:
gix-features: a newzlib::Compressiontype (levels 0–9, withNONE/BEST_SPEED/DEFAULT/BEST);Compress::new(),deflate::Write::new()now take it explicitly, andDefault for Compressis gone.Writeremembers its level soClonerecreates it faithfully.BEST_SPEEDis field-identical to the previousbest_speed()config, so existing loose-object output is byte-for-byte unchanged.gix-pack:output::Entry::from_data(),input::Entry::from_data_obj()andLookupRefDeltaObjectsItertake the level;iter_from_counts::Optionsandbundle::write::Optionsgained acompressionfield.gix-odb:loose::Store::at()and the dynamic store'sinit::Optionscarry the loose compression level;Sink::compress()takesOption<Compression>.gix: thecore.compression,core.looseCompressionandpack.compressionkeys are now understood and validated (rejecting anything outside-1..=9early), and respected when writing loose objects, creating packs, and completing thin packs during fetch. The level is picked up when the repository is opened, like the other object-database options.gitoxide-core:gix free pack createreadspack.compression/core.compressionfrom the repository.Defaults match
gitexactly (verified against its source)pack.compression→core.compression-1→ zlib default (6)core.looseCompression→core.compressionindex-pack, which deflates appended bases withzlib_compression_level-1maps to the zlib default at parse time for all keys, exactly asenvironment.cdoes it.Results
gix free pack createwithpack.compression=0→ 2.4MB pack,=9→ 187KB on the same objects.Validation
< 25000threshold), a loose-store level test (round-trip + size ordering), and config-key validation/conversion tests including the-1mapping.cargo testforgix-features,gix-pack,gix-odb,gix,gitoxide-core;cargo clippy --workspace --all-targets -- -D warnings(also--no-default-features --features max-pure);cargo fmt --all -- --check;RUSTDOCFLAGS='-D warnings' cargo doc --workspace --no-deps.