deflate: observe post-init compression level changes

[asonje]

deflateParams() was not intercepted, so DeflateSettings::level was written only at init and the Z_NO_COMPRESSION gate in deflate() never saw a level set afterwards. A stream initialized at level 1-9 and then lowered to level 0 was silently compressed on every backend -- 428 (QAT), 1046 (IAA) and 85 (IGZIP) bytes for 64 KiB of input where zlib's contract requires 65552 stored bytes -- and identically so with use_zlib_compress=0, making it a silent contract violation rather than an error.

Intercept deflateParams() and record the level and strategy, gated on the call returning Z_OK: zlib documents the parameters as unchanged on Z_BUF_ERROR, which is the same gating deflateInit*() and both *SetDictionary() functions already use. The existing per-call gate then pins such a stream to zlib, so it reaches orig_deflate even when use_zlib_compress is 0 -- the request was never an offload candidate.

A stream ISA-L already owns has to be exempted from that pin. The gate runs before the IGZIP stickiness check, so pinning a stream ISA-L has emitted a header and compressed data for hands it to a zlib deflate state that was never fed, which writes a second header mid-stream: measured Z_DATA_ERROR with only the pre-switch bytes recoverable. Such a stream stays on IGZIP with the new level unhonored -- the same deliberate residual as deflate()'s Z_BLOCK -> Z_SYNC_FLUSH aliasing -- and its output remains valid deflate that round-trips.

deflateReset() deliberately keeps the recorded level, matching zlib, which documents reset as leaving the level and strategy unchanged.

gzsetparams() is a separate gap and is unchanged here: GzipFile carries no compression level at all.

[Copilot]

Pull request overview

Intercepts deflateParams() so compression-level changes affect backend selection.

Changes:

• Tracks accepted level and strategy updates.
• Preserves active IGZIP streams to avoid corruption.
• Adds regression tests and documents limitations.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File	Description
zlib_accel.cpp	Adds interception and routing logic.
tests/zlib_accel_test.cpp	Tests level changes across backends.
README.md	Documents behavior and exceptions.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.

Suppressed comments (2)

README.md:51

• This says there are two exceptions but presents three bullets, and the first IGZIP bullet already subsumes the second. Merge those two bullets so the exception count and documented behavior are unambiguous.

- `Z_NO_COMPRESSION` (level 0) streams are always handled by zlib. Level 0 requests stored, uncompressed deflate blocks, which none of the backends can produce — ISA-L's own level 0 is still LZ77+Huffman compression, and QAT and IAA take no compression level at all — so such streams are routed to zlib rather than being silently compressed. A level change made *after* initialization via `deflateParams` is observed as well, so a stream initialized at level 1-9 and later set to level 0 is routed to zlib from that point on. There are two exceptions:
  - A stream that has already started on IGZIP when the level changes stays on IGZIP for the rest of that stream, and the new level is not honored. `deflateReset` ends the stream, and the next stream on the same `z_stream` does use the new level.
  - A stream that has already started on IGZIP when the level drops to 0 stays on IGZIP, and the new level is not honored. ISA-L has already emitted a header and compressed data and still holds unflushed stream state at that point, so the stream cannot be moved to zlib without corrupting the output — the same constraint that makes `Z_BLOCK` alias to `Z_SYNC_FLUSH` mid-stream. The output remains valid deflate that round-trips correctly; it is compressed where the application asked for stored blocks.
  - `gzsetparams` is not intercepted, so a level set through the `gz*` API is not observed at all.

zlib_accel.cpp:430

