From 4d07d7d0535735640d25799beceb1d6068faa549 Mon Sep 17 00:00:00 2001 From: Roman Gershman Date: Tue, 6 May 2025 12:33:00 +0300 Subject: [PATCH] chore: dash table clean ups (#5064) Remove stash template parameter because we only use dashtable with a single configuration of STASH_CNT=4. Signed-off-by: Roman Gershman --- src/core/dash.h | 8 ++-- src/core/dash_internal.h | 79 +++++++++++++++++---------------------- src/core/dash_test.cc | 14 +++---- src/server/detail/table.h | 4 +- 4 files changed, 48 insertions(+), 57 deletions(-) diff --git a/src/core/dash.h b/src/core/dash.h index 17b625c59..00676e3b9 100644 --- a/src/core/dash.h +++ b/src/core/dash.h @@ -16,7 +16,7 @@ namespace dfly { // After all, we added additional improvements we added as part of the dragonfly project, // that probably justify a right to choose our own name for this data structure. struct BasicDashPolicy { - enum { kSlotNum = 12, kBucketNum = 64, kStashBucketNum = 2 }; + enum { kSlotNum = 12, kBucketNum = 64 }; static constexpr bool kUseVersion = false; template static void DestroyValue(const U&) { @@ -62,11 +62,11 @@ class DashTable : public detail::DashTableBase { struct HotspotBuckets { static constexpr size_t kRegularBuckets = 4; - static constexpr size_t kNumBuckets = kRegularBuckets + Policy::kStashBucketNum; + static constexpr size_t kNumBuckets = kRegularBuckets + SegmentType::kStashBucketNum; struct ByType { bucket_iterator regular_buckets[kRegularBuckets]; - bucket_iterator stash_buckets[Policy::kStashBucketNum]; + bucket_iterator stash_buckets[SegmentType::kStashBucketNum]; }; union Probes { @@ -822,7 +822,7 @@ auto DashTable<_Key, _Value, Policy>::InsertInternal(U&& key, V&& value, Evictio hotspot.probes.by_type.regular_buckets[j] = bucket_iterator{this, target_seg_id, bid[j]}; } - for (unsigned i = 0; i < Policy::kStashBucketNum; ++i) { + for (unsigned i = 0; i < SegmentType::kStashBucketNum; ++i) { hotspot.probes.by_type.stash_buckets[i] = bucket_iterator{this, target_seg_id, uint8_t(Policy::kBucketNum + i), 0}; } diff --git a/src/core/dash_internal.h b/src/core/dash_internal.h index 7081ca33b..7d0ebac96 100644 --- a/src/core/dash_internal.h +++ b/src/core/dash_internal.h @@ -101,19 +101,17 @@ template class SlotBitmap { Unaligned val_[kLen]; }; // SlotBitmap -template class BucketBase { +template class BucketBase { // We can not allow more than 4 stash fps because we hold stash positions in single byte // stash_pos_ variable that uses 2 bits per stash bucket to point which bucket holds that fp. // Hence we can point at most from 4 fps to 4 stash buckets. // If any of those limits need to be raised we should increase stash_pos_ similarly to how we did // with SlotBitmap. - static_assert(NUM_STASH_FPS <= 4, "Can only hold at most 4 fp slots"); - - static constexpr unsigned kStashFpLen = NUM_STASH_FPS; + static constexpr unsigned kStashFpLen = 4; static constexpr unsigned kStashPresentBit = 1 << 4; using FpArray = std::array; - using StashFpArray = std::array; + using StashFpArray = std::array; public: using SlotId = uint8_t; @@ -233,9 +231,9 @@ template class BucketBase { uint8_t overflow_count_ = 0; }; // BucketBase -static_assert(sizeof(BucketBase<12, 4>) == 24, ""); -static_assert(alignof(BucketBase<14, 4>) == 1, ""); -static_assert(alignof(BucketBase<12, 4>) == 1, ""); +static_assert(sizeof(BucketBase<12>) == 24); +static_assert(alignof(BucketBase<14>) == 1); +static_assert(alignof(BucketBase<12>) == 1); // Optional version support as part of DashTable. // This works like this: each slot has 2 bytes for version and a bucket has another 6. @@ -243,9 +241,8 @@ static_assert(alignof(BucketBase<12, 4>) == 1, ""); // In order to achieve this we store high6(max{version(entry)}) for every entry. // Hence our version control may have false positives, i.e. signal that an entry has changed // when in practice its neighbour incremented the high6 part of its bucket. -template -class VersionedBB : public BucketBase { - using Base = BucketBase; +template class VersionedBB : public BucketBase { + using Base = BucketBase; public: // one common version per bucket. @@ -286,15 +283,14 @@ class VersionedBB : public BucketBase { // std::array low_ = {0}; }; -static_assert(alignof(VersionedBB<14, 4>) == 1, ""); -static_assert(sizeof(VersionedBB<12, 4>) == 12 * 2 + 8, ""); -static_assert(sizeof(VersionedBB<14, 4>) <= 14 * 2 + 8, ""); +static_assert(alignof(VersionedBB<14>) == 1); +static_assert(sizeof(VersionedBB<12>) == 12 * 2 + 8); +static_assert(sizeof(VersionedBB<14>) <= 14 * 2 + 8); // Segment - static-hashtable of size kSlotNum*(kBucketNum + kStashBucketNum). struct DefaultSegmentPolicy { static constexpr unsigned kSlotNum = 12; static constexpr unsigned kBucketNum = 64; - static constexpr unsigned kStashBucketNum = 2; static constexpr bool kUseVersion = true; }; @@ -302,15 +298,14 @@ template , BucketBase>; + using BucketType = std::conditional_t, BucketBase>; struct Bucket : public BucketType { using BucketType::kNanSlot; @@ -613,14 +608,14 @@ template -bool BucketBase::ClearStash(uint8_t fp, unsigned stash_pos, bool probe) { +template +bool BucketBase::ClearStash(uint8_t fp, unsigned stash_pos, bool probe) { auto cb = [stash_pos, this](unsigned i, unsigned pos) -> SlotId { if (pos == stash_pos) { stash_busy_ &= (~(1u << i)); @@ -828,16 +823,16 @@ bool BucketBase::ClearStash(uint8_t fp, unsigned stash_pos, return res.second != kNanSlot; } -template -void BucketBase::SetHash(unsigned slot_id, uint8_t meta_hash, bool probe) { +template +void BucketBase::SetHash(unsigned slot_id, uint8_t meta_hash, bool probe) { assert(slot_id < finger_arr_.size()); finger_arr_[slot_id] = meta_hash; slotb_.SetSlot(slot_id, probe); } -template -bool BucketBase::SetStash(uint8_t fp, unsigned stash_pos, bool probe) { +template +bool BucketBase::SetStash(uint8_t fp, unsigned stash_pos, bool probe) { // stash_busy_ is never 0xFFFFF so it's safe to run __builtin_ctz below. unsigned free_slot = __builtin_ctz(~stash_busy_); if (free_slot >= kStashFpLen) @@ -856,9 +851,8 @@ bool BucketBase::SetStash(uint8_t fp, unsigned stash_pos, bo return true; } -template -void BucketBase::SetStashPtr(unsigned stash_pos, uint8_t meta_hash, - BucketBase* next) { +template +void BucketBase::SetStashPtr(unsigned stash_pos, uint8_t meta_hash, BucketBase* next) { assert(stash_pos < 4); // we use only kStashFpLen fp slots for handling stash buckets, @@ -874,9 +868,9 @@ void BucketBase::SetStashPtr(unsigned stash_pos, uint8_t met stash_busy_ |= kStashPresentBit; } -template -unsigned BucketBase::UnsetStashPtr(uint8_t fp_hash, unsigned stash_pos, - BucketBase* next) { +template +unsigned BucketBase::UnsetStashPtr(uint8_t fp_hash, unsigned stash_pos, + BucketBase* next) { /*also needs to ensure that this meta_hash must belongs to other bucket*/ bool clear_success = ClearStash(fp_hash, stash_pos, false); unsigned res = 0; @@ -907,8 +901,7 @@ unsigned BucketBase::UnsetStashPtr(uint8_t fp_hash, unsigned } #ifdef __s390x__ -template -uint32_t BucketBase::CompareFP(uint8_t fp) const { +template uint32_t BucketBase::CompareFP(uint8_t fp) const { static_assert(FpArray{}.size() <= 16); vector unsigned char v1; @@ -934,8 +927,7 @@ uint32_t BucketBase::CompareFP(uint8_t fp) const { return mask; } #else -template -uint32_t BucketBase::CompareFP(uint8_t fp) const { +template uint32_t BucketBase::CompareFP(uint8_t fp) const { static_assert(FpArray{}.size() <= 16); // Replicate 16 times fp to key_data. @@ -958,7 +950,7 @@ uint32_t BucketBase::CompareFP(uint8_t fp) const { // Bucket slot array goes from left to right: [x, x, ...] // Shift right vacates the first slot on the left by shifting all the elements right and // possibly deleting the last one on the right. -template bool BucketBase::ShiftRight() { +template bool BucketBase::ShiftRight() { for (int i = NUM_SLOTS - 1; i > 0; --i) { finger_arr_[i] = finger_arr_[i - 1]; } @@ -970,9 +962,9 @@ template bool BucketBase +template template -auto BucketBase::IterateStash(uint8_t fp, bool is_probe, F&& func) const +auto BucketBase::IterateStash(uint8_t fp, bool is_probe, F&& func) const -> ::std::pair { unsigned om = is_probe ? stash_probe_mask_ : ~stash_probe_mask_; unsigned ob = stash_busy_; @@ -991,14 +983,13 @@ auto BucketBase::IterateStash(uint8_t fp, bool is_probe, F&& return std::pair(0, BucketBase::kNanSlot); } -template -void VersionedBB::SetVersion(uint64_t version) { +template void VersionedBB::SetVersion(uint64_t version) { absl::little_endian::Store64(version_, version); } #if 0 -template -uint64_t VersionedBB::MinVersion() const { +template +uint64_t VersionedBB::MinVersion() const { uint32_t mask = this->GetBusy(); if (mask == 0) return 0; diff --git a/src/core/dash_test.cc b/src/core/dash_test.cc index 304a67917..6f8adf7cd 100644 --- a/src/core/dash_test.cc +++ b/src/core/dash_test.cc @@ -240,8 +240,8 @@ TEST_F(DashTest, Segment) { for (size_t i = 2; i < Segment::kBucketNum; ++i) { EXPECT_EQ(0, segment_.GetBucket(i).Size()); } - EXPECT_EQ(4 * Segment::kSlotNum, keys.size()); - EXPECT_EQ(4 * Segment::kSlotNum, segment_.SlowSize()); + EXPECT_EQ(6 * Segment::kSlotNum, keys.size()); + EXPECT_EQ(6 * Segment::kSlotNum, segment_.SlowSize()); auto hfun = &UInt64Policy::HashFn; unsigned has_called = 0; @@ -268,7 +268,7 @@ TEST_F(DashTest, Segment) { ASSERT_TRUE(it.found()); segment_.Delete(it, hash); } - EXPECT_EQ(2 * Segment::kSlotNum, segment_.SlowSize()); + EXPECT_EQ(4 * Segment::kSlotNum, segment_.SlowSize()); ASSERT_FALSE(Contains(arr.front())); } @@ -331,7 +331,7 @@ TEST_F(DashTest, Split) { ASSERT_EQ(segment_.SlowSize(), sum[0]); EXPECT_EQ(s2.SlowSize(), sum[1]); EXPECT_EQ(keys.size(), sum[0] + sum[1]); - EXPECT_EQ(4 * Segment::kSlotNum, keys.size()); + EXPECT_EQ(6 * Segment::kSlotNum, keys.size()); } TEST_F(DashTest, Merge) { @@ -456,12 +456,12 @@ struct Item { constexpr size_t ItemAlign = alignof(Item); -struct MyBucket : public detail::BucketBase<16, 4> { +struct MyBucket : public detail::BucketBase<16> { Item key[14]; }; constexpr size_t kMySz = sizeof(MyBucket); -constexpr size_t kBBSz = sizeof(detail::BucketBase<16, 4>); +constexpr size_t kBBSz = sizeof(detail::BucketBase<16>); TEST_F(DashTest, Custom) { using ItemSegment = detail::Segment; @@ -659,7 +659,7 @@ struct TestEvictionPolicy { }; TEST_F(DashTest, Eviction) { - TestEvictionPolicy ev(1500); + TestEvictionPolicy ev(1540); size_t num = 0; auto loop = [&] { diff --git a/src/server/detail/table.h b/src/server/detail/table.h index 11e6c96e8..993f2aab6 100644 --- a/src/server/detail/table.h +++ b/src/server/detail/table.h @@ -16,7 +16,7 @@ using PrimeKey = CompactObj; using PrimeValue = CompactObj; struct PrimeTablePolicy { - enum { kSlotNum = 14, kBucketNum = 56, kStashBucketNum = 4 }; + enum { kSlotNum = 14, kBucketNum = 56 }; static constexpr bool kUseVersion = true; @@ -46,7 +46,7 @@ struct PrimeTablePolicy { }; struct ExpireTablePolicy { - enum { kSlotNum = 14, kBucketNum = 56, kStashBucketNum = 4 }; + enum { kSlotNum = 14, kBucketNum = 56 }; static constexpr bool kUseVersion = false; static uint64_t HashFn(const PrimeKey& s) {