Skip to content

Commit

Permalink
windows build failed in speedb_paired_bloom_internal files due to bad…
Browse files Browse the repository at this point in the history
… used of double / uint32 casting #383
  • Loading branch information
ayulas authored and Yuval-Ariel committed Apr 30, 2023
1 parent 43876b2 commit c4d8c09
Show file tree
Hide file tree
Showing 5 changed files with 45 additions and 36 deletions.
31 changes: 16 additions & 15 deletions plugin/speedb/memtable/hash_spd_rep.cc
Original file line number Diff line number Diff line change
Expand Up @@ -382,22 +382,19 @@ void SpdbVectorContainer::SeekIter(const IterAnchors& iter_anchor,
}

void SpdbVectorContainer::Merge(std::list<SpdbVectorPtr>::iterator& begin,
std::list<SpdbVectorPtr>::iterator& end) {
std::list<SpdbVectorPtr>::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_);
Expand All @@ -411,26 +408,30 @@ bool SpdbVectorContainer::TryMergeVectors(
std::list<SpdbVectorPtr>::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;
Expand Down
3 changes: 2 additions & 1 deletion plugin/speedb/memtable/spdb_sorted_vector.h
Original file line number Diff line number Diff line change
Expand Up @@ -225,7 +225,8 @@ class SpdbVectorContainer {
bool TryMergeVectors(std::list<SpdbVectorPtr>::iterator last);

void Merge(std::list<SpdbVectorPtr>::iterator& start,
std::list<SpdbVectorPtr>::iterator& last);
std::list<SpdbVectorPtr>::iterator& last,
size_t total_merge_vector_elements);

private:
port::RWMutexWr spdb_vectors_add_rwlock_;
Expand Down
39 changes: 21 additions & 18 deletions plugin/speedb/paired_filter/speedb_paired_bloom_internal.cc
Original file line number Diff line number Diff line change
Expand Up @@ -46,20 +46,21 @@ 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<uint32_t>(
std::ceil(std::log2(speedb_filter::kPairedBloomBatchSizeInBlocks)));

// kBlockSizeInBytes must be a power of 2 (= Cacheline size)
constexpr uint32_t kBlockSizeInBytes = 64U;
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<uint32_t>(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<uint32_t>(std::log2(kBlockSizeInBytes));
static const uint32_t KNumBitsInBlockBloom =
kBlockSizeInBits - kInBatchIdxNumBits;

Expand Down Expand Up @@ -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 <
Expand All @@ -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<double>((millibits_per_key * KNumBitsInBlockBloom) /
kBlockSizeInBits / 1000);
}

inline double CalcRawNumProbes(size_t millibits_per_key) {
Expand Down Expand Up @@ -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;
Expand All @@ -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;
Expand All @@ -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);
Expand All @@ -225,15 +227,15 @@ 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:
#ifdef HAVE_AVX2
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:
Expand All @@ -252,7 +254,7 @@ inline uint8_t ReadBlock::GetInBatchBlockIdxOfPair() const {
return static_cast<uint8_t>(*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
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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]
Expand Down Expand Up @@ -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<uint8_t>(pair_in_batch_block_idx));
}
}
}
Expand Down Expand Up @@ -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;
Expand Down
4 changes: 2 additions & 2 deletions plugin/speedb/paired_filter/speedb_paired_bloom_internal.h
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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;
};

Expand Down
4 changes: 4 additions & 0 deletions port/win/port_win.h
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,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:
Expand Down

0 comments on commit c4d8c09

Please sign in to comment.