From e02a4eb35c5b9c40258ce860ffc339f84e2e078d Mon Sep 17 00:00:00 2001 From: Ayelet Shani Date: Thu, 2 Feb 2023 15:20:44 +0200 Subject: [PATCH] windows build failed in speedb_paired_bloom_internal files due to bad used of double / uint32 casting #383 --- plugin/speedb/memtable/hash_spd_rep.cc | 31 ++++++++------- plugin/speedb/memtable/spdb_sorted_vector.h | 3 +- .../speedb_paired_bloom_internal.cc | 39 ++++++++++--------- .../speedb_paired_bloom_internal.h | 4 +- port/win/port_win.h | 4 ++ 5 files changed, 45 insertions(+), 36 deletions(-) diff --git a/plugin/speedb/memtable/hash_spd_rep.cc b/plugin/speedb/memtable/hash_spd_rep.cc index 69e2d7aed5..2afeb55867 100644 --- a/plugin/speedb/memtable/hash_spd_rep.cc +++ b/plugin/speedb/memtable/hash_spd_rep.cc @@ -382,22 +382,19 @@ void SpdbVectorContainer::SeekIter(const IterAnchors& iter_anchor, } void SpdbVectorContainer::Merge(std::list::iterator& begin, - std::list::iterator& end) { + std::list::iterator& end, + size_t total_merge_vector_elements) { SpdbVectorIterator iterator(this, comparator_, begin, end); - const size_t num_elements = std::accumulate( - begin, end, 0, - [](size_t n, const SpdbVectorPtr& vec) { return n + vec->Size(); }); - if (num_elements > 0) { + if (total_merge_vector_elements > 0) { SpdbVector::Vec merged; - merged.reserve(num_elements); + merged.reserve(total_merge_vector_elements); for (iterator.SeekToFirst(); iterator.Valid(); iterator.Next()) { merged.emplace_back(iterator.key()); } - const size_t actual_elements_count = merged.size(); SpdbVectorPtr new_vector( - new SpdbVector(std::move(merged), actual_elements_count)); + new SpdbVector(std::move(merged), total_merge_vector_elements)); // now replace WriteLock wl(&spdb_vectors_merge_rwlock_); @@ -411,26 +408,30 @@ bool SpdbVectorContainer::TryMergeVectors( std::list::iterator start = spdb_vectors_.begin(); const size_t merge_threshold = switch_spdb_vector_limit_ * 75 / 100; - size_t count = 0; + size_t merge_vectors_count = 0; + size_t total_merge_vector_elements = 0; + for (auto s = start; s != last; ++s) { if ((*s)->Size() > merge_threshold) { - if (count > 1) { + if (merge_vectors_count > 1) { last = s; break; } - count = 0; + merge_vectors_count = 0; + total_merge_vector_elements = 0; start = std::next(s); } else { - ++count; - if (count == kMergedVectorsMax) { + ++merge_vectors_count; + total_merge_vector_elements += (*s)->Size(); + if (merge_vectors_count == kMergedVectorsMax) { last = std::next(s); break; } } } - if (count > 1) { - Merge(start, last); + if (merge_vectors_count > 1) { + Merge(start, last, total_merge_vector_elements); return true; } return false; diff --git a/plugin/speedb/memtable/spdb_sorted_vector.h b/plugin/speedb/memtable/spdb_sorted_vector.h index a88cbdb90f..719992bfd6 100644 --- a/plugin/speedb/memtable/spdb_sorted_vector.h +++ b/plugin/speedb/memtable/spdb_sorted_vector.h @@ -225,7 +225,8 @@ class SpdbVectorContainer { bool TryMergeVectors(std::list::iterator last); void Merge(std::list::iterator& start, - std::list::iterator& last); + std::list::iterator& last, + size_t total_merge_vector_elements); private: port::RWMutexWr spdb_vectors_add_rwlock_; diff --git a/plugin/speedb/paired_filter/speedb_paired_bloom_internal.cc b/plugin/speedb/paired_filter/speedb_paired_bloom_internal.cc index 63690770e4..17cbf5e15e 100644 --- a/plugin/speedb/paired_filter/speedb_paired_bloom_internal.cc +++ b/plugin/speedb/paired_filter/speedb_paired_bloom_internal.cc @@ -46,8 +46,8 @@ static_assert((speedb_filter::kPairedBloomBatchSizeInBlocks > 0) && (speedb_filter::kPairedBloomBatchSizeInBlocks - 1)) == 0)); // Number of bits to point to any block in a batch (in-batch block index) -static const uint32_t kInBatchIdxNumBits = - std::ceil(std::log2(speedb_filter::kPairedBloomBatchSizeInBlocks)); +static const uint32_t kInBatchIdxNumBits = static_cast( + std::ceil(std::log2(speedb_filter::kPairedBloomBatchSizeInBlocks))); // kBlockSizeInBytes must be a power of 2 (= Cacheline size) constexpr uint32_t kBlockSizeInBytes = 64U; @@ -55,11 +55,12 @@ static_assert((kBlockSizeInBytes > 0) && ((kBlockSizeInBytes & (kBlockSizeInBytes - 1)) == 0)); constexpr uint32_t kBlockSizeInBits = kBlockSizeInBytes * 8U; static const uint32_t kBlockSizeNumBits = - std::ceil(std::log2(kBlockSizeInBits)); + static_cast(std::ceil(std::log2(kBlockSizeInBits))); static const uint32_t kNumBlockSizeBitsShiftBits = 32 - kBlockSizeNumBits; // Number of bits to represent kBlockSizeInBytes -static const uint32_t kNumBitsForBlockSize = std::log2(kBlockSizeInBytes); +static const uint32_t kNumBitsForBlockSize = + static_cast(std::log2(kBlockSizeInBytes)); static const uint32_t KNumBitsInBlockBloom = kBlockSizeInBits - kInBatchIdxNumBits; @@ -99,8 +100,8 @@ inline uint8_t GetInBatchBlockIdx(uint32_t global_block_idx) { return (global_block_idx % speedb_filter::kPairedBloomBatchSizeInBlocks); } -inline uint32_t GetHashSetSelector(uint32_t first_in_batch_block_idx, - uint32_t second_in_batch_block_idx) { +inline uint8_t GetHashSetSelector(uint32_t first_in_batch_block_idx, + uint32_t second_in_batch_block_idx) { assert((first_in_batch_block_idx < speedb_filter::kPairedBloomBatchSizeInBlocks) && (second_in_batch_block_idx < @@ -122,7 +123,8 @@ inline const char* GetBlockAddress(const char* data, } inline double CalcAdjustedBitsPerKey(size_t millibits_per_key) { - return ((millibits_per_key * KNumBitsInBlockBloom) / kBlockSizeInBits / 1000); + return static_cast((millibits_per_key * KNumBitsInBlockBloom) / + kBlockSizeInBits / 1000); } inline double CalcRawNumProbes(size_t millibits_per_key) { @@ -157,8 +159,8 @@ class BuildBlock { BuildBlock(char* data, uint32_t global_block_idx, bool prefetch_block); uint8_t GetInBatchBlockIdxOfPair() const; - void SetInBatchBlockIdxOfPair(InBatchBlockIdx pair_batch_block_idx); - void SetBlockBloomBits(uint32_t hash, uint32_t set_idx, size_t hash_set_size); + void SetInBatchBlockIdxOfPair(uint8_t pair_batch_block_idx); + void SetBlockBloomBits(uint32_t hash, uint8_t set_idx, size_t hash_set_size); private: char* const block_address_ = nullptr; @@ -185,7 +187,7 @@ inline void BuildBlock::SetInBatchBlockIdxOfPair( (pair_batch_block_idx | (*block_address_ & kFirstByteBitsMask)); } -inline int GetBitPosInBlockForHash(uint32_t hash, uint32_t set_idx) { +inline int GetBitPosInBlockForHash(uint32_t hash, uint8_t set_idx) { assert(set_idx <= 1U); int bitpos = 0; @@ -210,7 +212,7 @@ inline int GetBitPosInBlockForHash(uint32_t hash, uint32_t set_idx) { (kNumBlockSizeBitsShiftBits)); } -inline void BuildBlock::SetBlockBloomBits(uint32_t hash, uint32_t set_idx, +inline void BuildBlock::SetBlockBloomBits(uint32_t hash, uint8_t set_idx, size_t hash_set_size) { for (auto i = 0U; i < hash_set_size; ++i) { int bitpos = GetBitPosInBlockForHash(hash, set_idx); @@ -225,7 +227,7 @@ class ReadBlock { ReadBlock(const char* data, uint32_t global_block_idx, bool prefetch_block); uint8_t GetInBatchBlockIdxOfPair() const; - bool AreAllBlockBloomBitsSet(uint32_t hash, uint32_t set_idx, + bool AreAllBlockBloomBitsSet(uint32_t hash, uint8_t set_idx, size_t hash_set_size) const; private: @@ -233,7 +235,7 @@ class ReadBlock { bool AreAllBlockBloomBitsSetAvx2(uint32_t hash, uint32_t set_idx, size_t hash_set_size) const; #endif - bool AreAllBlockBloomBitsSetNonAvx2(uint32_t hash, uint32_t set_idx, + bool AreAllBlockBloomBitsSetNonAvx2(uint32_t hash, uint8_t set_idx, size_t hash_set_size) const; private: @@ -252,7 +254,7 @@ inline uint8_t ReadBlock::GetInBatchBlockIdxOfPair() const { return static_cast(*block_address_) & kInBatchIdxMask; } -bool ReadBlock::AreAllBlockBloomBitsSet(uint32_t hash, uint32_t set_idx, +bool ReadBlock::AreAllBlockBloomBitsSet(uint32_t hash, uint8_t set_idx, size_t hash_set_size) const { #ifdef HAVE_AVX2 // The AVX2 code currently supports only cache-line / block sizes of 64 bytes @@ -361,7 +363,7 @@ bool ReadBlock::AreAllBlockBloomBitsSetAvx2(uint32_t hash, uint32_t set_idx, #endif // HAVE_AVX2 -bool ReadBlock::AreAllBlockBloomBitsSetNonAvx2(uint32_t hash, uint32_t set_idx, +bool ReadBlock::AreAllBlockBloomBitsSetNonAvx2(uint32_t hash, uint8_t set_idx, size_t hash_set_size) const { for (auto i = 0U; i < hash_set_size; ++i) { int bitpos = GetBitPosInBlockForHash(hash, set_idx); @@ -589,7 +591,7 @@ void SpdbPairedBloomBitsBuilder::InitBlockHistogram() { sizeof(decltype(blocks_histogram_)::value_type)); for (auto batch_idx = 0U; batch_idx < blocks_histogram_.size(); ++batch_idx) { - for (auto in_batch_block_idx = 0U; + for (uint8_t in_batch_block_idx = 0; in_batch_block_idx < blocks_histogram_[batch_idx].size(); ++in_batch_block_idx) { blocks_histogram_[batch_idx][in_batch_block_idx] @@ -657,7 +659,8 @@ void SpdbPairedBloomBitsBuilder::SetBlocksPairs(char* data) { BuildBlock block(data, global_block_idx, false /* prefetch */); const uint32_t pair_in_batch_block_idx = pairing_table_[batch_idx][in_batch_block_idx].pair_in_batch_block_idx; - block.SetInBatchBlockIdxOfPair(pair_in_batch_block_idx); + block.SetInBatchBlockIdxOfPair( + static_cast(pair_in_batch_block_idx)); } } } @@ -818,7 +821,7 @@ bool SpdbPairedBloomBitsReader::HashMayMatch(const uint64_t hash) { return false; } - uint32_t secondary_block_hash_selector = 1 - primary_block_hash_selector; + uint8_t secondary_block_hash_selector = 1 - primary_block_hash_selector; uint32_t batch_idx = GetContainingBatchIdx(primary_global_block_idx); uint32_t secondary_global_block_idx = GetFirstGlobalBlockIdxOfBatch(batch_idx) + secondary_in_batch_block_idx; diff --git a/plugin/speedb/paired_filter/speedb_paired_bloom_internal.h b/plugin/speedb/paired_filter/speedb_paired_bloom_internal.h index 39d7eec9ff..6f67eb7fd1 100644 --- a/plugin/speedb/paired_filter/speedb_paired_bloom_internal.h +++ b/plugin/speedb/paired_filter/speedb_paired_bloom_internal.h @@ -25,7 +25,7 @@ namespace ROCKSDB_NAMESPACE { namespace speedb_filter { inline constexpr size_t kPairedBloomBatchSizeInBlocks = 32U; // Max supported BPK. Filters using higher BPK-s will use the max -inline constexpr int kMinMillibitsPerKey = 1000.0; +inline constexpr int kMinMillibitsPerKey = 1000; // Types of proprietary Speedb's filters enum class FilterType : uint8_t { @@ -120,7 +120,7 @@ class SpdbPairedBloomBitsBuilder : public XXPH3FilterBitsBuilder { // Records the info about a block's pair in the batch struct PairingInfo { - uint8_t pair_in_batch_block_idx; + uint32_t pair_in_batch_block_idx; uint8_t hash_set_selector; }; diff --git a/port/win/port_win.h b/port/win/port_win.h index 5a8f660516..5806f07650 100644 --- a/port/win/port_win.h +++ b/port/win/port_win.h @@ -167,6 +167,10 @@ class RWMutex { private: SRWLOCK srwLock_; }; +// in linux env the RW suffers from write starvation therefor we created a new +// class inherited from the original RWMutex that allows balance priority in +// windows env we dont have this issue. +using RWMutexWr = RWMutex; class CondVar { public: