From 6a893904cef76c5676dc2a8a70ecf977a991a3e6 Mon Sep 17 00:00:00 2001 From: Olasoji Date: Fri, 20 Mar 2026 13:31:45 -0700 Subject: [PATCH 01/21] igzip: fix raw deflate over-consumption via read_in_length ISAL's stateful inflate pre-loads input in 8-byte word chunks into a 64-bit shift register (read_in). After reaching ISAL_BLOCK_FINISH for raw deflate (window_bits < 0), read_in_length still holds the bit count of bytes fetched beyond the true stream end. Shifting right by 3 gives the exact over-consumed byte count for every avail_in scenario: avail_in in [1,7] : previous avail_in<8 heuristic approximated this avail_in == 0 : previous heuristic was blind; now handled avail_in == 8 : gap in both old heuristics; now handled avail_in > 8 : multi-frame continuation; now handled The corrected consumed count is reported back to the caller via *input_length so that avail_in / next_in on the zlib stream are advanced accurately, preventing downstream compressed-size and CRC mismatches (reproduces as ZipException in Java ZipInputStream). Boundary guard updated: skip the raw-boundary fallback when *tofixed==1 because the correction has already accounted for any remaining bytes, so non-zero remaining is legitimate trailing data rather than un-corrected over-consumption. BLOCK_INPUT_DONE path (output-buffer-limited mid-stream) retains the existing Z_DATA_ERROR->zlib-fallback behaviour unchanged. Previously attempted stateless-probe approach removed entirely; the read_in_length field is simpler, exact, and has no multi-frame failure mode. Validated: 8/8 IGZIPInflateRegressionTest pass; full make run passes with only the pre-existing ordering-sensitive case 37961; JAR repro (250 entries, scan + build-init) passes clean. --- igzip.cpp | 64 ++++++++++++++++++++++++++++++++----------------------- 1 file changed, 37 insertions(+), 27 deletions(-) diff --git a/igzip.cpp b/igzip.cpp index 954dbd7..7ec6630 100644 --- a/igzip.cpp +++ b/igzip.cpp @@ -435,11 +435,11 @@ IGZIPInflatePathAction IGZIPRunInflateAndSelectPathAction( const uint32_t remaining_after_igzip = (pre_avail_in >= *input_length) ? (pre_avail_in - *input_length) : 0; - if (*ret == 0 && window_bits < 0 && *end_of_stream && - remaining_after_igzip > 0 && strm->total_in == 0 && - strm->total_out == 0) { + if (*ret == 0 && window_bits < 0 && *end_of_stream && + remaining_after_igzip > 0 && *tofixed == 0 && + strm->total_in == 0 && strm->total_out == 0) { Log(LogLevel::LOG_ERROR, - "IGZIPRunInflateAndSelectPathAction() raw boundary guard strm=", + "IGZIPRunInflateAndSelectPathAction() raw boundary guard FIRED strm=", static_cast(strm), " bytes_in=", *input_length, " bytes_out=", *output_length, " pre_avail_in=", pre_avail_in, " remaining_in=", remaining_after_igzip, "\n"); @@ -497,31 +497,41 @@ int UncompressIGZIP(struct inflate_state *isal_strm_inflate, uint8_t *input, uint32_t rewind_adjust_bytes = 0; - // WORKAROUND: ISA-L over-consumption fix for raw deflate mode. - // Option 2 behavior: if ambiguity appears at INPUT_DONE, request caller - // fallback to zlib instead of carrying deferred state. + // WORKAROUND: ISA-L raw-deflate over-consumption fix. + // ISAL pre-loads input in 8-byte word chunks into a 64-bit shift register + // (read_in). After BLOCK_FINISH, read_in_length >> 3 is the exact byte + // count over-consumed, covering all avail_in scenarios: [0], [1,7], [8], + // and >8 (multi-frame), where prior heuristics were blind or inaccurate. + if (window_bits < 0 && + (decomp == ISAL_DECOMP_OK || decomp == ISAL_END_INPUT) && + isal_strm_inflate->block_state == ISAL_BLOCK_FINISH) { + const uint32_t read_in_correction = + (isal_strm_inflate->read_in_length > 0) + ? static_cast(isal_strm_inflate->read_in_length >> 3) + : 0u; + Log(LogLevel::LOG_INFO, "UncompressIGZIP() Line ", __LINE__, + " raw_finish avail_in ", isal_strm_inflate->avail_in, + " read_in_length_bits ", isal_strm_inflate->read_in_length, + " read_in_correction_bytes ", read_in_correction, "\n"); + if (read_in_correction > 0) { + rewind_adjust_bytes = (read_in_correction <= consumed_before_adjust) + ? read_in_correction + : consumed_before_adjust; + *tofixed = 1; + } + } + + // WORKAROUND: BLOCK_INPUT_DONE — output-buffer-limited with ambiguous + // trailer bytes. read_in_length does not apply here (not yet at BLOCK_FINISH); + // request caller fallback to zlib. BLOCK_FINISH is fully handled above. if (window_bits < 0 && decomp == ISAL_DECOMP_OK && *tofixed == 0 && - (isal_strm_inflate->block_state == ISAL_BLOCK_FINISH || - isal_strm_inflate->block_state == ISAL_BLOCK_INPUT_DONE) && + isal_strm_inflate->block_state == ISAL_BLOCK_INPUT_DONE && isal_strm_inflate->avail_in < 8 && isal_strm_inflate->avail_in > 0) { - const uint32_t expected_trailer_bytes = 8; - const uint32_t over_consumed = - expected_trailer_bytes - isal_strm_inflate->avail_in; - if (over_consumed >= 1 && over_consumed <= 7) { - if (isal_strm_inflate->block_state == ISAL_BLOCK_FINISH) { - rewind_adjust_bytes = consumed_before_adjust < over_consumed - ? consumed_before_adjust - : over_consumed; - if (rewind_adjust_bytes > 0) { - *tofixed = 1; - } - } else { - Log(LogLevel::LOG_INFO, "UncompressIGZIP() Line ", __LINE__, - " raw INPUT_DONE ambiguity detected: over_consumed ", over_consumed, - ", requesting zlib fallback\n"); - return Z_DATA_ERROR; - } - } + const uint32_t over_consumed = 8u - isal_strm_inflate->avail_in; + Log(LogLevel::LOG_INFO, "UncompressIGZIP() Line ", __LINE__, + " raw INPUT_DONE ambiguity detected: over_consumed ", over_consumed, + ", requesting zlib fallback\n"); + return Z_DATA_ERROR; } *output_length = original_avail_out - isal_strm_inflate->avail_out; From 93f144e3128c826fb7d8c97da306e625f5012f17 Mon Sep 17 00:00:00 2001 From: Olasoji Date: Fri, 20 Mar 2026 14:03:06 -0700 Subject: [PATCH 02/21] rename trailer_overconsumption_fixed -> read_in_correction_applied; remove stale post-reset pre-set The old name 'trailer_overconsumption_fixed' was a holdover from the avail_in < 8 heuristic era. Since EXP-6f the fix is driven entirely by isal_inflate.read_in_length >> 3; the flag only signals that a correction was applied *within the current inflate call*. Rename the field, its parameter aliases, and all local variables to read_in_correction_applied to match the actual semantics. Also remove the stale block in inflateReset: if (was_igzip_path) { inflate_settings->trailer_overconsumption_fixed = 1; } This block predated the read_in_length fix. It pre-armed the flag so that the old avail_in < 8 boundary guard would not fire on the first call after a reset. That logic was correct under the old heuristic but is actively wrong now: ResetUncompressIGZIP already zeros the flag; re-arming it to 1 would suppress the boundary guard on exactly the call where it is needed most (first call after reset on a raw stream). Remove was_igzip_path (now unused after the block removal) and inline the INPUT_DONE log expression to eliminate an unused-variable warning when DEBUG_LOG=OFF. No behaviour change for correct streams; test suite: 8/8 IGZIP inflate regressions PASS, only pre-existing 37961 flaky failure remains. --- igzip.cpp | 38 +++++++++++++++++++++----------------- igzip.h | 17 ++++++++++------- zlib_accel.cpp | 25 ++++++++++--------------- 3 files changed, 41 insertions(+), 39 deletions(-) diff --git a/igzip.cpp b/igzip.cpp index 7ec6630..24b93d8 100644 --- a/igzip.cpp +++ b/igzip.cpp @@ -360,7 +360,7 @@ struct inflate_state *InitUncompressIGZIP(int windowBits) { // strm->total_out = 0; // strm->total_in = 0; - // s->trailer_overconsumption_fixed = 0; // Initialize the workaround flag + // s->read_in_correction_applied = 0; ConfigureInflateWindow(isal_strm_inflate, windowBits); @@ -369,9 +369,9 @@ struct inflate_state *InitUncompressIGZIP(int windowBits) { IGZIPNoInputAction IGZIPHandleActiveStreamNoInput( z_streamp strm, struct inflate_state *isal_strm_inflate, int window_bits, - int *tofixed, int *ret) { + int *read_in_correction_applied, int *ret) { if (strm == nullptr || isal_strm_inflate == nullptr || ret == nullptr || - tofixed == nullptr || strm->avail_in != 0) { + read_in_correction_applied == nullptr || strm->avail_in != 0) { return IGZIP_NO_INPUT_NOT_HANDLED; } @@ -380,7 +380,8 @@ IGZIPNoInputAction IGZIPHandleActiveStreamNoInput( bool end_of_stream = true; *ret = UncompressIGZIP(isal_strm_inflate, strm->next_in, &input_len, - strm->next_out, &output_len, window_bits, tofixed, + strm->next_out, &output_len, window_bits, + read_in_correction_applied, &strm->total_in, &strm->total_out, &end_of_stream); if (*ret == Z_DATA_ERROR) { @@ -407,11 +408,12 @@ IGZIPNoInputAction IGZIPHandleActiveStreamNoInput( IGZIPInflatePathAction IGZIPRunInflateAndSelectPathAction( z_streamp strm, struct inflate_state **isal_strm_inflate, int window_bits, - int *tofixed, uint32_t *input_length, uint32_t *output_length, int *ret, - bool *end_of_stream, uint32_t pre_avail_in) { + int *read_in_correction_applied, uint32_t *input_length, + uint32_t *output_length, int *ret, bool *end_of_stream, + uint32_t pre_avail_in) { if (strm == nullptr || isal_strm_inflate == nullptr || input_length == nullptr || output_length == nullptr || ret == nullptr || - end_of_stream == nullptr || tofixed == nullptr) { + end_of_stream == nullptr || read_in_correction_applied == nullptr) { if (ret != nullptr) { *ret = Z_DATA_ERROR; } @@ -429,14 +431,15 @@ IGZIPInflatePathAction IGZIPRunInflateAndSelectPathAction( } *ret = UncompressIGZIP(*isal_strm_inflate, strm->next_in, input_length, - strm->next_out, output_length, window_bits, tofixed, + strm->next_out, output_length, window_bits, + read_in_correction_applied, &strm->total_in, &strm->total_out, end_of_stream); const uint32_t remaining_after_igzip = (pre_avail_in >= *input_length) ? (pre_avail_in - *input_length) : 0; if (*ret == 0 && window_bits < 0 && *end_of_stream && - remaining_after_igzip > 0 && *tofixed == 0 && + remaining_after_igzip > 0 && *read_in_correction_applied == 0 && strm->total_in == 0 && strm->total_out == 0) { Log(LogLevel::LOG_ERROR, "IGZIPRunInflateAndSelectPathAction() raw boundary guard FIRED strm=", @@ -463,7 +466,8 @@ IGZIPInflatePathAction IGZIPRunInflateAndSelectPathAction( int UncompressIGZIP(struct inflate_state *isal_strm_inflate, uint8_t *input, uint32_t *input_length, uint8_t *output, - uint32_t *output_length, int window_bits, int *tofixed, + uint32_t *output_length, int window_bits, + int *read_in_correction_applied, unsigned long *total_in, unsigned long *total_out, bool *end_of_stream) { (void)total_in; @@ -517,19 +521,20 @@ int UncompressIGZIP(struct inflate_state *isal_strm_inflate, uint8_t *input, rewind_adjust_bytes = (read_in_correction <= consumed_before_adjust) ? read_in_correction : consumed_before_adjust; - *tofixed = 1; + *read_in_correction_applied = 1; } } // WORKAROUND: BLOCK_INPUT_DONE — output-buffer-limited with ambiguous // trailer bytes. read_in_length does not apply here (not yet at BLOCK_FINISH); // request caller fallback to zlib. BLOCK_FINISH is fully handled above. - if (window_bits < 0 && decomp == ISAL_DECOMP_OK && *tofixed == 0 && + if (window_bits < 0 && decomp == ISAL_DECOMP_OK && + *read_in_correction_applied == 0 && isal_strm_inflate->block_state == ISAL_BLOCK_INPUT_DONE && isal_strm_inflate->avail_in < 8 && isal_strm_inflate->avail_in > 0) { - const uint32_t over_consumed = 8u - isal_strm_inflate->avail_in; Log(LogLevel::LOG_INFO, "UncompressIGZIP() Line ", __LINE__, - " raw INPUT_DONE ambiguity detected: over_consumed ", over_consumed, + " raw INPUT_DONE ambiguity detected: over_consumed ", + 8u - isal_strm_inflate->avail_in, ", requesting zlib fallback\n"); return Z_DATA_ERROR; } @@ -621,7 +626,7 @@ int inflateSetDictionary(z_streamp strm, unsigned char *dict_data, } int ResetUncompressIGZIP(struct inflate_state *isal_strm_inflate, - int *tofixed) { + int *read_in_correction_applied) { if (!isal_strm_inflate) { Log(LogLevel::LOG_ERROR, "ResetUncompressIGZIP() Line ", __LINE__, " isal_strm_inflate is NULL\n"); @@ -631,8 +636,7 @@ int ResetUncompressIGZIP(struct inflate_state *isal_strm_inflate, // Reset ISA-L inflate state isal_inflate_reset(isal_strm_inflate); - // Reset workaround flag - *tofixed = 0; + *read_in_correction_applied = 0; return Z_OK; } diff --git a/igzip.h b/igzip.h index 0317907..db65d79 100644 --- a/igzip.h +++ b/igzip.h @@ -12,8 +12,8 @@ typedef struct internal_state2 { int level; int w_bits; struct inflate_state *isal_strm_inflate; - int trailer_overconsumption_fixed; /* Indicates if fix has been applied for - gzip trailer overconsumption issue */ + int read_in_correction_applied; /* Set when read_in_length correction was + applied in the current inflate call */ } inflate_state2; typedef struct internal_state { @@ -50,12 +50,13 @@ enum IGZIPInflatePathAction { IGZIPNoInputAction IGZIPHandleActiveStreamNoInput( z_streamp strm, struct inflate_state *isal_strm_inflate, int window_bits, - int *tofixed, int *ret); + int *read_in_correction_applied, int *ret); IGZIPInflatePathAction IGZIPRunInflateAndSelectPathAction( z_streamp strm, struct inflate_state **isal_strm_inflate, int window_bits, - int *tofixed, uint32_t *input_length, uint32_t *output_length, int *ret, - bool *end_of_stream, uint32_t pre_avail_in); + int *read_in_correction_applied, uint32_t *input_length, + uint32_t *output_length, int *ret, bool *end_of_stream, + uint32_t pre_avail_in); bool IGZIPShouldFallbackDeflate(bool stream_on_igzip_path, int flush, uint32_t avail_in); @@ -64,10 +65,12 @@ int EndCompressIGZIP(struct isal_zstream *isal_strm); struct inflate_state *InitUncompressIGZIP(int windowBits); int UncompressIGZIP(struct inflate_state *isal_strm_inflate, uint8_t *input, uint32_t *input_length, uint8_t *output, - uint32_t *output_length, int window_bits, int *tofixed, + uint32_t *output_length, int window_bits, + int *read_in_correction_applied, unsigned long *total_in, unsigned long *total_out, bool *end_of_stream); int EndUncompressIGZIP(struct inflate_state *isal_strm_inflate); -int ResetUncompressIGZIP(struct inflate_state *isal_strm_inflate, int *tofixed); +int ResetUncompressIGZIP(struct inflate_state *isal_strm_inflate, + int *read_in_correction_applied); // #define Z_DEFAULT_COMPRESSION 6 #endif diff --git a/zlib_accel.cpp b/zlib_accel.cpp index 82b55c5..399ab35 100644 --- a/zlib_accel.cpp +++ b/zlib_accel.cpp @@ -203,10 +203,10 @@ struct DeflateSettings { struct InflateSettings { InflateSettings(int _window_bits) - : window_bits(_window_bits), trailer_overconsumption_fixed(0) {} + : window_bits(_window_bits), read_in_correction_applied(0) {} int window_bits; - int trailer_overconsumption_fixed; /* indicates if fix has been applied for - overconsumption issue*/ + int read_in_correction_applied; /* set when read_in_length correction was + applied in the current inflate call */ ExecutionPath path = UNDEFINED; struct inflate_state* isal_strm = nullptr; }; @@ -596,7 +596,7 @@ int ZEXPORT inflate(z_streamp strm, int flush) { in_call = true; IGZIPNoInputAction action = IGZIPHandleActiveStreamNoInput( strm, inflate_settings->isal_strm, inflate_settings->window_bits, - &inflate_settings->trailer_overconsumption_fixed, &ret); + &inflate_settings->read_in_correction_applied, &ret); in_call = false; if (action == IGZIP_NO_INPUT_FALLBACK_ZLIB) { @@ -700,7 +700,7 @@ int ZEXPORT inflate(z_streamp strm, int flush) { const IGZIPInflatePathAction path_action = IGZIPRunInflateAndSelectPathAction( strm, &inflate_settings->isal_strm, inflate_settings->window_bits, - &inflate_settings->trailer_overconsumption_fixed, &input_len, + &inflate_settings->read_in_correction_applied, &input_len, &output_len, &ret, &end_of_stream, pre_avail_in); in_call = false; @@ -803,8 +803,6 @@ int ZEXPORT inflateReset(z_streamp strm) { Log(LogLevel::LOG_INFO, "inflateReset Line ", __LINE__, ", strm ", static_cast(strm), "\n"); InflateSettings* inflate_settings = inflate_stream_settings.Get(strm); - const bool was_igzip_path = - (inflate_settings != nullptr && inflate_settings->path == IGZIP); int ret = orig_inflateReset(strm); if (inflate_settings != nullptr) { SetInflatePath(inflate_settings, strm, UNDEFINED, "inflateReset"); @@ -812,11 +810,8 @@ int ZEXPORT inflateReset(z_streamp strm) { if (inflate_settings->isal_strm != nullptr) { #ifdef USE_IGZIP ResetUncompressIGZIP(inflate_settings->isal_strm, - &inflate_settings->trailer_overconsumption_fixed); + &inflate_settings->read_in_correction_applied); #endif - if (was_igzip_path) { - inflate_settings->trailer_overconsumption_fixed = 1; - } } return ret; @@ -981,11 +976,11 @@ int ZEXPORT uncompress2(Bytef* dest, uLongf* destLen, const Bytef* source, if (isal_strm == nullptr) { ret = 1; } else { - int tofixed = 0; + int read_in_correction_applied = 0; unsigned long total_in = 0; unsigned long total_out = 0; ret = UncompressIGZIP(isal_strm, const_cast(source), &input_len, - dest, &output_len, 15, &tofixed, &total_in, + dest, &output_len, 15, &read_in_correction_applied, &total_in, &total_out, &end_of_stream); EndUncompressIGZIP(isal_strm); if (ret == 0 && !end_of_stream) { @@ -1358,12 +1353,12 @@ static int GzreadAcceleratorUncompress(GzipFile* gz, uint8_t* input, if (isal_strm == nullptr) { ret = 1; } else { - int tofixed = 0; + int read_in_correction_applied = 0; unsigned long total_in = 0; unsigned long total_out = 0; ret = UncompressIGZIP(isal_strm, input, input_length, output, output_length, - 31, &tofixed, &total_in, &total_out, end_of_stream); + 31, &read_in_correction_applied, &total_in, &total_out, end_of_stream); EndUncompressIGZIP(isal_strm); } gz->path = IGZIP; From 92eaf59ac18c3baddad9a12632a56572b9b343da Mon Sep 17 00:00:00 2001 From: Olasoji Date: Fri, 20 Mar 2026 14:32:12 -0700 Subject: [PATCH 03/21] add IGZIP inflate/deflate counters to statistics; enable ENABLE_STATISTICS build flag MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Statistics previously tracked QAT and IAA paths but left IGZIP with commented-out placeholders. Two problems with those placeholders: the enum values DEFLATE_IGZIP_COUNT, DEFLATE_IGZIP_ERROR_COUNT, INFLATE_IGZIP_COUNT, INFLATE_IGZIP_ERROR_COUNT did not exist, and the stat_names array had no entries for them. Add the four missing Statistic enum values (after the IAA entries, before ZLIB, matching the QAT/IAA grouping pattern) and their corresponding stat_names strings. Uncomment the INCREMENT_STAT calls in the deflate and inflate IGZIP dispatch blocks. The error condition mirrors IAA/QAT: unconditional IGZIP_COUNT on every dispatch, and IGZIP_ERROR_COUNT when ret != 0 (fallback or hard error). Enable ENABLE_STATISTICS=ON in cmake.txt so the counters compile in by default. Validation (OpenSearch opensearch-core-3.3.1.jar, 250-entry JAR repro, use_igzip_uncompress=1, log_stats_samples=50): Thread (main): inflate_count=750 igzip=749 igzip_errors=0 zlib=0 — 99.9% of inflate calls served by IGZIP with zero errors or zlib fallbacks; one call is the initial probe before path selection. --- cmake.txt | 1 + statistics.cpp | 4 +++- statistics.h | 4 ++++ zlib_accel.cpp | 8 ++++---- 4 files changed, 12 insertions(+), 5 deletions(-) create mode 100644 cmake.txt diff --git a/cmake.txt b/cmake.txt new file mode 100644 index 0000000..e200c75 --- /dev/null +++ b/cmake.txt @@ -0,0 +1 @@ +cmake -DUSE_QAT=ON -DUSE_IAA=ON -DUSE_IGZIP=ON -DQPL_PATH=/home/sdp/qpl -DISAL_PATH=/home/sdp/isa-l/ -DDEBUG_LOG=OFF -DENABLE_STATISTICS=ON -DCMAKE_BUILD_TYPE=Release .. diff --git a/statistics.cpp b/statistics.cpp index 9dabeb6..179024b 100644 --- a/statistics.cpp +++ b/statistics.cpp @@ -18,9 +18,11 @@ using namespace config; const std::array stat_names{ {"deflate_count", "deflate_error_count", "deflate_qat_count", "deflate_qat_error_count", "deflate_iaa_count", "deflate_iaa_error_count", + "deflate_igzip_count", "deflate_igzip_error_count", "deflate_zlib_count", "inflate_count", "inflate_error_count", "inflate_qat_count", "inflate_qat_error_count", "inflate_iaa_count", - "inflate_iaa_error_count", "inflate_zlib_count"}}; + "inflate_iaa_error_count", "inflate_igzip_count", + "inflate_igzip_error_count", "inflate_zlib_count"}}; thread_local std::array stats{}; diff --git a/statistics.h b/statistics.h index 8cee404..b3654ce 100644 --- a/statistics.h +++ b/statistics.h @@ -17,6 +17,8 @@ enum class Statistic : size_t { DEFLATE_QAT_ERROR_COUNT, DEFLATE_IAA_COUNT, DEFLATE_IAA_ERROR_COUNT, + DEFLATE_IGZIP_COUNT, + DEFLATE_IGZIP_ERROR_COUNT, DEFLATE_ZLIB_COUNT, INFLATE_COUNT, INFLATE_ERROR_COUNT, @@ -24,6 +26,8 @@ enum class Statistic : size_t { INFLATE_QAT_ERROR_COUNT, INFLATE_IAA_COUNT, INFLATE_IAA_ERROR_COUNT, + INFLATE_IGZIP_COUNT, + INFLATE_IGZIP_ERROR_COUNT, INFLATE_ZLIB_COUNT, STATS_COUNT }; diff --git a/zlib_accel.cpp b/zlib_accel.cpp index 399ab35..f654196 100644 --- a/zlib_accel.cpp +++ b/zlib_accel.cpp @@ -419,8 +419,8 @@ int ZEXPORT deflate(z_streamp strm, int flush) { "selected IGZIP accelerator"); in_call = false; - // INCREMENT_STAT(DEFLATE_IGZIP_COUNT); - // INCREMENT_STAT_COND(ret != 0, DEFLATE_IGZIP_ERROR_COUNT); + INCREMENT_STAT(DEFLATE_IGZIP_COUNT); + INCREMENT_STAT_COND(ret != 0, DEFLATE_IGZIP_ERROR_COUNT); #endif } @@ -725,8 +725,8 @@ int ZEXPORT inflate(z_streamp strm, int flush) { SetInflatePath(inflate_settings, strm, IGZIP, "selected IGZIP accelerator"); } - // INCREMENT_STAT(INFLATE_IGZIP_COUNT); - // INCREMENT_STAT_COND(ret != 0, INFLATE_IGZIP_ERROR_COUNT); + INCREMENT_STAT(INFLATE_IGZIP_COUNT); + INCREMENT_STAT_COND(ret != 0, INFLATE_IGZIP_ERROR_COUNT); #endif } From b48d1181c9cecb9a0dbcbc9030961ab4cb5c3e9b Mon Sep 17 00:00:00 2001 From: Olasoji Date: Fri, 20 Mar 2026 14:59:35 -0700 Subject: [PATCH 04/21] add iaa_fallback_igzip: IAA can fall back to IGZIP before software zlib When IAA inflate or deflate fails (ret != 0), and use_igzip_uncompress / use_igzip_compress is enabled, the new iaa_fallback_igzip config flag (default 0) routes the retry to IGZIP before allowing the call to fall through to software zlib. Motivation: IAA is stateless and leaves input unconsumed on failure, so IGZIP can retry from the same position at no extra cost. This keeps hardware-accelerated decompression in play for inputs that IAA rejects but IGZIP can handle (e.g. streams that fail IsIAADecompressible or exceed IAA's single-call constraint). Implementation: - New ConfigOption IAA_FALLBACK_IGZIP added between IGNORE_ZLIB_DICTIONARY and LOG_LEVEL; default 0 (opt-in), range [0,1]. - inflate(): after IAA dispatch block, if path_selected==IAA && ret!=0 && configs[IAA_FALLBACK_IGZIP] && igzip_available, reset input_len/output_len (IAA may have modified them on failure), clear read_in_correction_applied, and run IGZIPRunInflateAndSelectPathAction with the same path-action handling as the normal IGZIP dispatch. Falls through to zlib if IGZIP also fails. - deflate(): identical pattern; initialises isal_strm via InitCompressIGZIP if needed (matching normal IGZIP deflate path). - Both fallback blocks are wrapped in #ifdef USE_IGZIP and guarded by igzip_available so they compile away and are unreachable without IGZIP. - Existing INFLATE/DEFLATE_IGZIP_COUNT stats capture fallback calls. Test suite: 44756/44757 PASS (37961 pre-existing flaky only). --- config/config.cpp | 3 +++ config/config.h | 1 + zlib_accel.cpp | 69 +++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 73 insertions(+) diff --git a/config/config.cpp b/config/config.cpp index 51a30af..3fa6001 100644 --- a/config/config.cpp +++ b/config/config.cpp @@ -30,6 +30,7 @@ uint32_t configs[CONFIG_MAX] = { 1, /*qat_compression_level*/ 0, /*qat_compression_allow_chunking*/ 0, /*ignore_zlib_dictionary*/ + 0, /*iaa_fallback_igzip*/ 2, /*log_level*/ 1000 /*log_stats_samples*/ }; @@ -56,6 +57,7 @@ bool LoadConfigFile(std::string& file_content, const char* file_path) { "qat_compression_level", "qat_compression_allow_chunking", "ignore_zlib_dictionary", + "iaa_fallback_igzip", "log_level", "log_stats_samples" }; @@ -91,6 +93,7 @@ bool LoadConfigFile(std::string& file_content, const char* file_path) { trySetConfig(QAT_COMPRESSION_LEVEL, 9, 1); trySetConfig(QAT_COMPRESSION_ALLOW_CHUNKING, 1, 0); trySetConfig(IGNORE_ZLIB_DICTIONARY, 1, 0); + trySetConfig(IAA_FALLBACK_IGZIP, 1, 0); trySetConfig(LOG_LEVEL, 2, 0); trySetConfig(LOG_STATS_SAMPLES, UINT32_MAX, 0); diff --git a/config/config.h b/config/config.h index 12d3563..80f8807 100644 --- a/config/config.h +++ b/config/config.h @@ -25,6 +25,7 @@ enum ConfigOption { QAT_COMPRESSION_LEVEL, QAT_COMPRESSION_ALLOW_CHUNKING, IGNORE_ZLIB_DICTIONARY, + IAA_FALLBACK_IGZIP, LOG_LEVEL, LOG_STATS_SAMPLES, CONFIG_MAX diff --git a/zlib_accel.cpp b/zlib_accel.cpp index f654196..90d2915 100644 --- a/zlib_accel.cpp +++ b/zlib_accel.cpp @@ -424,6 +424,31 @@ int ZEXPORT deflate(z_streamp strm, int flush) { #endif } +#ifdef USE_IGZIP + // IAA→IGZIP fallback: if IAA failed and IGZIP is available, retry with + // IGZIP before falling through to software zlib. + if (path_selected == IAA && ret != 0 && + configs[IAA_FALLBACK_IGZIP] && igzip_available) { + // IAA may have modified input_len/output_len on failure — restore them. + input_len = strm->avail_in; + output_len = strm->avail_out; + if (deflate_settings->isal_strm == nullptr) { + deflate_settings->method = 0; + deflate_settings->isal_strm = InitCompressIGZIP( + deflate_settings->level, deflate_settings->window_bits); + } + in_call = true; + ret = CompressIGZIP(deflate_settings->isal_strm, flush, strm->next_in, + &input_len, strm->next_out, &output_len, + &strm->total_in, &strm->total_out); + SetDeflatePath(deflate_settings, strm, IGZIP, + "IAA failed, IGZIP fallback"); + in_call = false; + INCREMENT_STAT(DEFLATE_IGZIP_COUNT); + INCREMENT_STAT_COND(ret != 0, DEFLATE_IGZIP_ERROR_COUNT); + } +#endif // USE_IGZIP iaa fallback + if (ret == 0) { strm->next_in += input_len; strm->avail_in -= input_len; @@ -730,6 +755,50 @@ int ZEXPORT inflate(z_streamp strm, int flush) { #endif } +#ifdef USE_IGZIP + // IAA→IGZIP fallback: if IAA failed and IGZIP is available, retry with + // IGZIP before falling through to software zlib. + if (path_selected == IAA && ret != 0 && + configs[IAA_FALLBACK_IGZIP] && igzip_available) { + // IAA may have modified input_len/output_len on failure — restore them. + input_len = strm->avail_in; + output_len = strm->avail_out; + end_of_stream = true; + in_call = true; + inflate_settings->read_in_correction_applied = 0; + const IGZIPInflatePathAction path_action = + IGZIPRunInflateAndSelectPathAction( + strm, &inflate_settings->isal_strm, inflate_settings->window_bits, + &inflate_settings->read_in_correction_applied, &input_len, + &output_len, &ret, &end_of_stream, pre_avail_in); + in_call = false; + + if (inflate_settings->isal_strm == nullptr) { + return Z_DATA_ERROR; + } + + if (path_action == IGZIP_INFLATE_PATH_FALLBACK_NEED_DICT) { + Log(LogLevel::LOG_ERROR, " strm=", static_cast(strm), + " source=igzip (iaa fallback)", " total_in=", strm->total_in, + " total_out=", strm->total_out, " adler=", strm->adler, "\n"); + SetInflatePath(inflate_settings, strm, ZLIB, + "IAA->IGZIP fallback: Z_NEED_DICT"); + } else if (path_action == IGZIP_INFLATE_PATH_FALLBACK_DATA_ERROR) { + SetInflatePath(inflate_settings, strm, ZLIB, + "IAA->IGZIP fallback: raw trailer"); + } else if (path_action == IGZIP_INFLATE_PATH_FALLBACK_RAW_BOUNDARY) { + SetInflatePath(inflate_settings, strm, ZLIB, + "IAA->IGZIP fallback: raw boundary"); + } else if (path_action == IGZIP_INFLATE_PATH_SET_IGZIP && + inflate_settings->path != ZLIB) { + SetInflatePath(inflate_settings, strm, IGZIP, + "IAA failed, IGZIP fallback succeeded"); + } + INCREMENT_STAT(INFLATE_IGZIP_COUNT); + INCREMENT_STAT_COND(ret != 0, INFLATE_IGZIP_ERROR_COUNT); + } +#endif // USE_IGZIP iaa fallback + if (ret == 0) { strm->next_in += input_len; strm->avail_in -= input_len; From 0a857e65db8aa89645f57c61f3130818fd2a1296 Mon Sep 17 00:00:00 2001 From: Olasoji Date: Fri, 20 Mar 2026 15:33:39 -0700 Subject: [PATCH 05/21] cmake: set ENABLE_STATISTICS=OFF by default --- cmake.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmake.txt b/cmake.txt index e200c75..2e45380 100644 --- a/cmake.txt +++ b/cmake.txt @@ -1 +1 @@ -cmake -DUSE_QAT=ON -DUSE_IAA=ON -DUSE_IGZIP=ON -DQPL_PATH=/home/sdp/qpl -DISAL_PATH=/home/sdp/isa-l/ -DDEBUG_LOG=OFF -DENABLE_STATISTICS=ON -DCMAKE_BUILD_TYPE=Release .. +cmake -DUSE_QAT=ON -DUSE_IAA=ON -DUSE_IGZIP=ON -DQPL_PATH=/home/sdp/qpl -DISAL_PATH=/home/sdp/isa-l/ -DDEBUG_LOG=OFF -DENABLE_STATISTICS=OFF -DCMAKE_BUILD_TYPE=Release .. From 1971197f5d08121df0cdd634726138f341841d76 Mon Sep 17 00:00:00 2001 From: Olasoji Date: Fri, 20 Mar 2026 15:34:23 -0700 Subject: [PATCH 06/21] - Reset statistics to off by default and formatting Signed-off-by: Olasoji --- igzip.cpp | 23 +++++++++++------------ igzip.h | 5 ++--- statistics.cpp | 9 ++++----- zlib_accel.cpp | 18 +++++++++--------- 4 files changed, 26 insertions(+), 29 deletions(-) diff --git a/igzip.cpp b/igzip.cpp index 24b93d8..a9fa9d3 100644 --- a/igzip.cpp +++ b/igzip.cpp @@ -381,8 +381,8 @@ IGZIPNoInputAction IGZIPHandleActiveStreamNoInput( *ret = UncompressIGZIP(isal_strm_inflate, strm->next_in, &input_len, strm->next_out, &output_len, window_bits, - read_in_correction_applied, - &strm->total_in, &strm->total_out, &end_of_stream); + read_in_correction_applied, &strm->total_in, + &strm->total_out, &end_of_stream); if (*ret == Z_DATA_ERROR) { Log(LogLevel::LOG_INFO, "IGZIPHandleActiveStreamNoInput() Line ", __LINE__, @@ -432,13 +432,13 @@ IGZIPInflatePathAction IGZIPRunInflateAndSelectPathAction( *ret = UncompressIGZIP(*isal_strm_inflate, strm->next_in, input_length, strm->next_out, output_length, window_bits, - read_in_correction_applied, - &strm->total_in, &strm->total_out, end_of_stream); + read_in_correction_applied, &strm->total_in, + &strm->total_out, end_of_stream); const uint32_t remaining_after_igzip = (pre_avail_in >= *input_length) ? (pre_avail_in - *input_length) : 0; - if (*ret == 0 && window_bits < 0 && *end_of_stream && + if (*ret == 0 && window_bits < 0 && *end_of_stream && remaining_after_igzip > 0 && *read_in_correction_applied == 0 && strm->total_in == 0 && strm->total_out == 0) { Log(LogLevel::LOG_ERROR, @@ -467,9 +467,8 @@ IGZIPInflatePathAction IGZIPRunInflateAndSelectPathAction( int UncompressIGZIP(struct inflate_state *isal_strm_inflate, uint8_t *input, uint32_t *input_length, uint8_t *output, uint32_t *output_length, int window_bits, - int *read_in_correction_applied, - unsigned long *total_in, unsigned long *total_out, - bool *end_of_stream) { + int *read_in_correction_applied, unsigned long *total_in, + unsigned long *total_out, bool *end_of_stream) { (void)total_in; if (!isal_strm_inflate) { @@ -526,16 +525,16 @@ int UncompressIGZIP(struct inflate_state *isal_strm_inflate, uint8_t *input, } // WORKAROUND: BLOCK_INPUT_DONE — output-buffer-limited with ambiguous - // trailer bytes. read_in_length does not apply here (not yet at BLOCK_FINISH); - // request caller fallback to zlib. BLOCK_FINISH is fully handled above. + // trailer bytes. read_in_length does not apply here (not yet at + // BLOCK_FINISH); request caller fallback to zlib. BLOCK_FINISH is fully + // handled above. if (window_bits < 0 && decomp == ISAL_DECOMP_OK && *read_in_correction_applied == 0 && isal_strm_inflate->block_state == ISAL_BLOCK_INPUT_DONE && isal_strm_inflate->avail_in < 8 && isal_strm_inflate->avail_in > 0) { Log(LogLevel::LOG_INFO, "UncompressIGZIP() Line ", __LINE__, " raw INPUT_DONE ambiguity detected: over_consumed ", - 8u - isal_strm_inflate->avail_in, - ", requesting zlib fallback\n"); + 8u - isal_strm_inflate->avail_in, ", requesting zlib fallback\n"); return Z_DATA_ERROR; } diff --git a/igzip.h b/igzip.h index db65d79..0f61298 100644 --- a/igzip.h +++ b/igzip.h @@ -66,9 +66,8 @@ struct inflate_state *InitUncompressIGZIP(int windowBits); int UncompressIGZIP(struct inflate_state *isal_strm_inflate, uint8_t *input, uint32_t *input_length, uint8_t *output, uint32_t *output_length, int window_bits, - int *read_in_correction_applied, - unsigned long *total_in, unsigned long *total_out, - bool *end_of_stream); + int *read_in_correction_applied, unsigned long *total_in, + unsigned long *total_out, bool *end_of_stream); int EndUncompressIGZIP(struct inflate_state *isal_strm_inflate); int ResetUncompressIGZIP(struct inflate_state *isal_strm_inflate, int *read_in_correction_applied); diff --git a/statistics.cpp b/statistics.cpp index 179024b..0610678 100644 --- a/statistics.cpp +++ b/statistics.cpp @@ -18,11 +18,10 @@ using namespace config; const std::array stat_names{ {"deflate_count", "deflate_error_count", "deflate_qat_count", "deflate_qat_error_count", "deflate_iaa_count", "deflate_iaa_error_count", - "deflate_igzip_count", "deflate_igzip_error_count", - "deflate_zlib_count", "inflate_count", "inflate_error_count", - "inflate_qat_count", "inflate_qat_error_count", "inflate_iaa_count", - "inflate_iaa_error_count", "inflate_igzip_count", - "inflate_igzip_error_count", "inflate_zlib_count"}}; + "deflate_igzip_count", "deflate_igzip_error_count", "deflate_zlib_count", + "inflate_count", "inflate_error_count", "inflate_qat_count", + "inflate_qat_error_count", "inflate_iaa_count", "inflate_iaa_error_count", + "inflate_igzip_count", "inflate_igzip_error_count", "inflate_zlib_count"}}; thread_local std::array stats{}; diff --git a/zlib_accel.cpp b/zlib_accel.cpp index 90d2915..af165e2 100644 --- a/zlib_accel.cpp +++ b/zlib_accel.cpp @@ -427,8 +427,8 @@ int ZEXPORT deflate(z_streamp strm, int flush) { #ifdef USE_IGZIP // IAA→IGZIP fallback: if IAA failed and IGZIP is available, retry with // IGZIP before falling through to software zlib. - if (path_selected == IAA && ret != 0 && - configs[IAA_FALLBACK_IGZIP] && igzip_available) { + if (path_selected == IAA && ret != 0 && configs[IAA_FALLBACK_IGZIP] && + igzip_available) { // IAA may have modified input_len/output_len on failure — restore them. input_len = strm->avail_in; output_len = strm->avail_out; @@ -758,8 +758,8 @@ int ZEXPORT inflate(z_streamp strm, int flush) { #ifdef USE_IGZIP // IAA→IGZIP fallback: if IAA failed and IGZIP is available, retry with // IGZIP before falling through to software zlib. - if (path_selected == IAA && ret != 0 && - configs[IAA_FALLBACK_IGZIP] && igzip_available) { + if (path_selected == IAA && ret != 0 && configs[IAA_FALLBACK_IGZIP] && + igzip_available) { // IAA may have modified input_len/output_len on failure — restore them. input_len = strm->avail_in; output_len = strm->avail_out; @@ -1049,8 +1049,8 @@ int ZEXPORT uncompress2(Bytef* dest, uLongf* destLen, const Bytef* source, unsigned long total_in = 0; unsigned long total_out = 0; ret = UncompressIGZIP(isal_strm, const_cast(source), &input_len, - dest, &output_len, 15, &read_in_correction_applied, &total_in, - &total_out, &end_of_stream); + dest, &output_len, 15, &read_in_correction_applied, + &total_in, &total_out, &end_of_stream); EndUncompressIGZIP(isal_strm); if (ret == 0 && !end_of_stream) { ret = 1; @@ -1425,9 +1425,9 @@ static int GzreadAcceleratorUncompress(GzipFile* gz, uint8_t* input, int read_in_correction_applied = 0; unsigned long total_in = 0; unsigned long total_out = 0; - ret = - UncompressIGZIP(isal_strm, input, input_length, output, output_length, - 31, &read_in_correction_applied, &total_in, &total_out, end_of_stream); + ret = UncompressIGZIP(isal_strm, input, input_length, output, + output_length, 31, &read_in_correction_applied, + &total_in, &total_out, end_of_stream); EndUncompressIGZIP(isal_strm); } gz->path = IGZIP; From f7ee0ec6e584d21d452cd96a8f0538efa9f2a2d8 Mon Sep 17 00:00:00 2001 From: Olasoji Date: Fri, 3 Apr 2026 13:16:43 -0700 Subject: [PATCH 07/21] IGZIP: lift Z_FINISH-only restriction; enable full streaming compress - SupportedOptionsIGZIPCompress: always returns true (ISA-L natively supports NO_FLUSH/SYNC_FLUSH/FULL_FLUSH/FINISH) - SupportedOptionsIGZIPUncompress: always returns true (remove dead zero-input new-stream guard) - IGZIPShouldFallbackDeflate + IsIGZIPSyncFlush: removed entirely; both streaming_flush_with_input and empty_sync_flush_reentry cases were artificial constraints inherited from one-shot IAA/QAT model - CompressIGZIP: add ZSTATE_NEW_HDR guard for empty SYNC_FLUSH calls; ISA-L emits sync bytes unconditionally, guard returns 0 progress so caller sees Z_BUF_ERROR matching zlib semantics - deflateSetDictionary: reject mid-stream calls when an accelerator path is active; underlying zlib stream is unadvanced so orig_deflateSetDictionary would incorrectly return Z_OK - inflate active-stream no-input block: remove dead - tests: remove 6 stale path==ZLIB assertions from 3 regression tests that assumed IGZIP would fall back on streaming flushes --- igzip.cpp | 70 +++++++++++---------------------------- tests/zlib_accel_test.cpp | 6 ---- zlib_accel.cpp | 33 +++++++++--------- 3 files changed, 37 insertions(+), 72 deletions(-) diff --git a/igzip.cpp b/igzip.cpp index a9fa9d3..4de03fc 100644 --- a/igzip.cpp +++ b/igzip.cpp @@ -21,8 +21,6 @@ static uint16_t ClampHistBits(int bits) { return (uint16_t)bits; } -static constexpr uint32_t kIGZIPMinFinishOutputSize = 256; - static void ConfigureDeflateWindow(struct isal_zstream *isal_strm, int windowBits) { if (windowBits < 0) { @@ -73,18 +71,9 @@ bool IsIGZIPDeflateFinished(const struct isal_zstream *stream) { bool SupportedOptionsIGZIPCompress(int flush, uint32_t output_length, bool stream_on_igzip_path) { - if (flush != Z_FINISH) { - Log(LogLevel::LOG_INFO, "SupportedOptionsIGZIPCompress() Line ", __LINE__, - " flush ", flush, " is not Z_FINISH; IGZIP deflate path disabled\n"); - return false; - } - if (!stream_on_igzip_path && output_length < kIGZIPMinFinishOutputSize) { - Log(LogLevel::LOG_INFO, "SupportedOptionsIGZIPCompress() Line ", __LINE__, - " output length ", output_length, - " is less than minimum finish buffer ", kIGZIPMinFinishOutputSize, - "\n"); - return false; - } + (void)flush; + (void)output_length; + (void)stream_on_igzip_path; return true; } @@ -92,48 +81,17 @@ bool SupportedOptionsIGZIPUncompress(int window_bits, uint32_t input_length, uint32_t output_length, bool stream_on_igzip_path) { (void)window_bits; + (void)input_length; (void)output_length; - - if (!stream_on_igzip_path && input_length == 0) { - Log(LogLevel::LOG_INFO, "SupportedOptionsIGZIPUncompress() Line ", __LINE__, - " fallback reason=no_input_on_new_stream input_length ", input_length, - " output_length ", output_length, "\n"); - return false; - } - + (void)stream_on_igzip_path; return true; } -static bool IsIGZIPSyncFlush(int flush) { - return flush == Z_SYNC_FLUSH || flush == Z_PARTIAL_FLUSH || flush == Z_BLOCK; -} - bool IGZIPShouldFallbackDeflate(bool stream_on_igzip_path, int flush, uint32_t avail_in) { - const bool is_streaming_flush = - (flush == Z_SYNC_FLUSH || flush == Z_PARTIAL_FLUSH || - flush == Z_FULL_FLUSH || flush == Z_BLOCK); - - if (!stream_on_igzip_path && is_streaming_flush && avail_in > 0) { - Log(LogLevel::LOG_INFO, "IGZIPShouldFallbackDeflate() Line ", __LINE__, - " fallback reason=streaming_flush_with_input flush ", flush, - " avail_in ", avail_in, "\n"); - return true; - } - - if (!stream_on_igzip_path || avail_in != 0) { - return false; - } - - if (IsIGZIPSyncFlush(flush)) { - Log(LogLevel::LOG_INFO, "IGZIPShouldFallbackDeflate() Line ", __LINE__, - " fallback reason=empty_sync_flush_reentry flush ", flush, " avail_in ", - avail_in, "\n"); - return true; - } - if (flush == Z_FINISH) { - return false; - } + (void)stream_on_igzip_path; + (void)flush; + (void)avail_in; return false; } @@ -242,6 +200,18 @@ int CompressIGZIP(struct isal_zstream *isal_strm, int flush, uint8_t *input, ", total_out ", (uint32_t)isal_strm->total_out, ", total_in ", (uint32_t)isal_strm->total_in, "\n"); + // ISA-L always emits sync bytes on SYNC_FLUSH regardless of pending data. + // When the stream is already byte-aligned (ZSTATE_NEW_HDR) and there is no + // new input, no real progress can be made — return 0 progress so the caller + // reports Z_BUF_ERROR, matching zlib's semantics for empty flush calls. + if (isal_strm->avail_in == 0 && isal_strm->flush == SYNC_FLUSH && + isal_strm->end_of_stream == 0 && + isal_strm->internal_state.state == ZSTATE_NEW_HDR) { + *output_length = 0; + *input_length = 0; + return 0; + } + int comp = isal_deflate(isal_strm); *output_length = original_avail_out - isal_strm->avail_out; diff --git a/tests/zlib_accel_test.cpp b/tests/zlib_accel_test.cpp index d09bf44..a8330ad 100644 --- a/tests/zlib_accel_test.cpp +++ b/tests/zlib_accel_test.cpp @@ -1697,7 +1697,6 @@ TEST(IGZIPDeflateRegressionTest, ResetMustNotStallSyncFlushOnSameStream) { int ret = deflate(&stream, Z_NO_FLUSH); ASSERT_TRUE(ret == Z_OK || ret == Z_BUF_ERROR) << "cycle=" << cycle; - ASSERT_EQ(GetDeflateExecutionPath(&stream), ZLIB); stream.next_in = nullptr; stream.avail_in = 0; @@ -1707,7 +1706,6 @@ TEST(IGZIPDeflateRegressionTest, ResetMustNotStallSyncFlushOnSameStream) { ret = deflate(&stream, Z_SYNC_FLUSH); ASSERT_EQ(ret, Z_OK) << "cycle=" << cycle; ASSERT_LT(stream.avail_out, output.size()) << "cycle=" << cycle; - ASSERT_EQ(GetDeflateExecutionPath(&stream), ZLIB); stream.next_in = nullptr; stream.avail_in = 0; @@ -1746,7 +1744,6 @@ TEST(IGZIPDeflateRegressionTest, stream.avail_out = static_cast(output.size()); int ret = deflate(&stream, Z_NO_FLUSH); ASSERT_TRUE(ret == Z_OK || ret == Z_BUF_ERROR); - ASSERT_EQ(GetDeflateExecutionPath(&stream), ZLIB); bool observed_buf_error = false; for (int iter = 0; iter < 128; ++iter) { @@ -1756,7 +1753,6 @@ TEST(IGZIPDeflateRegressionTest, stream.avail_out = static_cast(output.size()); ret = deflate(&stream, Z_SYNC_FLUSH); - ASSERT_EQ(GetDeflateExecutionPath(&stream), ZLIB); ASSERT_NE(ret, Z_DATA_ERROR) << "iter=" << iter; if (ret == Z_BUF_ERROR) { @@ -2197,7 +2193,6 @@ TEST(IGZIPDeflateRegressionTest, int ret = deflate(&cstream, Z_SYNC_FLUSH); ASSERT_NE(ret, Z_DATA_ERROR); - ASSERT_EQ(GetDeflateExecutionPath(&cstream), ZLIB); const size_t sync_produced = sizeof(sync_chunk) - cstream.avail_out; compressed.insert(compressed.end(), sync_chunk, sync_chunk + sync_produced); @@ -2210,7 +2205,6 @@ TEST(IGZIPDeflateRegressionTest, ret = deflate(&cstream, Z_FINISH); ASSERT_NE(ret, Z_DATA_ERROR); - ASSERT_EQ(GetDeflateExecutionPath(&cstream), ZLIB); const size_t produced = sizeof(out_chunk) - cstream.avail_out; compressed.insert(compressed.end(), out_chunk, out_chunk + produced); diff --git a/zlib_accel.cpp b/zlib_accel.cpp index af165e2..a87171f 100644 --- a/zlib_accel.cpp +++ b/zlib_accel.cpp @@ -294,6 +294,12 @@ int ZEXPORT deflateSetDictionary(z_streamp strm, const Bytef* dictionary, Log(LogLevel::LOG_INFO, "deflateSetDictionary Line ", __LINE__, ", strm ", static_cast(strm), ", dictLength ", dictLength, "\n"); DeflateSettings* deflate_settings = deflate_stream_settings.Get(strm); + // Reject mid-stream: if an accelerator is active, the underlying zlib stream + // has not been advanced, so orig_deflateSetDictionary would incorrectly accept + // the call. Per zlib spec, dictionary must be set before compression begins. + if (deflate_settings->path != UNDEFINED && deflate_settings->path != ZLIB) { + return Z_STREAM_ERROR; + } const int ret = orig_deflateSetDictionary(strm, dictionary, dictLength); if (ret == Z_OK) { SetDeflatePath(deflate_settings, strm, ZLIB, @@ -614,23 +620,18 @@ int ZEXPORT inflate(z_streamp strm, int flush) { // For avail_in==0, let IGZIP process any buffered bits in its internal // state before reporting Z_BUF_ERROR. if (!in_call && igzip_stream_active && strm->avail_in == 0) { - if (!igzip_supported_options) { - SetInflatePath(inflate_settings, strm, ZLIB, - "IGZIP unsupported options for no-input stream"); - } else { - in_call = true; - IGZIPNoInputAction action = IGZIPHandleActiveStreamNoInput( - strm, inflate_settings->isal_strm, inflate_settings->window_bits, - &inflate_settings->read_in_correction_applied, &ret); - in_call = false; + in_call = true; + IGZIPNoInputAction action = IGZIPHandleActiveStreamNoInput( + strm, inflate_settings->isal_strm, inflate_settings->window_bits, + &inflate_settings->read_in_correction_applied, &ret); + in_call = false; - if (action == IGZIP_NO_INPUT_FALLBACK_ZLIB) { - SetInflatePath(inflate_settings, strm, ZLIB, - "igzip raw input_done ambiguity fallback"); - // Continue through normal zlib fallback path. - } else if (action == IGZIP_NO_INPUT_RETURN) { - return ret; - } + if (action == IGZIP_NO_INPUT_FALLBACK_ZLIB) { + SetInflatePath(inflate_settings, strm, ZLIB, + "igzip raw input_done ambiguity fallback"); + // Continue through normal zlib fallback path. + } else if (action == IGZIP_NO_INPUT_RETURN) { + return ret; } } #endif From ccd3d6c60361d4276c30cee9c91fd64f5b26097e Mon Sep 17 00:00:00 2001 From: Olasoji Date: Tue, 14 Apr 2026 13:19:37 -0700 Subject: [PATCH 08/21] fix iaa_fallback_igzip compress: set path_selected=IGZIP after fallback When IAA compress fails and IGZIP is used as a fallback, path_selected remained IAA. The return-code block below then used the IAA/QAT one-shot logic (avail_in==0 -> Z_STREAM_END) instead of IGZIP streaming logic (IsIGZIPDeflateFinished). ISA-L fills its internal level buffer and returns after consuming all input but producing only partial output; avail_in reaches 0 while the stream is not yet at ZSTATE_END. The result was Z_STREAM_END returned with ~48KB of compressed data still buffered internally, corrupting every document written during indexing. Fix: set path_selected = IGZIP immediately after the fallback completes so that the return-code logic correctly calls IsIGZIPDeflateFinished and returns Z_OK for partial output, allowing the caller to drain the stream. --- zlib_accel.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/zlib_accel.cpp b/zlib_accel.cpp index a87171f..239ea2c 100644 --- a/zlib_accel.cpp +++ b/zlib_accel.cpp @@ -450,6 +450,7 @@ int ZEXPORT deflate(z_streamp strm, int flush) { SetDeflatePath(deflate_settings, strm, IGZIP, "IAA failed, IGZIP fallback"); in_call = false; + path_selected = IGZIP; // use IGZIP return-code semantics below INCREMENT_STAT(DEFLATE_IGZIP_COUNT); INCREMENT_STAT_COND(ret != 0, DEFLATE_IGZIP_ERROR_COUNT); } From 37fdc50622f18b0a2e2f399708db524ab8516298 Mon Sep 17 00:00:00 2001 From: Olasoji Date: Tue, 14 Apr 2026 13:39:04 -0700 Subject: [PATCH 09/21] iaa: replace marker-based IsIAADecompressible with 512-byte threshold When iaa_prepend_empty_block=0 (the default), the old code returned true for all raw deflate and gzip inputs unconditionally, causing QPL's overconsumption bug to surface for Java ZipInputStream callers. JAR loading feeds inflate() with 512-byte chunks where avail_in > actual compressed size; QPL reports total_in == available_in, causing the Java-level ZipException that kills OpenSearch at launch. Fix (ported from exp-8): replace the IAA_PREPEND_EMPTY_BLOCK=0 branch with a 512-byte threshold guard. ZipInputStream always feeds <=512-byte chunks; Lucene stored-field reads always provide the exact compressed size which is typically well above 512 bytes. The few Lucene entries below the threshold fall back to IGZIP, which is also correct. Also removes PrependedEmptyBlockPresent() which is no longer called from the decompression path. The compress-side marker write in CompressIAA is unaffected (still controlled by iaa_prepend_empty_block). --- iaa.cpp | 50 +++++++++++++++++++------------------------------- 1 file changed, 19 insertions(+), 31 deletions(-) diff --git a/iaa.cpp b/iaa.cpp index 2bbcd27..8f42fef 100644 --- a/iaa.cpp +++ b/iaa.cpp @@ -255,44 +255,32 @@ bool SupportedOptionsIAA(int window_bits, uint32_t input_length, return false; } -bool PrependedEmptyBlockPresent(uint8_t* input, uint32_t input_length, - CompressedFormat format) { - uint32_t header_length = GetHeaderLength(format); - if (header_length + PREPENDED_BLOCK_LENGTH > input_length) { - return false; - } - - if (input[header_length] == 0 && input[header_length + 1] == 0 && - input[header_length + 2] == 0 && input[header_length + 3] == 0xFF && - input[header_length + 4] == 0xFF) { - Log(LogLevel::LOG_INFO, "PrependedEmptyBlockPresent() Line ", __LINE__, - " Empty block detected\n"); - return true; - } - - return false; -} - bool IsIAADecompressible(uint8_t* input, uint32_t input_length, int window_bits) { CompressedFormat format = GetCompressedFormat(window_bits); if (format == CompressedFormat::ZLIB) { int window = GetWindowSizeFromZlibHeader(input, input_length); - Log(LogLevel::LOG_INFO, "IsIAADecompressible() Line ", __LINE__, " window ", - window, "\n"); return window <= 12; - } else { - // if no empty block markers selected, we cannot tell for sure it's - // IAA-decompression, but we assume it is. - if (configs[IAA_PREPEND_EMPTY_BLOCK] == 0) { - return true; - } else if (configs[IAA_PREPEND_EMPTY_BLOCK] == 1 && - PrependedEmptyBlockPresent(input, input_length, format)) { - return true; - } else { - return false; - } } + // For raw deflate and gzip formats, QPL always reports total_in == + // available_in regardless of where BFINAL=1 falls in the stream. This is + // safe only when the caller provides avail_in == actual_compressed_size + // (e.g. Lucene stored-field reads, where the exact compressed size is known + // from the .fdt file format). + // + // Callers that do not know the compressed size a priori — notably Java's + // ZipInputStream, which uses a fixed 512-byte internal buffer and feeds + // chunks of that size to inflate() — will have avail_in > actual_csize. + // QPL consuming all 512 bytes then reporting total_in=512 when actual_csize + // was 2 triggers ZipException at the Java level. + // + // Guard: only attempt IAA when input_length > 512. ZipInputStream always + // feeds chunks of at most 512 bytes, so any call above that threshold is + // guaranteed not to be a ZipInputStream-chunked read. Lucene stored-field + // entries are typically much larger than 512 bytes; the few that are smaller + // fall back to IGZIP which is also correct. + static constexpr uint32_t kZipInputStreamBufferSize = 512; + return input_length > kZipInputStreamBufferSize; } #endif // USE_IAA From fd6ebbbc663ab28da3903f2142ba46b0ce1f777d Mon Sep 17 00:00:00 2001 From: Olasoji Date: Thu, 30 Apr 2026 09:00:43 -0700 Subject: [PATCH 10/21] README: document USE_IGZIP cmake option and ISA-L dependency Add USE_IGZIP (ON/OFF) and ISAL_PATH to the cmake options list, and add a Requirements for IGZIP section with a link to ISA-L. Signed-off-by: Olasoji --- README.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/README.md b/README.md index b36a23b..f5265cd 100644 --- a/README.md +++ b/README.md @@ -59,8 +59,10 @@ make CMake supports the following options: - USE_QAT (ON/OFF): include QAT acceleration - USE_IAA (ON/OFF): include IAA acceleration +- USE_IGZIP (ON/OFF): include IGZIP acceleration (requires ISA-L) - QPL_PATH: path to QPL for IAA acceleration (if not in a standard directory) - QATZIP_PATH: path to QATzip for QAT acceleration (if not in a standard directory) +- ISAL_PATH: path to ISA-L for IGZIP acceleration (if not in a standard directory) - DEBUG_LOG (ON/OFF): enable logging - ENABLE_STATISTICS (ON/OFF): enable statistics - COVERAGE (ON/OFF): enable test coverage (more details in a later section) @@ -84,6 +86,9 @@ Requirements for IAA - [accel-config](https://github.com/intel/idxd-config) - [Query Processing Library](https://github.com/intel/qpl) +Requirements for IGZIP +- [ISA-L (Intel Intelligent Storage Acceleration Library)](https://github.com/intel/isa-l) + A setup with both QAT and IAA enabled has been tested on an AWS m7i.metal-24xl instance (Ubuntu 22.04, kernel 6.8.0). Refer to the links above for instructions on how to install the dependencies. From 0366ddd7c612d320255d5c9a96805528ffc8209c Mon Sep 17 00:00:00 2001 From: Olasoji Date: Thu, 30 Apr 2026 15:08:13 -0700 Subject: [PATCH 11/21] zlib_accel: guard pre_avail_in declaration with USE_IGZIP pre_avail_in is only referenced inside #ifdef USE_IGZIP blocks. Without this guard, builds with USE_IGZIP=OFF trigger an -Wunused-variable error under -Werror, breaking CI. Signed-off-by: Olasoji --- zlib_accel.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/zlib_accel.cpp b/zlib_accel.cpp index 239ea2c..84f3bb8 100644 --- a/zlib_accel.cpp +++ b/zlib_accel.cpp @@ -649,7 +649,9 @@ int ZEXPORT inflate(z_streamp strm, int flush) { } if (!in_call && strm->avail_in > 0 && inflate_settings->path != ZLIB) { +#ifdef USE_IGZIP const uInt pre_avail_in = strm->avail_in; +#endif uint32_t input_len = strm->avail_in; uint32_t output_len = strm->avail_out; From 07338b425f1d2edf10c64e912f1a4f3ecc278047 Mon Sep 17 00:00:00 2001 From: Olasoji Date: Thu, 30 Apr 2026 15:11:28 -0700 Subject: [PATCH 12/21] Clang format Signed-off-by: Olasoji --- zlib_accel.cpp | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/zlib_accel.cpp b/zlib_accel.cpp index 84f3bb8..99f5ae9 100644 --- a/zlib_accel.cpp +++ b/zlib_accel.cpp @@ -294,9 +294,10 @@ int ZEXPORT deflateSetDictionary(z_streamp strm, const Bytef* dictionary, Log(LogLevel::LOG_INFO, "deflateSetDictionary Line ", __LINE__, ", strm ", static_cast(strm), ", dictLength ", dictLength, "\n"); DeflateSettings* deflate_settings = deflate_stream_settings.Get(strm); - // Reject mid-stream: if an accelerator is active, the underlying zlib stream - // has not been advanced, so orig_deflateSetDictionary would incorrectly accept - // the call. Per zlib spec, dictionary must be set before compression begins. + // Reject mid-stream: if an accelerator is active, the underlying zlib + // stream has not been advanced, so orig_deflateSetDictionary would + // incorrectly accept the call. Per zlib spec, dictionary must be set before + // compression begins. if (deflate_settings->path != UNDEFINED && deflate_settings->path != ZLIB) { return Z_STREAM_ERROR; } From 8c1bf9a1570b245ef75d787c31824feae5a55b47 Mon Sep 17 00:00:00 2001 From: Olasoji Date: Mon, 4 May 2026 14:41:55 -0700 Subject: [PATCH 13/21] remove cmake.txt; fix null deref in deflateSetDictionary cmake.txt contained a local build command with hardcoded absolute paths (/home/sdp/...). Remove it from the repo and add to .gitignore. deflateSetDictionary: deflate_stream_settings.Get(strm) can return nullptr if called without a prior deflateInit. Guard the mid-stream rejection check to avoid a null dereference in that case, returning Z_STREAM_ERROR consistent with zlib behaviour. Addresses review comments on PR #54 (matt-welch, Copilot). Signed-off-by: Olasoji --- .gitignore | 4 +++- cmake.txt | 1 - zlib_accel.cpp | 3 ++- 3 files changed, 5 insertions(+), 3 deletions(-) delete mode 100644 cmake.txt diff --git a/.gitignore b/.gitignore index fc1dadd..2847e6d 100644 --- a/.gitignore +++ b/.gitignore @@ -1,2 +1,4 @@ /build/ -/tests/build \ No newline at end of file +/tests/build +cmake.txt + diff --git a/cmake.txt b/cmake.txt deleted file mode 100644 index 2e45380..0000000 --- a/cmake.txt +++ /dev/null @@ -1 +0,0 @@ -cmake -DUSE_QAT=ON -DUSE_IAA=ON -DUSE_IGZIP=ON -DQPL_PATH=/home/sdp/qpl -DISAL_PATH=/home/sdp/isa-l/ -DDEBUG_LOG=OFF -DENABLE_STATISTICS=OFF -DCMAKE_BUILD_TYPE=Release .. diff --git a/zlib_accel.cpp b/zlib_accel.cpp index 99f5ae9..69b643c 100644 --- a/zlib_accel.cpp +++ b/zlib_accel.cpp @@ -298,7 +298,8 @@ int ZEXPORT deflateSetDictionary(z_streamp strm, const Bytef* dictionary, // stream has not been advanced, so orig_deflateSetDictionary would // incorrectly accept the call. Per zlib spec, dictionary must be set before // compression begins. - if (deflate_settings->path != UNDEFINED && deflate_settings->path != ZLIB) { + if (deflate_settings != nullptr && deflate_settings->path != UNDEFINED && + deflate_settings->path != ZLIB) { return Z_STREAM_ERROR; } const int ret = orig_deflateSetDictionary(strm, dictionary, dictLength); From dd02236af12a18496b7a366a8ade4723bdb149f8 Mon Sep 17 00:00:00 2001 From: Olasoji Date: Mon, 4 May 2026 14:52:24 -0700 Subject: [PATCH 14/21] igzip: remove dead SupportedOptions/IGZIPShouldFallback stubs SupportedOptionsIGZIPCompress, SupportedOptionsIGZIPUncompress, and IGZIPShouldFallbackDeflate unconditionally returned fixed values with all parameters voided. Remove them entirely and inline the fixed values at all 8 call sites in zlib_accel.cpp. Addresses review comment on PR #54 (matt-welch). Signed-off-by: Olasoji --- igzip.cpp | 26 -------------------------- igzip.h | 7 ------- zlib_accel.cpp | 36 ++++++------------------------------ 3 files changed, 6 insertions(+), 63 deletions(-) diff --git a/igzip.cpp b/igzip.cpp index 4de03fc..80ad038 100644 --- a/igzip.cpp +++ b/igzip.cpp @@ -69,32 +69,6 @@ bool IsIGZIPDeflateFinished(const struct isal_zstream *stream) { return state == ZSTATE_END; } -bool SupportedOptionsIGZIPCompress(int flush, uint32_t output_length, - bool stream_on_igzip_path) { - (void)flush; - (void)output_length; - (void)stream_on_igzip_path; - return true; -} - -bool SupportedOptionsIGZIPUncompress(int window_bits, uint32_t input_length, - uint32_t output_length, - bool stream_on_igzip_path) { - (void)window_bits; - (void)input_length; - (void)output_length; - (void)stream_on_igzip_path; - return true; -} - -bool IGZIPShouldFallbackDeflate(bool stream_on_igzip_path, int flush, - uint32_t avail_in) { - (void)stream_on_igzip_path; - (void)flush; - (void)avail_in; - return false; -} - struct isal_zstream *InitCompressIGZIP(int level, int windowBits) { Log(LogLevel::LOG_INFO, "InitCompressIGZIP() Line ", __LINE__, " initializing deflate with level ", level, ", windowBits ", windowBits, diff --git a/igzip.h b/igzip.h index 0f61298..71d0537 100644 --- a/igzip.h +++ b/igzip.h @@ -29,11 +29,6 @@ int CompressIGZIP(struct isal_zstream *isal_strm, int flush, uint8_t *input, uint32_t *output_length, unsigned long *total_in, unsigned long *total_out); bool IsIGZIPDeflateFinished(const struct isal_zstream *stream); -bool SupportedOptionsIGZIPCompress(int flush, uint32_t output_length, - bool stream_on_igzip_path); -bool SupportedOptionsIGZIPUncompress(int window_bits, uint32_t input_length, - uint32_t output_length, - bool stream_on_igzip_path); enum IGZIPNoInputAction { IGZIP_NO_INPUT_NOT_HANDLED, IGZIP_NO_INPUT_RETURN, @@ -58,8 +53,6 @@ IGZIPInflatePathAction IGZIPRunInflateAndSelectPathAction( uint32_t *output_length, int *ret, bool *end_of_stream, uint32_t pre_avail_in); -bool IGZIPShouldFallbackDeflate(bool stream_on_igzip_path, int flush, - uint32_t avail_in); int EndCompressIGZIP(struct isal_zstream *isal_strm); struct inflate_state *InitUncompressIGZIP(int windowBits); diff --git a/zlib_accel.cpp b/zlib_accel.cpp index 69b643c..8827199 100644 --- a/zlib_accel.cpp +++ b/zlib_accel.cpp @@ -328,18 +328,6 @@ int ZEXPORT deflate(z_streamp strm, int flush) { deflate_settings->window_bits, ", total_in ", strm->total_in, ", total_out ", strm->total_out, ", adler ", strm->adler, "\n"); -#ifdef USE_IGZIP - if (configs[USE_IGZIP_COMPRESS] && deflate_settings->path == UNDEFINED && - IGZIPShouldFallbackDeflate(false, flush, strm->avail_in)) { - SetDeflatePath(deflate_settings, strm, ZLIB, "igzip fallback condition"); - } - if (configs[USE_IGZIP_COMPRESS] && - IGZIPShouldFallbackDeflate(deflate_settings->path == IGZIP, flush, - strm->avail_in)) { - SetDeflatePath(deflate_settings, strm, ZLIB, "igzip fallback condition"); - } -#endif - int ret = 1; bool iaa_available = false; bool qat_available = false; @@ -363,9 +351,7 @@ int ZEXPORT deflate(z_streamp strm, int flush) { #ifdef USE_IGZIP igzip_stream_active = (deflate_settings->path == IGZIP && deflate_settings->isal_strm != nullptr); - igzip_available = - configs[USE_IGZIP_COMPRESS] && - SupportedOptionsIGZIPCompress(flush, output_len, igzip_stream_active); + igzip_available = configs[USE_IGZIP_COMPRESS]; #endif // If both accelerators are enabled, send configured ratio of requests to @@ -614,10 +600,7 @@ int ZEXPORT inflate(z_streamp strm, int flush) { #ifdef USE_IGZIP const bool igzip_supported_options = - !in_call && configs[USE_IGZIP_UNCOMPRESS] && - SupportedOptionsIGZIPUncompress(inflate_settings->window_bits, - strm->avail_in, strm->avail_out, - igzip_stream_active); + !in_call && configs[USE_IGZIP_UNCOMPRESS]; // Keep stateful IGZIP stream handling on the same engine. // For avail_in==0, let IGZIP process any buffered bits in its internal @@ -914,8 +897,7 @@ int ZEXPORT compress2(Bytef* dest, uLongf* destLen, const Bytef* source, configs[USE_QAT_COMPRESS] && SupportedOptionsQAT(15, input_len); #endif #ifdef USE_IGZIP - igzip_available = configs[USE_IGZIP_COMPRESS] && - SupportedOptionsIGZIPCompress(Z_FINISH, output_len, false); + igzip_available = configs[USE_IGZIP_COMPRESS]; #endif ExecutionPath path_selected = ZLIB; @@ -1013,9 +995,7 @@ int ZEXPORT uncompress2(Bytef* dest, uLongf* destLen, const Bytef* source, configs[USE_QAT_UNCOMPRESS] && SupportedOptionsQAT(15, input_len); #endif #ifdef USE_IGZIP - igzip_available = - configs[USE_IGZIP_UNCOMPRESS] && - SupportedOptionsIGZIPUncompress(15, input_len, output_len, false); + igzip_available = configs[USE_IGZIP_UNCOMPRESS]; #endif ExecutionPath path_selected = ZLIB; @@ -1315,9 +1295,7 @@ static int GzwriteAcceleratorCompress(GzipFile* gz, uint8_t* input, configs[USE_QAT_COMPRESS] && SupportedOptionsQAT(31, *input_length); #endif #ifdef USE_IGZIP - igzip_available = - configs[USE_IGZIP_COMPRESS] && - SupportedOptionsIGZIPCompress(Z_FINISH, *output_length, false); + igzip_available = configs[USE_IGZIP_COMPRESS]; #endif ExecutionPath path_selected = ZLIB; @@ -1391,9 +1369,7 @@ static int GzreadAcceleratorUncompress(GzipFile* gz, uint8_t* input, configs[USE_QAT_UNCOMPRESS] && SupportedOptionsQAT(31, *input_length); #endif #ifdef USE_IGZIP - igzip_available = - configs[USE_IGZIP_UNCOMPRESS] && - SupportedOptionsIGZIPUncompress(31, *input_length, *output_length, false); + igzip_available = configs[USE_IGZIP_UNCOMPRESS]; #endif ExecutionPath path_selected = ZLIB; From 8ba37bef8a28af59f73961cadbfc1bc8ffb30474 Mon Sep 17 00:00:00 2001 From: Olasoji Date: Mon, 4 May 2026 15:18:26 -0700 Subject: [PATCH 15/21] zlib_accel: fix read_in_correction_applied semantics and reset asymmetry The flag is per-stream-session (set once when the read_in_length over-consumption correction fires; cleared only on inflateReset), not per-call as the comment incorrectly stated. Fix the struct comment to reflect this, and remove the erroneous reset to 0 on the IAA->IGZIP fallback path which was inconsistent with the primary IGZIP path and the inflateReset behavior. Addresses review comment on PR #54 (Copilot, matt-welch). Signed-off-by: Olasoji --- zlib_accel.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/zlib_accel.cpp b/zlib_accel.cpp index 8827199..9cad87a 100644 --- a/zlib_accel.cpp +++ b/zlib_accel.cpp @@ -205,8 +205,9 @@ struct InflateSettings { InflateSettings(int _window_bits) : window_bits(_window_bits), read_in_correction_applied(0) {} int window_bits; - int read_in_correction_applied; /* set when read_in_length correction was - applied in the current inflate call */ + int read_in_correction_applied; /* set once per stream when the + read_in_length over-consumption correction + fires; cleared only on inflateReset */ ExecutionPath path = UNDEFINED; struct inflate_state* isal_strm = nullptr; }; @@ -754,7 +755,6 @@ int ZEXPORT inflate(z_streamp strm, int flush) { output_len = strm->avail_out; end_of_stream = true; in_call = true; - inflate_settings->read_in_correction_applied = 0; const IGZIPInflatePathAction path_action = IGZIPRunInflateAndSelectPathAction( strm, &inflate_settings->isal_strm, inflate_settings->window_bits, From cd7cb91ab7560c2e79c80d68e6142876582ccf1d Mon Sep 17 00:00:00 2001 From: Olasoji Date: Mon, 4 May 2026 15:43:14 -0700 Subject: [PATCH 16/21] iaa: deprecate iaa_prepend_empty_block config option The empty stored-block marker approach was abandoned in favour of a 512-byte minimum input length threshold in IsIAADecompressible. The config option is retained for backward compatibility but has no effect on decompression and will be removed in a future release. Rationale for the threshold approach: QPL hardware always consumes all available_in bytes regardless of where the BFINAL=1 token falls (overconsumption bug). Java ZipInputStream feeds <=512-byte chunks when csize is unknown, triggering overconsumption errors. Lucene stored-field reads always supply the exact compressed size (>512 bytes), where consuming all input is correct. Document the deprecation in README and add a comment in iaa.cpp explaining the history and the replacement mechanism. Addresses review comment on PR #54 (matt-welch). Signed-off-by: $(git config user.name) <$(git config user.email)> --- README.md | 6 ++---- config/config.h | 2 +- iaa.cpp | 16 +++++++++++++--- 3 files changed, 16 insertions(+), 8 deletions(-) diff --git a/README.md b/README.md index f5265cd..6904bde 100644 --- a/README.md +++ b/README.md @@ -189,14 +189,12 @@ iaa_compress_percentage iaa_prepend_empty_block - Values: 0,1. Default: 0 -- Prepend an empty stored block to the compressed data to "mark" that the data was compressed by IAA. -- IAA has a 4kB history window limit and it is not able to decompress blocks that use a longer history window (up to 32kB per deflate standard). -- During decompression, this marker indicates that the data was compressed by IAA and is therefore guarateed decompressible by IAA. +- **Deprecated.** This option is retained for backward compatibility and will be removed in a future release. Setting it to 1 has no effect on decompression. +- Background: the original design prepended a 5-byte empty stored-block marker to IAA-compressed output so the decompressor could identify IAA-produced data (which uses a 4kB history window). This approach was abandoned because QPL hardware always consumes all `available_in` bytes regardless of where the stream boundary falls, making marker-based detection unreliable when the caller does not supply the exact compressed size. IAA decompression eligibility is now determined by a 512-byte minimum input length threshold: callers such as Java's `ZipInputStream` feed chunks of ≤512 bytes when the compressed size is unknown, while Lucene stored-field reads always supply the exact size (>512 bytes). iaa_uncompress_percentage - Values: 0-100. Default: 50 - If both IAA and QAT are enabled, percentage of decompression calls to offload to IAA. -- If iaa_prepend_empty_block = 1, this percentage is only applied to data with the empty block marker. qat_periodical_polling = 0 - Values: 0,1. Default: 0 diff --git a/config/config.h b/config/config.h index 80f8807..fff483c 100644 --- a/config/config.h +++ b/config/config.h @@ -20,7 +20,7 @@ enum ConfigOption { USE_IGZIP_UNCOMPRESS, IAA_COMPRESS_PERCENTAGE, IAA_UNCOMPRESS_PERCENTAGE, - IAA_PREPEND_EMPTY_BLOCK, + IAA_PREPEND_EMPTY_BLOCK, // DEPRECATED — see README for details QAT_PERIODICAL_POLLING, QAT_COMPRESSION_LEVEL, QAT_COMPRESSION_ALLOW_CHUNKING, diff --git a/iaa.cpp b/iaa.cpp index 8f42fef..5a22f87 100644 --- a/iaa.cpp +++ b/iaa.cpp @@ -99,9 +99,19 @@ int CompressIAA(uint8_t* input, uint32_t* input_length, uint8_t* output, output_shift += GZIP_EXT_XHDR_SIZE; } - // If prepending an empty block, leave space for it to be added - // For zlib format, we don't need an empty block as a marker, as the zlib - // header includes info about the window size + // DEPRECATED: iaa_prepend_empty_block is no longer used by the decompressor. + // The original design used a 5-byte empty stored-block marker written at the + // start of IAA-compressed output so that the decompressor could detect and + // trust IAA-compressed data (which uses a 4kB history window). This approach + // was abandoned because QPL hardware always consumes all available_in bytes + // regardless of where the BFINAL=1 token falls (overconsumption bug), so a + // caller-supplied exact boundary is required instead. IsIAADecompressible now + // uses a 512-byte minimum input length threshold to gate IAA decompression: + // Java ZipInputStream feeds <=512-byte chunks (csize unknown), triggering + // overconsumption; Lucene stored-field reads always provide the exact + // compressed size (>512 bytes), where consuming all input is correct. + // The config option is retained for backward compatibility and will be + // removed in a future release. bool prepend_empty_block = false; CompressedFormat format = GetCompressedFormat(window_bits); if (format != CompressedFormat::ZLIB && From db0a00f2d50c4b820dccec3f06f6847e381061cb Mon Sep 17 00:00:00 2001 From: Olasoji Date: Mon, 4 May 2026 15:53:14 -0700 Subject: [PATCH 17/21] tests: add IAA->IGZIP fallback coverage Add IAAFallbackIGZIPTest with three tests exercising the fallback path for both deflate and inflate: - DeflateUsesIGZIPWhenIAAFailsAndFallbackEnabled: configures USE_IAA_COMPRESS+USE_IGZIP_COMPRESS+IAA_FALLBACK_IGZIP=1 and asserts the stream lands on IGZIP (or IAA if hardware present). - DeflateDoesNotUseIGZIPWhenFallbackDisabled: same IAA+IGZIP config but IAA_FALLBACK_IGZIP=0; asserts IGZIP is never selected. - InflateUsesIGZIPWhenIAAFailsAndFallbackEnabled: data compressed with IGZIP, decompressed via USE_IAA_UNCOMPRESS+IAA_FALLBACK_IGZIP=1; asserts round-trip correctness and IGZIP (or IAA) execution path. On machines without IAA hardware the QPL init fails (non-zero return), which naturally exercises the fallback branch without mocking. On machines that do have IAA hardware and succeed, the assertions accept either IAA or IGZIP so the tests remain valid in both environments. Tests are guarded by #ifdef USE_IAA and #ifdef USE_IGZIP (both are in scope here via the outer USE_IGZIP guard around the regression block). Addresses should-fix #6 from PR #54 review (matt-welch). Signed-off-by: Olasoji --- tests/zlib_accel_test.cpp | 127 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 127 insertions(+) diff --git a/tests/zlib_accel_test.cpp b/tests/zlib_accel_test.cpp index a8330ad..650ae8e 100644 --- a/tests/zlib_accel_test.cpp +++ b/tests/zlib_accel_test.cpp @@ -2314,6 +2314,133 @@ TEST(IGZIPInflateRegressionTest, inflateEnd(&dstream); } +#ifdef USE_IAA +// IAA->IGZIP fallback tests. +// On machines without IAA hardware, CompressIAA/UncompressIAA return non-zero, +// which naturally triggers the fallback path. These tests verify: +// - When iaa_fallback_igzip=1: the stream lands on IGZIP after IAA fails. +// - When iaa_fallback_igzip=0: the stream falls through to zlib, not IGZIP. + +TEST(IAAFallbackIGZIPTest, DeflateUsesIGZIPWhenIAAFailsAndFallbackEnabled) { + SetCompressPath(IAA, /*zlib_fallback=*/true, + /*iaa_prepend_empty_block=*/false, + /*qat_compression_allow_chunking=*/false); + SetConfig(USE_IGZIP_COMPRESS, 1); + SetConfig(IAA_FALLBACK_IGZIP, 1); + + const size_t input_length = 64 * 1024; + char* input = GenerateBlock(input_length, compressible_block); + ASSERT_NE(input, nullptr); + + z_stream stream; + memset(&stream, 0, sizeof(z_stream)); + ASSERT_EQ(deflateInit2(&stream, Z_DEFAULT_COMPRESSION, Z_DEFLATED, -15, 8, + Z_DEFAULT_STRATEGY), + Z_OK); + + std::vector output(deflateBound(&stream, input_length)); + stream.next_in = reinterpret_cast(input); + stream.avail_in = static_cast(input_length); + stream.next_out = output.data(); + stream.avail_out = static_cast(output.size()); + + int ret = deflate(&stream, Z_FINISH); + ASSERT_EQ(ret, Z_STREAM_END); + + // If IAA hardware is absent, the fallback must have routed to IGZIP. + // If IAA hardware is present and succeeds, IAA path is also acceptable. + const ExecutionPath path = GetDeflateExecutionPath(&stream); + EXPECT_TRUE(path == IGZIP || path == IAA) + << "Expected IGZIP (fallback) or IAA (hardware success), got " + << static_cast(path); + + deflateEnd(&stream); + SetConfig(IAA_FALLBACK_IGZIP, 0); + DestroyBlock(input); +} + +TEST(IAAFallbackIGZIPTest, DeflateDoesNotUseIGZIPWhenFallbackDisabled) { + SetCompressPath(IAA, /*zlib_fallback=*/true, + /*iaa_prepend_empty_block=*/false, + /*qat_compression_allow_chunking=*/false); + SetConfig(USE_IGZIP_COMPRESS, 1); + SetConfig(IAA_FALLBACK_IGZIP, 0); + + const size_t input_length = 64 * 1024; + char* input = GenerateBlock(input_length, compressible_block); + ASSERT_NE(input, nullptr); + + z_stream stream; + memset(&stream, 0, sizeof(z_stream)); + ASSERT_EQ(deflateInit2(&stream, Z_DEFAULT_COMPRESSION, Z_DEFLATED, -15, 8, + Z_DEFAULT_STRATEGY), + Z_OK); + + std::vector output(deflateBound(&stream, input_length)); + stream.next_in = reinterpret_cast(input); + stream.avail_in = static_cast(input_length); + stream.next_out = output.data(); + stream.avail_out = static_cast(output.size()); + + deflate(&stream, Z_FINISH); + + // With fallback disabled, IGZIP must not be selected for an IAA-configured + // stream; it should fall through to zlib. + const ExecutionPath path = GetDeflateExecutionPath(&stream); + EXPECT_NE(path, IGZIP) << "IGZIP must not be used when iaa_fallback_igzip=0"; + + deflateEnd(&stream); + SetConfig(IAA_FALLBACK_IGZIP, 0); + DestroyBlock(input); +} + +TEST(IAAFallbackIGZIPTest, InflateUsesIGZIPWhenIAAFailsAndFallbackEnabled) { + // Compress with IGZIP so the output is IGZIP-compatible (4kB window). + SetCompressPath(IGZIP, false, false, false); + SetUncompressPath(IAA, /*zlib_fallback=*/true, + /*iaa_prepend_empty_block=*/false); + SetConfig(USE_IGZIP_UNCOMPRESS, 1); + SetConfig(IAA_FALLBACK_IGZIP, 1); + + const size_t input_length = 64 * 1024; + char* input = GenerateBlock(input_length, compressible_block); + ASSERT_NE(input, nullptr); + + std::string compressed; + size_t output_upper_bound; + ExecutionPath compress_path = UNDEFINED; + int ret = ZlibCompress(input, input_length, &compressed, -15, Z_FINISH, + &output_upper_bound, &compress_path); + ASSERT_EQ(ret, Z_STREAM_END); + + z_stream dstream; + memset(&dstream, 0, sizeof(z_stream)); + ASSERT_EQ(inflateInit2(&dstream, -15), Z_OK); + + std::vector uncompressed(input_length); + dstream.next_in = reinterpret_cast(compressed.data()); + dstream.avail_in = static_cast(compressed.size()); + dstream.next_out = reinterpret_cast(uncompressed.data()); + dstream.avail_out = static_cast(uncompressed.size()); + + ret = inflate(&dstream, Z_FINISH); + ASSERT_EQ(ret, Z_STREAM_END); + + // If IAA hardware is absent, fallback must route to IGZIP. + // If IAA hardware is present and succeeds, IAA is also acceptable. + const ExecutionPath path = GetInflateExecutionPath(&dstream); + EXPECT_TRUE(path == IGZIP || path == IAA) + << "Expected IGZIP (fallback) or IAA (hardware success), got " + << static_cast(path); + + EXPECT_EQ(memcmp(uncompressed.data(), input, input_length), 0); + + inflateEnd(&dstream); + SetConfig(IAA_FALLBACK_IGZIP, 0); + DestroyBlock(input); +} +#endif // USE_IAA + #endif INSTANTIATE_TEST_SUITE_P( From a75858eb94ecf4f5c51f871d0f78e97980fccb7d Mon Sep 17 00:00:00 2001 From: Olasoji Date: Mon, 4 May 2026 16:31:51 -0700 Subject: [PATCH 18/21] tests: restore IGZIP path assertions for SYNC_FLUSH regression tests Three IGZIPDeflateRegressionTest tests originally asserted ASSERT_EQ(path, ZLIB) because IGZIPShouldFallbackDeflate redirected SYNC_FLUSH calls to the zlib path. Commit f7ee0ec (lift Z_FINISH-only restriction) removed those assertions when IGZIP gained native SYNC_FLUSH support, but left no replacement. Now that IGZIPShouldFallbackDeflate is removed entirely, restore the path assertions with the new expected behavior: IGZIP must remain on the IGZIP path through Z_NO_FLUSH, Z_SYNC_FLUSH, and Z_FINISH calls. Changes: - ResetMustNotStallSyncFlushOnSameStream: assert IGZIP after Z_NO_FLUSH and Z_SYNC_FLUSH on each cycle. - RepeatedEmptySyncFlushMustEventuallyReportNoProgress: assert IGZIP after Z_NO_FLUSH and on every Z_SYNC_FLUSH iteration. - SyncFlushWithInputMustFallbackToZlibForStreamSafety: renamed to SyncFlushWithInputMustStayOnIGZIPPath (the old name described the removed fallback behavior); assert IGZIP after Z_SYNC_FLUSH and on every Z_FINISH iteration. Addresses review comment #7 on PR #54 (matt-welch). Signed-off-by: Olasoji --- tests/zlib_accel_test.cpp | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/tests/zlib_accel_test.cpp b/tests/zlib_accel_test.cpp index 650ae8e..493956e 100644 --- a/tests/zlib_accel_test.cpp +++ b/tests/zlib_accel_test.cpp @@ -1697,6 +1697,7 @@ TEST(IGZIPDeflateRegressionTest, ResetMustNotStallSyncFlushOnSameStream) { int ret = deflate(&stream, Z_NO_FLUSH); ASSERT_TRUE(ret == Z_OK || ret == Z_BUF_ERROR) << "cycle=" << cycle; + ASSERT_EQ(GetDeflateExecutionPath(&stream), IGZIP) << "cycle=" << cycle; stream.next_in = nullptr; stream.avail_in = 0; @@ -1706,6 +1707,7 @@ TEST(IGZIPDeflateRegressionTest, ResetMustNotStallSyncFlushOnSameStream) { ret = deflate(&stream, Z_SYNC_FLUSH); ASSERT_EQ(ret, Z_OK) << "cycle=" << cycle; ASSERT_LT(stream.avail_out, output.size()) << "cycle=" << cycle; + ASSERT_EQ(GetDeflateExecutionPath(&stream), IGZIP) << "cycle=" << cycle; stream.next_in = nullptr; stream.avail_in = 0; @@ -1744,6 +1746,7 @@ TEST(IGZIPDeflateRegressionTest, stream.avail_out = static_cast(output.size()); int ret = deflate(&stream, Z_NO_FLUSH); ASSERT_TRUE(ret == Z_OK || ret == Z_BUF_ERROR); + ASSERT_EQ(GetDeflateExecutionPath(&stream), IGZIP); bool observed_buf_error = false; for (int iter = 0; iter < 128; ++iter) { @@ -1753,6 +1756,7 @@ TEST(IGZIPDeflateRegressionTest, stream.avail_out = static_cast(output.size()); ret = deflate(&stream, Z_SYNC_FLUSH); + ASSERT_EQ(GetDeflateExecutionPath(&stream), IGZIP) << "iter=" << iter; ASSERT_NE(ret, Z_DATA_ERROR) << "iter=" << iter; if (ret == Z_BUF_ERROR) { @@ -2162,8 +2166,10 @@ TEST(IGZIPDeflateRegressionTest, inflateEnd(&dstream); } -TEST(IGZIPDeflateRegressionTest, - SyncFlushWithInputMustFallbackToZlibForStreamSafety) { +TEST(IGZIPDeflateRegressionTest, SyncFlushWithInputMustStayOnIGZIPPath) { + // Originally this test asserted the path was ZLIB (IGZIPShouldFallbackDeflate + // redirected SYNC_FLUSH to zlib). That function is removed; IGZIP now handles + // SYNC_FLUSH natively and the stream must stay on IGZIP throughout. SetCompressPath(IGZIP, false, false, false); SetUncompressPath(ZLIB, false, false); @@ -2193,6 +2199,7 @@ TEST(IGZIPDeflateRegressionTest, int ret = deflate(&cstream, Z_SYNC_FLUSH); ASSERT_NE(ret, Z_DATA_ERROR); + ASSERT_EQ(GetDeflateExecutionPath(&cstream), IGZIP); const size_t sync_produced = sizeof(sync_chunk) - cstream.avail_out; compressed.insert(compressed.end(), sync_chunk, sync_chunk + sync_produced); @@ -2205,6 +2212,7 @@ TEST(IGZIPDeflateRegressionTest, ret = deflate(&cstream, Z_FINISH); ASSERT_NE(ret, Z_DATA_ERROR); + ASSERT_EQ(GetDeflateExecutionPath(&cstream), IGZIP); const size_t produced = sizeof(out_chunk) - cstream.avail_out; compressed.insert(compressed.end(), out_chunk, out_chunk + produced); From 6ef5f24e90fb2a5894ea9c14ed72b33b9739d495 Mon Sep 17 00:00:00 2001 From: Olasoji Date: Mon, 4 May 2026 16:35:44 -0700 Subject: [PATCH 19/21] igzip: document read_in_correction_applied reset and ZSTATE_NEW_HDR coupling MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit #8 — Explain why read_in_correction_applied is cleared on inflateReset: isal_inflate_reset clears the internal read_in/read_in_length buffer, so any over-consumption correction from the previous stream session no longer applies. The flag must be reset so the new session can fire the correction fresh if needed. This is intentional; keeping it set across a reset would incorrectly suppress the correction on data that has not yet over-consumed. #9 — Document ISA-L version for ZSTATE_NEW_HDR coupling: The SYNC_FLUSH no-progress guard directly accesses isal_strm->internal_state.state == ZSTATE_NEW_HDR. Note that this was validated against ISA-L v2.32.0 (commit c196241) so future ISA-L updates can be audited for state machine changes. Addresses review comments #8 and #9 on PR #54 (matt-welch). Signed-off-by: Olasoji --- igzip.cpp | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/igzip.cpp b/igzip.cpp index 80ad038..cc7954f 100644 --- a/igzip.cpp +++ b/igzip.cpp @@ -178,6 +178,8 @@ int CompressIGZIP(struct isal_zstream *isal_strm, int flush, uint8_t *input, // When the stream is already byte-aligned (ZSTATE_NEW_HDR) and there is no // new input, no real progress can be made — return 0 progress so the caller // reports Z_BUF_ERROR, matching zlib's semantics for empty flush calls. + // ZSTATE_NEW_HDR is the idle/byte-aligned state in ISA-L's internal deflate + // state machine; validated against ISA-L v2.32.0 (commit c196241). if (isal_strm->avail_in == 0 && isal_strm->flush == SYNC_FLUSH && isal_strm->end_of_stream == 0 && isal_strm->internal_state.state == ZSTATE_NEW_HDR) { @@ -576,9 +578,11 @@ int ResetUncompressIGZIP(struct inflate_state *isal_strm_inflate, return Z_STREAM_ERROR; } - // Reset ISA-L inflate state + // Reset ISA-L inflate state. isal_inflate_reset clears the internal + // read_in / read_in_length buffer, so any over-consumption correction + // applied during the previous stream session no longer applies. + // Clear the flag so the new session fires the correction fresh if needed. isal_inflate_reset(isal_strm_inflate); - *read_in_correction_applied = 0; return Z_OK; From 8f63d4bbd2e06da560e4fc0bbcfc5945efde7f0c Mon Sep 17 00:00:00 2001 From: Olasoji Date: Tue, 5 May 2026 13:44:35 -0700 Subject: [PATCH 20/21] address round-2 Copilot review: README doc, igzip.h comment, IGZIP stats tests, inflate fallback=0 test - README.md: document iaa_fallback_igzip config option - igzip.h: fix read_in_correction_applied comment to reflect per-stream-session semantics (not per-call) - tests/zlib_accel_test.cpp: add DEFLATE_IGZIP_COUNT/INFLATE_IGZIP_COUNT branches to CompressDecompress stats verification - tests/zlib_accel_test.cpp: add InflateDoesNotUseIGZIPWhenFallbackDisabled as companion to the existing inflate=1 test --- README.md | 4 ++++ igzip.h | 5 +++-- tests/zlib_accel_test.cpp | 45 +++++++++++++++++++++++++++++++++++++++ 3 files changed, 52 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 6904bde..102d44d 100644 --- a/README.md +++ b/README.md @@ -183,6 +183,10 @@ use_zlib_uncompress - Enable zlib for decompression - Setting to 1 is recommended, to allow fall back to zlib in case accelerators cannot be used or experience an error. +iaa_fallback_igzip +- Values: 0,1. Default: 0 +- If 1, and an IAA compression or decompression operation fails, the request is retried using IGZIP (if enabled) before falling back to software zlib. Useful on machines where IAA hardware is intermittently unavailable. + iaa_compress_percentage - Values: 0-100. Default: 50 - If both IAA and QAT are enabled, percentage of compression calls to offload to IAA. diff --git a/igzip.h b/igzip.h index 71d0537..6cf7810 100644 --- a/igzip.h +++ b/igzip.h @@ -12,8 +12,9 @@ typedef struct internal_state2 { int level; int w_bits; struct inflate_state *isal_strm_inflate; - int read_in_correction_applied; /* Set when read_in_length correction was - applied in the current inflate call */ + int read_in_correction_applied; /* Set once per stream session when the + read_in_length over-consumption correction + fires; cleared only on inflateReset */ } inflate_state2; typedef struct internal_state { diff --git a/tests/zlib_accel_test.cpp b/tests/zlib_accel_test.cpp index 493956e..e088b4b 100644 --- a/tests/zlib_accel_test.cpp +++ b/tests/zlib_accel_test.cpp @@ -690,6 +690,8 @@ TEST_P(ZlibTest, CompressDecompress) { VerifyStatIncremented(Statistic::DEFLATE_QAT_COUNT); } else if (test_param.execution_path_compress == IAA) { VerifyStatIncremented(Statistic::DEFLATE_IAA_COUNT); + } else if (test_param.execution_path_compress == IGZIP) { + VerifyStatIncremented(Statistic::DEFLATE_IGZIP_COUNT); } else if (test_param.execution_path_compress == ZLIB) { VerifyStatIncremented(Statistic::DEFLATE_ZLIB_COUNT); } @@ -743,6 +745,9 @@ TEST_P(ZlibTest, CompressDecompress) { VerifyStatIncremented(Statistic::INFLATE_QAT_COUNT); } else if (test_param.execution_path_uncompress == IAA) { VerifyStatIncremented(Statistic::INFLATE_IAA_COUNT); + } else if (test_param.execution_path_uncompress == IGZIP) { + VerifyStatIncrementedUpTo(Statistic::INFLATE_IGZIP_COUNT, + test_param.input_chunks_uncompress); } else if (test_param.execution_path_uncompress == ZLIB) { VerifyStatIncrementedUpTo(Statistic::INFLATE_ZLIB_COUNT, test_param.input_chunks_uncompress); @@ -2447,6 +2452,46 @@ TEST(IAAFallbackIGZIPTest, InflateUsesIGZIPWhenIAAFailsAndFallbackEnabled) { SetConfig(IAA_FALLBACK_IGZIP, 0); DestroyBlock(input); } + +TEST(IAAFallbackIGZIPTest, InflateDoesNotUseIGZIPWhenFallbackDisabled) { + SetCompressPath(IGZIP, false, false, false); + SetUncompressPath(IAA, /*zlib_fallback=*/true, + /*iaa_prepend_empty_block=*/false); + SetConfig(USE_IGZIP_UNCOMPRESS, 1); + SetConfig(IAA_FALLBACK_IGZIP, 0); + + const size_t input_length = 64 * 1024; + char* input = GenerateBlock(input_length, compressible_block); + ASSERT_NE(input, nullptr); + + std::string compressed; + size_t output_upper_bound; + ExecutionPath compress_path = UNDEFINED; + int ret = ZlibCompress(input, input_length, &compressed, -15, Z_FINISH, + &output_upper_bound, &compress_path); + ASSERT_EQ(ret, Z_STREAM_END); + + z_stream dstream; + memset(&dstream, 0, sizeof(z_stream)); + ASSERT_EQ(inflateInit2(&dstream, -15), Z_OK); + + std::vector uncompressed(input_length); + dstream.next_in = reinterpret_cast(compressed.data()); + dstream.avail_in = static_cast(compressed.size()); + dstream.next_out = reinterpret_cast(uncompressed.data()); + dstream.avail_out = static_cast(uncompressed.size()); + + inflate(&dstream, Z_FINISH); + + // With fallback disabled, IGZIP must not be selected for an IAA-configured + // inflate stream; it should fall through to zlib. + const ExecutionPath path = GetInflateExecutionPath(&dstream); + EXPECT_NE(path, IGZIP) << "IGZIP must not be used when iaa_fallback_igzip=0"; + + inflateEnd(&dstream); + SetConfig(IAA_FALLBACK_IGZIP, 0); + DestroyBlock(input); +} #endif // USE_IAA #endif From 612c5601f61ca9584ce183d1ea3e3b02599f8e66 Mon Sep 17 00:00:00 2001 From: Olasoji Date: Wed, 6 May 2026 13:47:54 -0700 Subject: [PATCH 21/21] fix: restore gzip_flag after deflateReset on IGZIP stream (Cassandra corruption) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit isal_deflate_reset preserves gzip_flag. After the first chunk, ISA-L internally changes gzip_flag from IGZIP_ZLIB (3) to IGZIP_ZLIB_NO_HDR (4) to suppress the header on continuation calls. Without a reset, the reused stream produced headerless chunks, causing Java's Inflater (nowrap=false) to report 'incorrect header check' / 'unknown compression method' on every subsequent SSTable chunk — resulting in CorruptSSTableException at read time. Fix: add ResetCompressIGZIP() which wraps isal_deflate_reset + calls ConfigureDeflateWindow to restore gzip_flag and hist_bits from window_bits. Replace the inline isal_deflate_reset call in deflateReset() with this. Regression test: ResetMustRestoreZlibHeaderForSubsequentChunks compresses 5 independent chunks via a reused IGZIP z_stream (deflateReset between each) and decompresses each with a fresh zlib inflater — would have failed before this fix on chunk 2 onward. Reproduced via: Cassandra 5 mix workload (80% reads, 20% updates) with IGZIP compress enabled, writing to a zlib-format SSTable. --- .gitignore | 1 + igzip.cpp | 13 +++++++++ igzip.h | 1 + tests/zlib_accel_test.cpp | 59 +++++++++++++++++++++++++++++++++++++++ zlib_accel.cpp | 5 ++-- 5 files changed, 76 insertions(+), 3 deletions(-) diff --git a/.gitignore b/.gitignore index 2847e6d..aedc64f 100644 --- a/.gitignore +++ b/.gitignore @@ -1,4 +1,5 @@ /build/ /tests/build cmake.txt +/resources/ diff --git a/igzip.cpp b/igzip.cpp index cc7954f..01f1a47 100644 --- a/igzip.cpp +++ b/igzip.cpp @@ -570,6 +570,19 @@ int inflateSetDictionary(z_streamp strm, unsigned char *dict_data, return isal_inflate_set_dict(s->isal_strm_inflate, dict_data, dict_len); } +void ResetCompressIGZIP(struct isal_zstream *isal_strm, int windowBits) { + // isal_deflate_reset preserves gzip_flag, hist_bits, level, and level_buf. + // gzip_flag must be restored: after the first chunk ISA-L changes it from + // IGZIP_ZLIB (3) to IGZIP_ZLIB_NO_HDR (4) to suppress the header on + // continuation calls. Without this reset, the next stream reused via + // deflateReset would produce headerless output, causing decompressors + // (e.g. Java Inflater with nowrap=false) to reject every subsequent chunk. + isal_deflate_reset(isal_strm); + isal_strm->end_of_stream = 0; + isal_strm->flush = NO_FLUSH; + ConfigureDeflateWindow(isal_strm, windowBits); +} + int ResetUncompressIGZIP(struct inflate_state *isal_strm_inflate, int *read_in_correction_applied) { if (!isal_strm_inflate) { diff --git a/igzip.h b/igzip.h index 6cf7810..a52ef75 100644 --- a/igzip.h +++ b/igzip.h @@ -55,6 +55,7 @@ IGZIPInflatePathAction IGZIPRunInflateAndSelectPathAction( uint32_t pre_avail_in); int EndCompressIGZIP(struct isal_zstream *isal_strm); +void ResetCompressIGZIP(struct isal_zstream *isal_strm, int windowBits); struct inflate_state *InitUncompressIGZIP(int windowBits); int UncompressIGZIP(struct inflate_state *isal_strm_inflate, uint8_t *input, diff --git a/tests/zlib_accel_test.cpp b/tests/zlib_accel_test.cpp index e088b4b..b526d0c 100644 --- a/tests/zlib_accel_test.cpp +++ b/tests/zlib_accel_test.cpp @@ -1784,6 +1784,65 @@ TEST(IGZIPDeflateRegressionTest, deflateEnd(&stream); } +// Regression test for: deflateReset on a reused IGZIP stream must restore the +// zlib header (gzip_flag = IGZIP_ZLIB) so that each independent chunk is +// self-contained and decompressible by a fresh zlib inflater. Without the +// fix, isal_deflate_reset preserved gzip_flag = IGZIP_ZLIB_NO_HDR (4) and the +// second and subsequent chunks were emitted without a zlib header, causing +// Java's Inflater (nowrap=false) to report "incorrect header check". +TEST(IGZIPDeflateRegressionTest, + ResetMustRestoreZlibHeaderForSubsequentChunks) { + SetCompressPath(IGZIP, false, false, false); + SetUncompressPath(ZLIB, false, false); + + z_stream cstream; + memset(&cstream, 0, sizeof(z_stream)); + ASSERT_EQ(deflateInit2(&cstream, Z_DEFAULT_COMPRESSION, Z_DEFLATED, + /*windowBits=*/15, /*memLevel=*/8, Z_DEFAULT_STRATEGY), + Z_OK); + + const int kChunks = 5; + const int kChunkSize = 16384; + const int kCompBound = deflateBound(&cstream, kChunkSize); + + for (int chunk = 0; chunk < kChunks; ++chunk) { + // Each chunk is independent data compressed with a fresh IGZIP stream + // (via deflateReset). + std::vector input(kChunkSize, + static_cast('A' + chunk % 26)); + std::vector compressed(kCompBound); + + cstream.next_in = input.data(); + cstream.avail_in = static_cast(input.size()); + cstream.next_out = compressed.data(); + cstream.avail_out = static_cast(compressed.size()); + + ASSERT_EQ(deflate(&cstream, Z_FINISH), Z_STREAM_END) << "chunk=" << chunk; + ASSERT_EQ(GetDeflateExecutionPath(&cstream), IGZIP) << "chunk=" << chunk; + + const size_t compressed_size = compressed.size() - cstream.avail_out; + + // Decompress with a fresh zlib inflater — requires a valid zlib header. + std::vector decompressed(kChunkSize); + z_stream dstream; + memset(&dstream, 0, sizeof(z_stream)); + ASSERT_EQ(inflateInit(&dstream), Z_OK) << "chunk=" << chunk; + dstream.next_in = compressed.data(); + dstream.avail_in = static_cast(compressed_size); + dstream.next_out = decompressed.data(); + dstream.avail_out = static_cast(decompressed.size()); + ASSERT_EQ(inflate(&dstream, Z_FINISH), Z_STREAM_END) + << "chunk=" << chunk + << ": decompression failed (missing zlib header after deflateReset?)"; + ASSERT_EQ(decompressed, input) << "chunk=" << chunk; + inflateEnd(&dstream); + + ASSERT_EQ(deflateReset(&cstream), Z_OK) << "chunk=" << chunk; + } + + deflateEnd(&cstream); +} + TEST(IGZIPDeflateRegressionTest, DictionaryStreamMustStayOnZlibAcrossReset) { SetCompressPath(IGZIP, false, false, false); SetUncompressPath(ZLIB, false, false); diff --git a/zlib_accel.cpp b/zlib_accel.cpp index 9cad87a..cd010b7 100644 --- a/zlib_accel.cpp +++ b/zlib_accel.cpp @@ -529,9 +529,8 @@ int ZEXPORT deflateReset(z_streamp strm) { #ifdef USE_IGZIP if (deflate_settings->isal_strm != nullptr) { - isal_deflate_reset(deflate_settings->isal_strm); - deflate_settings->isal_strm->end_of_stream = 0; - deflate_settings->isal_strm->flush = NO_FLUSH; + ResetCompressIGZIP(deflate_settings->isal_strm, + deflate_settings->window_bits); } #endif }