nlog: replace ConcurrentHashMap with phmap::parallel_flat_hash_map#1138
Open
SBALAVIGNESH123 wants to merge 1 commit intotimeplus-io:developfrom
Open
nlog: replace ConcurrentHashMap with phmap::parallel_flat_hash_map#1138SBALAVIGNESH123 wants to merge 1 commit intotimeplus-io:developfrom
SBALAVIGNESH123 wants to merge 1 commit intotimeplus-io:developfrom
Conversation
Fixes timeplus-io#441 Replace the single std::shared_mutex + absl::flat_hash_map design with phmap::parallel_flat_hash_map using per-submap std::shared_mutex locking. 16 independent submaps enable reader-writer parallelism: operations on keys in different submaps proceed with zero contention.
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
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.
Fixes #441
This PR replaces the current ConcurrentHashMap implementation in NativeLog, which uses a single std::shared_mutex around an absl::flat_hash_map, with phmap::parallel_flat_hash_map from the parallel-hashmap library. The old design meant every read and write operation competed for one global lock, which becomes a bottleneck under concurrent access.The new implementation uses phmap's internal submap-level locking — the map is split into 16 independent submaps, each with its own std::shared_mutex. This means operations on keys that hash to different submaps run fully in parallel with zero contention. Multiple readers on the same submap can also proceed concurrently thanks to the shared_mutex reader-writer separation.parallel-hashmap is a well-established header-only library built on top of Google's absl::flat_hash_map, widely used in production OLAP systems like Apache Doris. It was the library suggested in the original issue.All 15 public methods are preserved with identical signatures, so LogManager.cpp requires no changes. The only method removed is eraseIf, which was dead code — never called anywhere in the codebase. Template defaults (std::hash, std::equal_to) are kept the same to maintain backward compatibility with StreamIDShard's existing std::hash specialization.
Benchmark references from the original issue:
Note: The .gitmodules merge conflict is due to new submodules added to
developsince my fork was last synced. Happy to rebase onto latestdevelopif needed — just let me know. The actual code change (ConcurrentHashMap.h) merges cleanly.