• The new Z_OK gate is specifically intended to prevent a rejected deflateParams() change from affecting later path selection, but every added test makes the call succeed. Add a regression that forces Z_BUF_ERROR, resets the stream, and verifies the rejected level was not recorded (for example, that the next stream remains eligible for IGZIP).

  if (ret == Z_OK) {
    auto deflate_settings = deflate_stream_settings.Get(strm);
    if (deflate_settings != nullptr) {
      deflate_settings->level = level;
      deflate_settings->strategy = strategy;

[matt-welch]

Verdict: Approve with comment

One low-severity edge case identified (compression ratio fidelity only, no corruption). Core fix is correct and well-tested.

---

Summary

Intercepts deflateParams() to track post-init level changes, fixing a silent contract violation: a stream lowered to level 0 after init was compressed (428-1046 bytes) instead of stored (65552 bytes) on every backend. The fix gates recording on ret == Z_OK, exempts active IGZIP streams from the level-0 pin (can't hand mid-stream state to zlib), and discards the ISA-L stream in deflateReset() when the level no longer matches.

---

What's correct

• deflateParams interceptor: forwards to zlib first, records only on Z_OK — same pattern as deflateInit/SetDictionary
• IGZIP mid-stream exemption: igzip_owns_stream = (path == IGZIP && isal_strm != nullptr) — only true when ISA-L has actually been fed data; prevents second-header corruption
• deflateReset level-change detection: uses CompressLevelChangedIGZIP (shared MapCompressionLevelIGZIP ensures no disagreement with init); common no-change case keeps the 276-340 KB level_buf allocation
• Level mapping refactor: exact preservation of the existing mapping with better readability (Z_DEFAULT_COMPRESSION vs raw -1)
• Tests: strong oracles (stored output > input; byte-equality against fresh stream; non-vacuous assertion that levels differ)

---

Finding: stale ISA-L stream after deflateReset → deflateParams

Sequence: init(9) → deflate → reset → deflateParams(1) → deflate

After deflateReset, the level hasn't changed yet (still 9), so the ISA-L stream (level 3) is kept. Then deflateParams(1) updates the recorded level to 1 but doesn't discard the ISA-L stream. The next deflate() sees isal_strm != nullptr, skips InitCompressIGZIP, and uses the stale stream at ISA-L level 3 instead of rebuilding at level 1.

Impact: Compression ratio only — output is valid, round-trips. Self-corrects on next deflateReset().
Severity: Low. Requires the uncommon reset-then-params pattern (vs. the handled params-then-reset).

Suggested fix (for a follow-up): in deflateParams(), after recording the level, discard a non-active ISA-L stream whose level no longer matches:

#ifdef USE_IGZIP
if (deflate_settings->path != IGZIP &&
    deflate_settings->isal_strm != nullptr &&
    CompressLevelChangedIGZIP(deflate_settings->isal_strm, level)) {
    EndCompressIGZIP(deflate_settings->isal_strm);
    deflate_settings->isal_strm = nullptr;
}
#endif

The path != IGZIP guard preserves the mid-stream exemption.

---

Files changed

zlib_accel.cpp (+58/-2 lines)

• deflateParams interceptor (forward + record on Z_OK)
• IGZIP mid-stream exemption in level-0 gate
• deflateReset conditional discard of ISA-L stream

igzip.cpp (+39/-13 lines)

• MapCompressionLevelIGZIP() extracted from InitCompressIGZIP()
• CompressLevelChangedIGZIP() uses the shared mapping

igzip.h (+2 lines)

• VISIBLE_FOR_TESTING declaration of CompressLevelChangedIGZIP

tests/zlib_accel_test.cpp (+372 lines)

• 9 test cases: level-0 regression (3 backends), rejection gate (3 backends), legal-change stays offloadable, mid-stream exemption, reset-rebuilds-stream

README.md (+6/-3 lines)

• Documents the two exceptions (IGZIP mid-stream, gzsetparams)
• Adds deflateParams to intercepted functions list

[Copilot]

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.

Suppressed comments (1)

zlib_accel.cpp:335

• This predicate also treats a zero-progress path selection as ownership. On a fresh stream, an empty Z_SYNC_FLUSH allocates isal_strm, hits CompressIGZIP()'s ZSTATE_NEW_HDR fast path, and returns Z_BUF_ERROR without consuming or emitting anything, but deflate() still records path == IGZIP. A following deflateParams(..., Z_NO_COMPRESSION, ...) is therefore exempted from rebuilding/pinning and later input is compressed at the old level even though migration was still safe. Require actual ISA-L input/output progress before considering the stream owned.

static bool IgzipOwnsDeflateStream(
    const std::shared_ptr<DeflateSettings>& settings) {
  return settings != nullptr && settings->path == IGZIP &&
         settings->isal_strm != nullptr;