From 466c76d55c244e70eee5114856b870622fa438e5 Mon Sep 17 00:00:00 2001 From: Mohamed Issa Date: Mon, 29 Jun 2026 00:14:27 -0700 Subject: [PATCH 01/10] Rewrite sharded map data structure --- CMakeLists.txt | 2 + README.md | 12 ++++-- config/config.cpp | 36 +++++++++-------- config/config.h | 1 + fuzzing/CMakeLists.txt | 3 ++ sharded_map.h | 92 +++++++++++++++++++++++++++++++----------- tests/CMakeLists.txt | 3 ++ 7 files changed, 104 insertions(+), 45 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 1b36c5e..dc1e68c 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -8,6 +8,8 @@ project (zlib-accel DESCRIPTION "Zlib shim to accelerate deflate/inflate") include(common.cmake) +find_package(TBB REQUIRED COMPONENTS tbb) +link_libraries(TBB::tbb) link_libraries(dl) option(ENABLE_STATISTICS "Enable statistics" OFF) diff --git a/README.md b/README.md index 4ed2712..cf92f0d 100644 --- a/README.md +++ b/README.md @@ -4,17 +4,17 @@ zlib-accel is a software shim that intercepts zlib calls and offloads compressio The shim allows applications to leverage hardware accelerators transparently without code changes. The only requirement is to preload the shim's shared library (e.g., using LD_PRELOAD). Two accelerators are supported -- Intel(R) QuickAssist Technology (QAT) -- Intel(R) In-Memory Analytics Accelerator (IAA) +- Intel® QuickAssist Technology (QAT) +- Intel® In-Memory Analytics Accelerator (IAA) ## Scope/Constraints -This shim is not a general-purpose replacement for zlib, and it is able to offload compression/decompression jobs in certain conditions. Therefore, not all applications can take advantage of the transparent offload, depending on how they use zlib. It is important to test thoroughly with your specific application and configuration. The use cases we have tested so far are listed in a section below. +This shim is not a general-purpose replacement for zlib, and it is able to offload compression/decompression jobs in certain conditions. Therefore, not all applications can take advantage of the transparent offload, depending on how they use zlib. It is important to test thoroughly with your specific application and configuration. The use cases we have tested so far are listed in a section below. In general, the shim is able to offload zlib calls that complete compression/decompression of one deflate stream in one call. "Streaming" compression/decompression (where compression/decompression is done incrementally) are not currently supported. If the shim is not able to offload a job to an accelerator, it will fall back to zlib, ensuring the application still works correctly. -The shim has only been tested on Linux. +The shim has only been tested on Linux. Additionally, the [Intel® oneTBB](https://www.intel.com/content/www/us/en/developer/tools/oneapi/onetbb.html) performance library is required and can be acquired by installing the [Intel® oneAPI Base Toolkit](https://www.intel.com/content/www/us/en/developer/tools/oneapi/oneapi-toolkit-download.html). ### Hardware Acceleration @@ -226,6 +226,10 @@ log_file - This option applies only if the shim is built with DEBUG_LOG=ON or ENABLE_STATISTICS=ON. - If specified, store log messages in the file. If not specified, log messages are printed to stdout. +map_shards +- Values: 1-65536. Default 64 +- Sets the size of every thread safe concurrent hash map array used by different streams. +- It must be a power of two, so Fibonacci hashing can be used to calculate uniformly distributed shard indexes. ## Tested Applications/Use Cases diff --git a/config/config.cpp b/config/config.cpp index aefe066..336812a 100644 --- a/config/config.cpp +++ b/config/config.cpp @@ -15,21 +15,22 @@ std::string log_file = ""; // default config values initialization uint32_t configs[CONFIG_MAX] = { - 1, /*use_qat_compress*/ - 1, /*use_qat_uncompress*/ - 0, /*use_iaa_compress*/ - 0, /*use_iaa_uncompress*/ - 1, /*use_zlib_compress*/ - 1, /*use_zlib_uncompress*/ - 50, /*iaa_compress_percentage*/ - 50, /*iaa_uncompress_percentage*/ - 0, /*iaa_prepend_empty_block*/ - 0, /*qat_periodical_polling*/ - 1, /*qat_compression_level*/ - 0, /*qat_compression_allow_chunking*/ - 0, /*ignore_zlib_dictionary*/ - 1, /*log_level*/ - 1000 /*log_stats_samples*/ + 1, /*use_qat_compress*/ + 1, /*use_qat_uncompress*/ + 0, /*use_iaa_compress*/ + 0, /*use_iaa_uncompress*/ + 1, /*use_zlib_compress*/ + 1, /*use_zlib_uncompress*/ + 50, /*iaa_compress_percentage*/ + 50, /*iaa_uncompress_percentage*/ + 0, /*iaa_prepend_empty_block*/ + 0, /*qat_periodical_polling*/ + 1, /*qat_compression_level*/ + 0, /*qat_compression_allow_chunking*/ + 0, /*ignore_zlib_dictionary*/ + 1, /*log_level*/ + 1000, /*log_stats_samples*/ + 64 /*map_shards*/ }; bool LoadConfigFile(std::string& file_content, const char* file_path) { @@ -53,7 +54,8 @@ bool LoadConfigFile(std::string& file_content, const char* file_path) { "qat_compression_allow_chunking", "ignore_zlib_dictionary", "log_level", - "log_stats_samples" + "log_stats_samples", + "map_shards" }; // clang-format on @@ -87,7 +89,7 @@ bool LoadConfigFile(std::string& file_content, const char* file_path) { trySetConfig(IGNORE_ZLIB_DICTIONARY, 1, 0); trySetConfig(LOG_LEVEL, 3, 0); trySetConfig(LOG_STATS_SAMPLES, UINT32_MAX, 0); - + trySetConfig(MAP_SHARDS, 65536, 1); config_reader.GetValue("log_file", log_file); file_content.append(config_reader.DumpValues()); diff --git a/config/config.h b/config/config.h index acc8055..2b4cd34 100644 --- a/config/config.h +++ b/config/config.h @@ -25,6 +25,7 @@ enum ConfigOption { IGNORE_ZLIB_DICTIONARY, LOG_LEVEL, LOG_STATS_SAMPLES, + MAP_SHARDS, CONFIG_MAX }; diff --git a/fuzzing/CMakeLists.txt b/fuzzing/CMakeLists.txt index f4f7fe3..b10ae61 100644 --- a/fuzzing/CMakeLists.txt +++ b/fuzzing/CMakeLists.txt @@ -14,6 +14,9 @@ link_libraries(zlib-accel) include(../common.cmake) +find_package(TBB REQUIRED COMPONENTS tbb) +link_libraries(TBB::tbb) + set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -g -fsanitize=address,fuzzer") set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -g -fsanitize=address,fuzzer") diff --git a/sharded_map.h b/sharded_map.h index ab8dda8..abd6d05 100644 --- a/sharded_map.h +++ b/sharded_map.h @@ -3,45 +3,89 @@ #pragma once +#include #include -#include +#include +#include #include -#include #include -static constexpr int SHARDS = 64; +#include "config/config.h" + +using namespace config; template class ShardedMap { public: - auto Get(Key key) -> decltype(std::declval().get()) { - unsigned int shard = GetShard(key); - std::shared_lock lock(shard_mutexes[shard]); - auto it = map[shard].find(key); - if (it == map[shard].end()) { + ShardedMap(void) { + const auto num_shards = configs[MAP_SHARDS]; + pd_map_arr = std::make_unique(num_shards); + } + + ~ShardedMap(void) { + pd_map_arr.reset(); + } + + auto Get(const Key& key) -> decltype(std::declval().get()) { + const auto shard = GetShard(key); + typename MapType::const_accessor acc; + if (!pd_map_arr[shard].map.find(acc, key)) { return nullptr; } - return it->second.get(); + return acc->second.get(); } - void Set(Key key, Value&& value) { - unsigned int shard = GetShard(key); - std::unique_lock lock(shard_mutexes[shard]); - map[shard][key] = std::move(value); + void Set(const Key& key, Value&& value) { + const auto shard = GetShard(key); + typename MapType::accessor acc; + if (!pd_map_arr[shard].map.find(acc, key)) { + // Key doesn't exist - add entry first + pd_map_arr[shard].map.insert(acc, key); + } + + // Set or update value associated with key + acc->second = std::move(value); } - void Unset(Key key) { - unsigned int shard = GetShard(key); - std::unique_lock lock(shard_mutexes[shard]); - auto it = map[shard].find(key); - if (it != map[shard].end()) { - map[shard].erase(it); - } + void Unset(const Key& key) { + const auto shard = GetShard(key); + pd_map_arr[shard].map.erase(key); } private: - unsigned int GetShard(Key key) { return std::hash{}(key) % SHARDS; } - std::unordered_map map[SHARDS]; - std::shared_mutex shard_mutexes[SHARDS]; - std::hash hash; + // Number of bytes in a cache line + static constexpr std::size_t CACHE_LINE_SIZE = 64; + + // Fibonacci hash constant: value is (2^64 / phi) to spread bits uniformly, + // where phi is golden ratio and 64 is machine word length - constant is + // multiplied by hash and then shifted right by top bits to get shard index + static constexpr std::size_t FIBONACCI_MULTIPLIER = 11400714819323198485ull; + + struct HashCompare { + static auto hash(const Key& key) -> std::size_t { + return std::hash{}(key); + } + + static auto equal(const Key& lhs, const Key& rhs) -> bool { + return lhs == rhs; + } + }; + + using MapType = oneapi::tbb::concurrent_hash_map; + struct alignas(CACHE_LINE_SIZE) PaddedMapType { + MapType map; + }; + std::unique_ptr pd_map_arr; + + // Number of map shards must be a power of 2 + auto GetShard(const Key& key) const -> unsigned int { + const auto num_shards = configs[MAP_SHARDS]; + assert((num_shards & (num_shards - 1)) == 0); + const auto shard_bits = __builtin_ctz(num_shards); + const auto hash = static_cast(std::hash{}(key)); + const auto dist_hash = hash * FIBONACCI_MULTIPLIER; + const auto top_bits = 64 - shard_bits; + const auto shard_index = static_cast(dist_hash >> top_bits); + return shard_index; + } }; diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index b0c4564..cce2f1f 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -10,6 +10,9 @@ link_libraries(zlib-accel) include(../common.cmake) +find_package(TBB REQUIRED COMPONENTS tbb) +link_libraries(TBB::tbb) + find_package(GTest REQUIRED) link_libraries(gtest pthread) From 4e39384741a34287adaf84d6235ce52087b41771 Mon Sep 17 00:00:00 2001 From: Mohamed Issa Date: Tue, 30 Jun 2026 12:43:22 -0700 Subject: [PATCH 02/10] Fix indentation in config_names array --- config/config.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/config/config.cpp b/config/config.cpp index 336812a..d4c726c 100644 --- a/config/config.cpp +++ b/config/config.cpp @@ -51,7 +51,7 @@ bool LoadConfigFile(std::string& file_content, const char* file_path) { "iaa_prepend_empty_block", "qat_periodical_polling", "qat_compression_level", - "qat_compression_allow_chunking", + "qat_compression_allow_chunking", "ignore_zlib_dictionary", "log_level", "log_stats_samples", From c72f2afff55681be20ed9b4b897a0c95c30e545f Mon Sep 17 00:00:00 2001 From: Mohamed Issa Date: Tue, 21 Jul 2026 15:19:12 -0700 Subject: [PATCH 03/10] Create TBB compile time definition and use it to separate different implementations of sharded map. --- CMakeLists.txt | 2 -- common.cmake | 6 +++++ fuzzing/CMakeLists.txt | 3 --- sharded_map.h | 53 ++++++++++++++++++++++++++++++++++-------- tests/CMakeLists.txt | 3 --- 5 files changed, 49 insertions(+), 18 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index dc1e68c..1b36c5e 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -8,8 +8,6 @@ project (zlib-accel DESCRIPTION "Zlib shim to accelerate deflate/inflate") include(common.cmake) -find_package(TBB REQUIRED COMPONENTS tbb) -link_libraries(TBB::tbb) link_libraries(dl) option(ENABLE_STATISTICS "Enable statistics" OFF) diff --git a/common.cmake b/common.cmake index 7c60c0f..046a51d 100644 --- a/common.cmake +++ b/common.cmake @@ -98,4 +98,10 @@ if(USE_QAT) link_libraries(qatzip) endif() +if(USE_QAT OR USE_IAA OR USE_IGZIP) + find_package(TBB REQUIRED COMPONENTS tbb) + link_libraries(TBB::tbb) + add_compile_definitions(USE_TBB) +endif() + link_libraries(z) diff --git a/fuzzing/CMakeLists.txt b/fuzzing/CMakeLists.txt index b10ae61..f4f7fe3 100644 --- a/fuzzing/CMakeLists.txt +++ b/fuzzing/CMakeLists.txt @@ -14,9 +14,6 @@ link_libraries(zlib-accel) include(../common.cmake) -find_package(TBB REQUIRED COMPONENTS tbb) -link_libraries(TBB::tbb) - set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -g -fsanitize=address,fuzzer") set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -g -fsanitize=address,fuzzer") diff --git a/sharded_map.h b/sharded_map.h index abd6d05..199a941 100644 --- a/sharded_map.h +++ b/sharded_map.h @@ -5,11 +5,19 @@ #include #include -#include -#include #include #include +#ifdef USE_TBB + // tbb::concurrent_hash_map implementation + #include + #include // portable: old TBB + oneTBB +#else + // stdlib implementation + #include + #include +#endif + #include "config/config.h" using namespace config; @@ -19,37 +27,54 @@ class ShardedMap { public: ShardedMap(void) { const auto num_shards = configs[MAP_SHARDS]; - pd_map_arr = std::make_unique(num_shards); + pd_map_arr_ = std::make_unique(num_shards); } ~ShardedMap(void) { - pd_map_arr.reset(); + pd_map_arr_.reset(); } auto Get(const Key& key) -> decltype(std::declval().get()) { const auto shard = GetShard(key); +#ifdef USE_TBB typename MapType::const_accessor acc; - if (!pd_map_arr[shard].map.find(acc, key)) { + if (!pd_map_arr_[shard].map.find(acc, key)) { return nullptr; } return acc->second.get(); +#else + std::shared_lock lock(pd_map_arr_[shard].mutex); + auto it = pd_map_arr_[shard].map.find(key); + if (it == pd_map_arr_[shard].map.end()) { + return nullptr; + } + return it->second.get(); +#endif } void Set(const Key& key, Value&& value) { const auto shard = GetShard(key); +#ifdef USE_TBB typename MapType::accessor acc; - if (!pd_map_arr[shard].map.find(acc, key)) { + if (!pd_map_arr_[shard].map.find(acc, key)) { // Key doesn't exist - add entry first - pd_map_arr[shard].map.insert(acc, key); + pd_map_arr_[shard].map.insert(acc, key); } // Set or update value associated with key acc->second = std::move(value); +#else + std::unique_lock lock(pd_map_arr_[shard].mutex); + pd_map_arr_[shard].map[key] = std::move(value); +#endif } void Unset(const Key& key) { const auto shard = GetShard(key); - pd_map_arr[shard].map.erase(key); +#ifndef USE_TBB + std::unique_lock lock(pd_map_arr_[shard].mutex); +#endif + pd_map_arr_[shard].map.erase(key); } private: @@ -61,6 +86,7 @@ class ShardedMap { // multiplied by hash and then shifted right by top bits to get shard index static constexpr std::size_t FIBONACCI_MULTIPLIER = 11400714819323198485ull; +#ifdef USE_TBB struct HashCompare { static auto hash(const Key& key) -> std::size_t { return std::hash{}(key); @@ -71,11 +97,18 @@ class ShardedMap { } }; - using MapType = oneapi::tbb::concurrent_hash_map; + using MapType = tbb::concurrent_hash_map; +#else + using MapType = std::unordered_map; +#endif + struct alignas(CACHE_LINE_SIZE) PaddedMapType { MapType map; +#ifndef USE_TBB + mutable std::shared_mutex mutex; +#endif }; - std::unique_ptr pd_map_arr; + std::unique_ptr pd_map_arr_; // Number of map shards must be a power of 2 auto GetShard(const Key& key) const -> unsigned int { diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index cce2f1f..b0c4564 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -10,9 +10,6 @@ link_libraries(zlib-accel) include(../common.cmake) -find_package(TBB REQUIRED COMPONENTS tbb) -link_libraries(TBB::tbb) - find_package(GTest REQUIRED) link_libraries(gtest pthread) From da2e524d6975c00aae613602c989cbf1ee8b2fdb Mon Sep 17 00:00:00 2001 From: Mohamed Issa Date: Tue, 21 Jul 2026 15:35:05 -0700 Subject: [PATCH 04/10] Remove unecessary namespace usage in ShardedMap class and save shard count in member field for required references. --- sharded_map.h | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/sharded_map.h b/sharded_map.h index 199a941..be06480 100644 --- a/sharded_map.h +++ b/sharded_map.h @@ -20,14 +20,11 @@ #include "config/config.h" -using namespace config; - template class ShardedMap { public: - ShardedMap(void) { - const auto num_shards = configs[MAP_SHARDS]; - pd_map_arr_ = std::make_unique(num_shards); + ShardedMap(void) : num_shards_(config::configs[config::MAP_SHARDS]) { + pd_map_arr_ = std::make_unique(num_shards_); } ~ShardedMap(void) { @@ -109,12 +106,12 @@ class ShardedMap { #endif }; std::unique_ptr pd_map_arr_; + const unsigned int num_shards_; // Number of map shards must be a power of 2 auto GetShard(const Key& key) const -> unsigned int { - const auto num_shards = configs[MAP_SHARDS]; - assert((num_shards & (num_shards - 1)) == 0); - const auto shard_bits = __builtin_ctz(num_shards); + assert((num_shards_ & (num_shards_ - 1)) == 0); + const auto shard_bits = __builtin_ctz(num_shards_); const auto hash = static_cast(std::hash{}(key)); const auto dist_hash = hash * FIBONACCI_MULTIPLIER; const auto top_bits = 64 - shard_bits; From 4696e0bc582c5ceb6f4b482f36e0032c26b774df Mon Sep 17 00:00:00 2001 From: Mohamed Issa Date: Tue, 21 Jul 2026 16:54:23 -0700 Subject: [PATCH 05/10] Add proper check for invalid map shard values and return error if found. --- config/config.cpp | 7 +++++++ logging.h | 2 -- sharded_map.h | 2 -- 3 files changed, 7 insertions(+), 4 deletions(-) diff --git a/config/config.cpp b/config/config.cpp index d4c726c..dff21b7 100644 --- a/config/config.cpp +++ b/config/config.cpp @@ -8,6 +8,7 @@ #include #include "config_reader.h" +#include "../logging.h" namespace config { @@ -90,6 +91,12 @@ bool LoadConfigFile(std::string& file_content, const char* file_path) { trySetConfig(LOG_LEVEL, 3, 0); trySetConfig(LOG_STATS_SAMPLES, UINT32_MAX, 0); trySetConfig(MAP_SHARDS, 65536, 1); + if (configs[MAP_SHARDS] & (configs[MAP_SHARDS] - 1)) { + Log(LogLevel::LOG_ERROR, "config::LoadConfigFile Line ", __LINE__, + ", map_shards must be a power of 2, ignoring value ", + configs[MAP_SHARDS], ", will not proceed further\n"); + return false; + } config_reader.GetValue("log_file", log_file); file_content.append(config_reader.DumpValues()); diff --git a/logging.h b/logging.h index d384cdb..6a151ff 100644 --- a/logging.h +++ b/logging.h @@ -13,8 +13,6 @@ #include "config/config.h" #include "utils.h" -using namespace config; - // Log verbosity levels. 0 = silent; higher values = more verbose. // Matches QATzip's QzLogLevel_T convention. enum class LogLevel { diff --git a/sharded_map.h b/sharded_map.h index be06480..3709f25 100644 --- a/sharded_map.h +++ b/sharded_map.h @@ -3,7 +3,6 @@ #pragma once -#include #include #include #include @@ -110,7 +109,6 @@ class ShardedMap { // Number of map shards must be a power of 2 auto GetShard(const Key& key) const -> unsigned int { - assert((num_shards_ & (num_shards_ - 1)) == 0); const auto shard_bits = __builtin_ctz(num_shards_); const auto hash = static_cast(std::hash{}(key)); const auto dist_hash = hash * FIBONACCI_MULTIPLIER; From fe6eddccaefd834b09fb382771a88a14e1e5bf9d Mon Sep 17 00:00:00 2001 From: Mohamed Issa Date: Tue, 21 Jul 2026 17:25:27 -0700 Subject: [PATCH 06/10] Fix clang-format violations. --- config/config.cpp | 2 +- sharded_map.h | 18 +++++++----------- 2 files changed, 8 insertions(+), 12 deletions(-) diff --git a/config/config.cpp b/config/config.cpp index dff21b7..5058d5c 100644 --- a/config/config.cpp +++ b/config/config.cpp @@ -7,8 +7,8 @@ #include #include -#include "config_reader.h" #include "../logging.h" +#include "config_reader.h" namespace config { diff --git a/sharded_map.h b/sharded_map.h index 3709f25..69bd2b7 100644 --- a/sharded_map.h +++ b/sharded_map.h @@ -4,17 +4,17 @@ #pragma once #include +#include #include #include #ifdef USE_TBB - // tbb::concurrent_hash_map implementation - #include - #include // portable: old TBB + oneTBB +// tbb::concurrent_hash_map implementation +#include // portable: old TBB + oneTBB #else - // stdlib implementation - #include - #include +// stdlib implementation +#include +#include #endif #include "config/config.h" @@ -22,14 +22,10 @@ template class ShardedMap { public: - ShardedMap(void) : num_shards_(config::configs[config::MAP_SHARDS]) { + ShardedMap() : num_shards_(config::configs[config::MAP_SHARDS]) { pd_map_arr_ = std::make_unique(num_shards_); } - ~ShardedMap(void) { - pd_map_arr_.reset(); - } - auto Get(const Key& key) -> decltype(std::declval().get()) { const auto shard = GetShard(key); #ifdef USE_TBB From 4c7ff723a7f9869b2614068aef7ece5d38dde572 Mon Sep 17 00:00:00 2001 From: Mohamed Issa Date: Tue, 21 Jul 2026 18:54:12 -0700 Subject: [PATCH 07/10] Update comment above default config value array. --- config/config.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/config/config.cpp b/config/config.cpp index 5058d5c..6201c15 100644 --- a/config/config.cpp +++ b/config/config.cpp @@ -14,7 +14,7 @@ namespace config { std::string log_file = ""; -// default config values initialization +// default config values at initialization uint32_t configs[CONFIG_MAX] = { 1, /*use_qat_compress*/ 1, /*use_qat_uncompress*/ From 5582a4e7da4cc8615d54f26e57455fcf3d0da869 Mon Sep 17 00:00:00 2001 From: Olasoji Date: Wed, 22 Jul 2026 13:33:22 -0700 Subject: [PATCH 08/10] sharded_map: fix LTO crash and add missing #include MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit config::configs has hidden visibility (-fvisibility=hidden) and is absent from the DSO's dynamic symbol table. Any external consumer (e.g. test binary) that references it gets an unresolved weak reference; under LTO the compiler folds this to 0, giving num_shards_=0. That triggers UB in GetShard (__builtin_ctz(0)) and an out-of-bounds array access → segfault in pthread_rwlock_wrlock. Fix: replace configs[MAP_SHARDS] with GetConfig(MAP_SHARDS) in the ShardedMap constructor. GetConfig is exported and resolves correctly in all consumers. Also add missing #include for std::unique_lock. Also add map_shards = 64 to default_config so LoadConfigFile does not silently leave the key at its hard-coded default. --- config/default_config | 1 + sharded_map.h | 5 ++++- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/config/default_config b/config/default_config index 827b7b2..c735aa2 100644 --- a/config/default_config +++ b/config/default_config @@ -11,5 +11,6 @@ qat_periodical_polling = 0 qat_compression_level = 1 qat_compression_allow_chunking = 0 ignore_zlib_dictionary = 0 +map_shards = 64 log_level = 1 log_file = /tmp/zlib-accel.log diff --git a/sharded_map.h b/sharded_map.h index 69bd2b7..9e6b766 100644 --- a/sharded_map.h +++ b/sharded_map.h @@ -13,6 +13,7 @@ #include // portable: old TBB + oneTBB #else // stdlib implementation +#include #include #include #endif @@ -22,7 +23,9 @@ template class ShardedMap { public: - ShardedMap() : num_shards_(config::configs[config::MAP_SHARDS]) { + // Use GetConfig() rather than configs[] directly: configs has hidden + // visibility and is not accessible from external translation units under LTO. + ShardedMap() : num_shards_(config::GetConfig(config::MAP_SHARDS)) { pd_map_arr_ = std::make_unique(num_shards_); } From 1b589ef4a69afc64326c11c5153867ab6c04fe91 Mon Sep 17 00:00:00 2001 From: Olasoji Date: Fri, 24 Jul 2026 14:30:33 -0700 Subject: [PATCH 09/10] sharded_map: address Copilot review comments - Raise map_shards minimum from 1 to 2; eliminates shift-by-64 UB in GetShard when num_shards_==1 (__builtin_ctz(1)=0, dist_hash>>64 is UB) - Change FIBONACCI_MULTIPLIER to uint64_t; std::size_t truncates the 64-bit constant on 32-bit platforms - Add optional validator param to ConfigReader::GetValue; use it for map_shards power-of-2 check so validation is consistent with other config keys (log error, keep default, continue) rather than aborting config load - Remove now-unused logging.h include from config/config.cpp - Simplify TBB Set: replace find+insert with single insert; accessor is valid either way so the find was redundant - README: clarify TBB is required only with hardware acceleration flags --- README.md | 2 +- config/config.cpp | 15 +++++---------- config/config_reader.cpp | 11 ++++++++++- config/config_reader.h | 4 +++- sharded_map.h | 9 ++------- 5 files changed, 21 insertions(+), 20 deletions(-) diff --git a/README.md b/README.md index cf92f0d..f2d754a 100644 --- a/README.md +++ b/README.md @@ -14,7 +14,7 @@ This shim is not a general-purpose replacement for zlib, and it is able to offlo In general, the shim is able to offload zlib calls that complete compression/decompression of one deflate stream in one call. "Streaming" compression/decompression (where compression/decompression is done incrementally) are not currently supported. If the shim is not able to offload a job to an accelerator, it will fall back to zlib, ensuring the application still works correctly. -The shim has only been tested on Linux. Additionally, the [Intel® oneTBB](https://www.intel.com/content/www/us/en/developer/tools/oneapi/onetbb.html) performance library is required and can be acquired by installing the [Intel® oneAPI Base Toolkit](https://www.intel.com/content/www/us/en/developer/tools/oneapi/oneapi-toolkit-download.html). +The shim has only been tested on Linux. When building with hardware acceleration enabled (`USE_QAT`, `USE_IAA`, or `USE_IGZIP`), the [Intel® oneTBB](https://www.intel.com/content/www/us/en/developer/tools/oneapi/onetbb.html) performance library is required and can be acquired by installing the [Intel® oneAPI Base Toolkit](https://www.intel.com/content/www/us/en/developer/tools/oneapi/oneapi-toolkit-download.html). ### Hardware Acceleration diff --git a/config/config.cpp b/config/config.cpp index 6201c15..cf99d98 100644 --- a/config/config.cpp +++ b/config/config.cpp @@ -7,7 +7,6 @@ #include #include -#include "../logging.h" #include "config_reader.h" namespace config { @@ -68,9 +67,10 @@ bool LoadConfigFile(std::string& file_content, const char* file_path) { ConfigReader config_reader; config_reader.ParseFile(file_path); - auto trySetConfig = [&](ConfigOption opt, uint32_t max, uint32_t min) { + auto trySetConfig = [&](ConfigOption opt, uint32_t max, uint32_t min, + std::function validator = nullptr) { uint32_t value; - if (config_reader.GetValue(config_names[opt], value, max, min)) { + if (config_reader.GetValue(config_names[opt], value, max, min, validator)) { configs[opt] = value; } }; @@ -90,13 +90,8 @@ bool LoadConfigFile(std::string& file_content, const char* file_path) { trySetConfig(IGNORE_ZLIB_DICTIONARY, 1, 0); trySetConfig(LOG_LEVEL, 3, 0); trySetConfig(LOG_STATS_SAMPLES, UINT32_MAX, 0); - trySetConfig(MAP_SHARDS, 65536, 1); - if (configs[MAP_SHARDS] & (configs[MAP_SHARDS] - 1)) { - Log(LogLevel::LOG_ERROR, "config::LoadConfigFile Line ", __LINE__, - ", map_shards must be a power of 2, ignoring value ", - configs[MAP_SHARDS], ", will not proceed further\n"); - return false; - } + trySetConfig(MAP_SHARDS, 65536, 2, + [](uint32_t v) { return (v & (v - 1)) == 0; }); config_reader.GetValue("log_file", log_file); file_content.append(config_reader.DumpValues()); diff --git a/config/config_reader.cpp b/config/config_reader.cpp index 93c0be9..b3b9676 100644 --- a/config/config_reader.cpp +++ b/config/config_reader.cpp @@ -16,7 +16,8 @@ constexpr int CUSTOM_PATH_MAX = 4096; bool ConfigReader::GetValue(const std::string& tag, uint32_t& value, - uint32_t max_value, uint32_t min_value) { + uint32_t max_value, uint32_t min_value, + std::function validator) { auto it = config_settings_map.find(tag); if (it == config_settings_map.end()) { return false; @@ -41,6 +42,14 @@ bool ConfigReader::GetValue(const std::string& tag, uint32_t& value, } value = static_cast(temp); + + if (validator && !validator(value)) { + Log(LogLevel::LOG_ERROR, "ConfigReader::GetValue Line ", __LINE__, + " invalid input value for tag ", tag.c_str(), "\n"); + value = 0; + return false; + } + return true; } catch (const std::exception&) { diff --git a/config/config_reader.h b/config/config_reader.h index 6b61d00..81a8eb2 100644 --- a/config/config_reader.h +++ b/config/config_reader.h @@ -4,6 +4,7 @@ #pragma once #include +#include #include #include @@ -19,7 +20,8 @@ class ConfigReader { public: bool ParseFile(const std::string& file_name); bool GetValue(const std::string& tag, uint32_t& value, - uint32_t max_value = 100, uint32_t min_value = 0); + uint32_t max_value = 100, uint32_t min_value = 0, + std::function validator = nullptr); bool GetValue(const std::string& tag, std::string& value); std::string DumpValues(); diff --git a/sharded_map.h b/sharded_map.h index 9e6b766..b6c626c 100644 --- a/sharded_map.h +++ b/sharded_map.h @@ -51,12 +51,7 @@ class ShardedMap { const auto shard = GetShard(key); #ifdef USE_TBB typename MapType::accessor acc; - if (!pd_map_arr_[shard].map.find(acc, key)) { - // Key doesn't exist - add entry first - pd_map_arr_[shard].map.insert(acc, key); - } - - // Set or update value associated with key + pd_map_arr_[shard].map.insert(acc, key); acc->second = std::move(value); #else std::unique_lock lock(pd_map_arr_[shard].mutex); @@ -79,7 +74,7 @@ class ShardedMap { // Fibonacci hash constant: value is (2^64 / phi) to spread bits uniformly, // where phi is golden ratio and 64 is machine word length - constant is // multiplied by hash and then shifted right by top bits to get shard index - static constexpr std::size_t FIBONACCI_MULTIPLIER = 11400714819323198485ull; + static constexpr uint64_t FIBONACCI_MULTIPLIER = 11400714819323198485ull; #ifdef USE_TBB struct HashCompare { From ddc60e5776919396f7ed0402672f172ca1f87db2 Mon Sep 17 00:00:00 2001 From: Olasoji Date: Fri, 24 Jul 2026 16:51:55 -0700 Subject: [PATCH 10/10] Fix map_shards config not taking effect for global registries MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit LTO causes global constructors to run before __attribute__((constructor)), so the three ShardedMap globals were always allocated with the default 64 shards. Add ShardedMap::Init() and call it from a new InitStreamRegistries() after LoadConfigFile in init_zlib_accel. Also stop treating a missing config file as fatal, and fix the README map_shards description (range 1-65536 → 2-65536, clarify description). --- README.md | 4 ++-- sharded_map.h | 12 +++++++++++- zlib_accel.cpp | 27 +++++++++++++++++++++++---- 3 files changed, 36 insertions(+), 7 deletions(-) diff --git a/README.md b/README.md index f2d754a..6941c2d 100644 --- a/README.md +++ b/README.md @@ -227,8 +227,8 @@ log_file - If specified, store log messages in the file. If not specified, log messages are printed to stdout. map_shards -- Values: 1-65536. Default 64 -- Sets the size of every thread safe concurrent hash map array used by different streams. +- Values: 2-65536. Default 64 +- Sets the number of shards in the thread-safe concurrent hash map. Each shard holds an independent map instance. - It must be a power of two, so Fibonacci hashing can be used to calculate uniformly distributed shard indexes. ## Tested Applications/Use Cases diff --git a/sharded_map.h b/sharded_map.h index b6c626c..8224aea 100644 --- a/sharded_map.h +++ b/sharded_map.h @@ -25,10 +25,20 @@ class ShardedMap { public: // Use GetConfig() rather than configs[] directly: configs has hidden // visibility and is not accessible from external translation units under LTO. + // + // For the three global registry instances, LTO causes the constructor to run + // before init_zlib_accel loads the config file, so num_shards_ is + // initialised with the default value. InitStreamRegistries() calls Init() + // afterwards to re-allocate with the configured shard count. ShardedMap() : num_shards_(config::GetConfig(config::MAP_SHARDS)) { pd_map_arr_ = std::make_unique(num_shards_); } + void Init() { + num_shards_ = config::GetConfig(config::MAP_SHARDS); + pd_map_arr_ = std::make_unique(num_shards_); + } + auto Get(const Key& key) -> decltype(std::declval().get()) { const auto shard = GetShard(key); #ifdef USE_TBB @@ -99,7 +109,7 @@ class ShardedMap { #endif }; std::unique_ptr pd_map_arr_; - const unsigned int num_shards_; + unsigned int num_shards_; // Number of map shards must be a power of 2 auto GetShard(const Key& key) const -> unsigned int { diff --git a/zlib_accel.cpp b/zlib_accel.cpp index 26188e7..6bb5cab 100644 --- a/zlib_accel.cpp +++ b/zlib_accel.cpp @@ -68,6 +68,10 @@ static int (*orig_gzread)(gzFile file, voidp buf, unsigned len); static int (*orig_gzclose)(gzFile file); static int (*orig_gzeof)(gzFile file); +// Forward declaration — defined after DeflateStreamSettings, InflateStreamSettings, +// and GzipFiles class definitions below +static void InitStreamRegistries(); + // Initialize/cleanup functions when library is loaded static int init_zlib_accel(void) __attribute__((constructor)); static void cleanup_zlib_accel(void) __attribute__((destructor)); @@ -150,15 +154,18 @@ static int init_zlib_accel(void) { LOAD_SYMBOL(orig_gzeof, int (*)(gzFile), "gzeof"); - // Load configuration file + // Load configuration file; on failure (file absent or is a symlink) continue + // with compiled-in defaults — a missing config is not fatal. std::string config_file_content; - if (!config::LoadConfigFile(config_file_content)) { + const bool config_loaded = config::LoadConfigFile(config_file_content); + if (!config_loaded) { Log(LogLevel::LOG_ERROR, "Error: Failed to load configuration file\n"); - return 1; } + InitStreamRegistries(); + #if defined(DEBUG_LOG) || defined(ENABLE_STATISTICS) - if (!config::log_file.empty()) { + if (config_loaded && !config::log_file.empty()) { CreateLogFile(config::log_file.c_str()); } #endif @@ -213,6 +220,8 @@ class DeflateStreamSettings { DeflateSettings* Get(z_streamp strm) { return map.Get(strm); } + void Init() { map.Init(); } + private: ShardedMap> map; }; @@ -229,6 +238,8 @@ class InflateStreamSettings { InflateSettings* Get(z_streamp strm) { return map.Get(strm); } + void Init() { map.Init(); } + private: ShardedMap> map; }; @@ -835,11 +846,19 @@ class GzipFiles { GzipFile* Get(gzFile file) { return map.Get(file); } + void Init() { map.Init(); } + private: ShardedMap> map; }; GzipFiles gzip_files; +static void InitStreamRegistries() { + deflate_stream_settings.Init(); + inflate_stream_settings.Init(); + gzip_files.Init(); +} + // Inspired by gz_open in gzlib.c int GetOpenFlags(const char* mode, FileMode* file_mode) { bool cloexec = false;