igzip: sync main (LOG_DEBUG) and rename dict stubs#61
Merged
Conversation
…ntel#56) * Adds test to validate log_stats_samples config Signed-off-by: Olasoji <olasoji.denloye@intel.com> * Reduces excessive error logging in QAT mode Signed-off-by: Olasoji <olasoji.denloye@intel.com> * Bugfix for the log stream Signed-off-by: Olasoji <olasoji.denloye@intel.com> * Treats QPL_STS_MORE_OUTPUT_NEEDED as a non error condition for IAA Signed-off-by: Olasoji <olasoji.denloye@intel.com> * Fixed formatting Signed-off-by: Olasoji <olasoji.denloye@intel.com> * - Modifies iaa behavior so that when QPL_STS_MORE_OUTPUT_NEEDED is returned eos isnt erronously set. Signed-off-by: Olasoji <olasoji.denloye@intel.com> * -Adds proper formatting Signed-off-by: Olasoji <olasoji.denloye@intel.com> * logging: add LOG_DEBUG level; qat: map log level to QATzip log level Introduce LOG_DEBUG (value=1) as the most verbose log level, shifting LOG_INFO to 2 and LOG_ERROR to 3. This follows the conventional ordering used by syslog, Log4j, Python logging, and QATzip itself (where higher values mean more verbose). The default log level remains LOG_ERROR (now value=3), so existing deployments see no behavioral change unless they explicitly set a lower value in /etc/zlib-accel.conf. The config parser max for log_level is updated from 2 to 3 accordingly. In qat.cpp, replace the hardcoded qzSetLogLevel(LOG_NONE) with a dynamic mapping from the configured log_level: LOG_DEBUG -> LOG_DEBUG3 (maximum QATzip verbosity) LOG_INFO -> LOG_INFO LOG_ERROR -> LOG_ERROR default -> LOG_NONE This corrects an inversion present in PR intel#55 of intel/zlib-accel, where LOG_ERROR was incorrectly mapped to LOG_DEBUG3. New tests: LogDebugLevel, LogDebugFilteredByInfo; LogLevelFiltering extended to cover DEBUG filtering. Builds on: intel#55 Signed-off-by: Olasoji <olasoji.denloye@intel.com> * Update README.md Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * qat: gate qzSetLogLevel mapping behind DEBUG_LOG; apply formatting Without the DEBUG_LOG guard, a release build (DEBUG_LOG=OFF) with a non-zero log_level in the config would cause QATzip to emit verbose output, contradicting the documented behaviour that log_level only applies when built with DEBUG_LOG=ON. Gate the dynamic mapping behind #ifdef DEBUG_LOG and unconditionally set LOG_NONE in the else branch, matching the original behaviour for non-debug builds. Addresses Copilot review comment on PR intel#56. Signed-off-by: Olasoji <olasoji.denloye@intel.com> * address review comments: enum ordering, call_once, explicit LOG_NONE case - logging.h: reorder LogLevel enum to match QATzip's QzLogLevel_T convention {LOG_NONE=0, LOG_ERROR=1, LOG_INFO=2, LOG_DEBUG=3}; flip filter direction from 'level < config' to 'level > config' so higher values = more verbose. Apply same fix to PrintDeflateBlockHeader. - qat.cpp: call qzSetLogLevel once per process via std::call_once instead of per session; add explicit case LogLevel::LOG_NONE; move default to end with comment; restructure #ifdef to be inside the once lambda. - config/config.cpp, config/default_config: update default log_level from 3 to 1 (LOG_ERROR = errors only, matching previous error-only default). - README.md: update log_level documentation to describe new ordering. - tests/zlib_accel_test.cpp: update expected log_level default to 1. --------- Signed-off-by: Olasoji <olasoji.denloye@intel.com> Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…ants Rename deflateSetDictionary and inflateSetDictionary in igzip.cpp to DeflateSetDictionaryIGZIP and InflateSetDictionaryIGZIP to eliminate the duplicate symbol conflict with the ZEXPORT versions in zlib_accel.cpp. The functions are stubs for future dictionary support (pending performance validation). Add declarations to igzip.h for when they are wired in.
matt-welch
approved these changes
Jul 15, 2026
matt-welch
left a comment
matt-welch
left a comment
There was a problem hiding this comment.
Testing: PR #61 — all 3220 tests pass
Checked out and built PR #61 on Ubuntu 22.04.5 LTS (kernel 6.8.0-1060-aws), GCC 11.4.0 (g++ 11.4.0-1ubuntu1~22.04.3), libgtest-dev 1.11.0-3, glibc 2.35-0ubuntu3.13. Library and test suite built cleanly; make run passed all 3220 tests with no failures or regressions.
Side issue found (pre-existing, not introduced by this PR): sharded_map.h missing #include
The test build fails with std::unique_lock is not a member of std unless #include is added to sharded_map.h. The fix is a one-liner and was applied locally to unblock testing; it is not part of this PR.
The bug is latent on GCC 11 (Ubuntu 22.04): <shared_mutex> pulls in <bits/unique_lock.h> only indirectly, via <condition_variable>, and only when _GLIBCXX_HAS_GTHREADS is set at the point the header is parsed. Whether unique_lock is visible without an explicit #include therefore depends on include order and build environment. On GCC 13+ (Ubuntu 24.04), <shared_mutex> includes <bits/unique_lock.h> directly and unconditionally, so the bug is invisible there. This explains why it was not caught during development.
Recommend a follow-up investigation to verify the missing "#include " in sharded_map.h is an actual bug and not an artifact of the local review testing.
std::unique_lock is defined in <mutex>, not <shared_mutex>. On GCC 13+ (Ubuntu 24.04), <shared_mutex> transitively includes <bits/unique_lock.h> making this invisible. On GCC 11 (Ubuntu 22.04) the include is not pulled in and the build fails with 'std::unique_lock is not a member of std'. Add explicit <mutex> include for standard compliance. Reported by matt-welch in PR intel#61 review.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Two housekeeping commits to prepare igzip for merge into main.
Merge branch 'main' into igzip
Resolves conflict in config/config.cpp: keeps igzip's new
igzip_fallback config key while picking up main's updated
log_level defaults from PR logging: add LOG_DEBUG level; qat: map log level to QATzip log level #56 (LOG_DEBUG).
Rename deflateSetDictionary/inflateSetDictionary to IGZIP variants
Renames the stub implementations in igzip.cpp to
DeflateSetDictionaryIGZIP and InflateSetDictionaryIGZIP to
eliminate the duplicate symbol conflict with the ZEXPORT versions
in zlib_accel.cpp. Adds declarations to igzip.h. The stubs are
preserved for future dictionary support pending performance
validation.