Commit d25d17e7 authored by Phil Willoughby's avatar Phil Willoughby Committed by Facebook Github Bot

Dead shift in ConcurrentHashMapSegment

Summary: Original problem detected by compiling with `-Werror,-Wunused-value`. On further inspection the only place which uses this detail class ensures that the `max_size` parameter is a power of two already, so we can discard the logic to manipulate `max_size` and put a `CHECK` clause in its place to guard against future changes to the caller that break this assumption.

Reviewed By: yfeldblum

Differential Revision: D5799110

fbshipit-source-id: d21ed9ff196d54ef91e38254df8b1b88bbf29275
parent 001275d1
...@@ -467,7 +467,7 @@ class ConcurrentHashMap { ...@@ -467,7 +467,7 @@ class ConcurrentHashMap {
auto seg = segments_[i].load(std::memory_order_acquire); auto seg = segments_[i].load(std::memory_order_acquire);
if (!seg) { if (!seg) {
auto newseg = (SegmentT*)Allocator().allocate(sizeof(SegmentT)); auto newseg = (SegmentT*)Allocator().allocate(sizeof(SegmentT));
new (newseg) newseg = new (newseg)
SegmentT(size_ >> ShardBits, load_factor_, max_size_ >> ShardBits); SegmentT(size_ >> ShardBits, load_factor_, max_size_ >> ShardBits);
if (!segments_[i].compare_exchange_strong(seg, newseg)) { if (!segments_[i].compare_exchange_strong(seg, newseg)) {
// seg is updated with new value, delete ours. // seg is updated with new value, delete ours.
......
...@@ -215,17 +215,13 @@ class FOLLY_ALIGNED(64) ConcurrentHashMapSegment { ...@@ -215,17 +215,13 @@ class FOLLY_ALIGNED(64) ConcurrentHashMapSegment {
size_t initial_buckets, size_t initial_buckets,
float load_factor, float load_factor,
size_t max_size) size_t max_size)
: load_factor_(load_factor) { : load_factor_(load_factor), max_size_(max_size) {
auto buckets = (Buckets*)Allocator().allocate(sizeof(Buckets)); auto buckets = (Buckets*)Allocator().allocate(sizeof(Buckets));
initial_buckets = folly::nextPowTwo(initial_buckets); initial_buckets = folly::nextPowTwo(initial_buckets);
if (max_size != 0) { DCHECK(
max_size_ = folly::nextPowTwo(max_size); max_size_ == 0 ||
} (isPowTwo(max_size_) &&
if (max_size_ > max_size) { (folly::popcount(max_size_ - 1) + ShardBits <= 32)));
max_size_ >> 1;
}
CHECK(max_size_ == 0 || (folly::popcount(max_size_ - 1) + ShardBits <= 32));
new (buckets) Buckets(initial_buckets); new (buckets) Buckets(initial_buckets);
buckets_.store(buckets, std::memory_order_release); buckets_.store(buckets, std::memory_order_release);
load_factor_nodes_ = initial_buckets * load_factor_; load_factor_nodes_ = initial_buckets * load_factor_;
...@@ -734,7 +730,7 @@ class FOLLY_ALIGNED(64) ConcurrentHashMapSegment { ...@@ -734,7 +730,7 @@ class FOLLY_ALIGNED(64) ConcurrentHashMapSegment {
float load_factor_; float load_factor_;
size_t load_factor_nodes_; size_t load_factor_nodes_;
size_t size_{0}; size_t size_{0};
size_t max_size_{0}; size_t const max_size_;
Atom<Buckets*> buckets_{nullptr}; Atom<Buckets*> buckets_{nullptr};
Mutex m_; Mutex m_;
}; };
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment