From 4851356b2712d17143879e4b911b5c0afee14e4b Mon Sep 17 00:00:00 2001 From: Lukas Krenz Date: Tue, 21 Jan 2025 15:47:21 +0000 Subject: [PATCH 1/2] Fix tests by using results --- folly/concurrency/test/ConcurrentHashMapBench.cpp | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/folly/concurrency/test/ConcurrentHashMapBench.cpp b/folly/concurrency/test/ConcurrentHashMapBench.cpp index 25e37cf585f..e18fa710377 100644 --- a/folly/concurrency/test/ConcurrentHashMapBench.cpp +++ b/folly/concurrency/test/ConcurrentHashMapBench.cpp @@ -21,6 +21,7 @@ #include #include +#include DEFINE_int32(reps, 10, "number of reps"); DEFINE_int32(ops, 1000 * 1000, "number of operations per rep"); @@ -92,7 +93,7 @@ uint64_t bench_ctor_dtor( for (int i = 0; i < ops; ++i) { folly::ConcurrentHashMap m; for (int j = 0; j < size; ++j) { - m.insert(j, j); + folly::doNotOptimizeAway(m.insert(j, j)); } } }; @@ -114,11 +115,11 @@ uint64_t bench_find( auto fn = [&](int) { if (sameItem) { for (int i = 0; i < ops; ++i) { - m.find(key); + folly::doNotOptimizeAway(m.find(key)); } } else { for (int i = 0; i < ops; ++i) { - m.find(i); + folly::doNotOptimizeAway(m.find(i)); } } }; @@ -138,7 +139,7 @@ uint64_t bench_iter(const int nthr, int size, const std::string& name) { auto repFn = [&] { auto fn = [&](int) { for (int i = 0; i < reps; ++i) { - for (auto it = m.begin(); it != m.end(); ++it) { + for (auto it = m.begin(); it != m.end(); doNotOptimizeAway(++it)) { } } }; @@ -157,7 +158,7 @@ uint64_t bench_begin(const int nthr, int size, const std::string& name) { auto repFn = [&] { auto fn = [&](int) { for (int i = 0; i < ops; ++i) { - auto it = m.begin(); + folly::doNotOptimizeAway(m.begin()); } }; auto endfn = [&] {}; @@ -175,7 +176,7 @@ uint64_t bench_empty(const int nthr, int size, const std::string& name) { auto repFn = [&] { auto fn = [&](int) { for (int i = 0; i < ops; ++i) { - m.empty(); + folly::doNotOptimizeAway(m.empty()); } }; auto endfn = [&] {}; @@ -193,7 +194,7 @@ uint64_t bench_size(const int nthr, int size, const std::string& name) { auto repFn = [&] { auto fn = [&](int) { for (int i = 0; i < ops; ++i) { - m.size(); + folly::doNotOptimizeAway(m.size()); } }; auto endfn = [&] {}; From 832f726964d1dbf8a1c18369e5d8a25944cbb244 Mon Sep 17 00:00:00 2001 From: Lukas Krenz Date: Tue, 21 Jan 2025 15:50:09 +0000 Subject: [PATCH 2/2] Add more inlining hints to hashmap Improves performance of begin/find for clang --- folly/concurrency/ConcurrentHashMap.h | 11 +++++++---- folly/concurrency/detail/ConcurrentHashMap-detail.h | 6 +++--- 2 files changed, 10 insertions(+), 7 deletions(-) diff --git a/folly/concurrency/ConcurrentHashMap.h b/folly/concurrency/ConcurrentHashMap.h index 7375d8adffa..7803b36ff43 100644 --- a/folly/concurrency/ConcurrentHashMap.h +++ b/folly/concurrency/ConcurrentHashMap.h @@ -288,10 +288,12 @@ class ConcurrentHashMap { return true; } - ConstIterator find(const KeyType& k) const { return findImpl(k); } + FOLLY_ALWAYS_INLINE ConstIterator find(const KeyType& k) const { + return findImpl(k); + } template = 0> - ConstIterator find(const K& k) const { + FOLLY_ALWAYS_INLINE ConstIterator find(const K& k) const { return findImpl(k); } @@ -604,7 +606,8 @@ class ConcurrentHashMap { segment_(std::exchange(o.segment_, uint64_t(NumShards))), parent_(std::exchange(o.parent_, nullptr)) {} - ConstIterator(const ConcurrentHashMap* parent, uint64_t segment) + FOLLY_ALWAYS_INLINE ConstIterator( + const ConcurrentHashMap* parent, uint64_t segment) : segment_(segment), parent_(parent) {} private: @@ -656,7 +659,7 @@ class ConcurrentHashMap { private: template - ConstIterator findImpl(const K& k) const { + FOLLY_ALWAYS_INLINE ConstIterator findImpl(const K& k) const { auto h = HashFn{}(k); auto segment = pickSegment(h); ConstIterator res(this, segment); diff --git a/folly/concurrency/detail/ConcurrentHashMap-detail.h b/folly/concurrency/detail/ConcurrentHashMap-detail.h index 51be7f28372..4906cffe130 100644 --- a/folly/concurrency/detail/ConcurrentHashMap-detail.h +++ b/folly/concurrency/detail/ConcurrentHashMap-detail.h @@ -482,7 +482,7 @@ class alignas(64) BucketTable { bucket_count_.load(std::memory_order_relaxed) * load_factor_; } - Iterator cbegin() { + FOLLY_ALWAYS_INLINE Iterator cbegin() { Iterator res; size_t bcount; Buckets* buckets; @@ -605,7 +605,7 @@ class alignas(64) BucketTable { Iterator& operator=(const Iterator& o) = delete; - Iterator& operator=(Iterator&& o) noexcept { + FOLLY_ALWAYS_INLINE Iterator& operator=(Iterator&& o) noexcept { if (this != &o) { hazptrs_ = std::move(o.hazptrs_); node_ = std::exchange(o.node_, nullptr); @@ -1899,7 +1899,7 @@ class alignas(64) ConcurrentHashMapSegment { void max_load_factor(float factor) { impl_.max_load_factor(factor); } - Iterator cbegin() { return impl_.cbegin(); } + FOLLY_ALWAYS_INLINE Iterator cbegin() { return impl_.cbegin(); } Iterator cend() { return impl_.cend(); }