From 846db9d7b1d03cdce467a75e534e3e5f9aea2425 Mon Sep 17 00:00:00 2001 From: Peter Dillinger Date: Tue, 18 Jul 2023 12:09:27 -0700 Subject: [PATCH] Refactor ClockCache ApplyToEntries (#11609) Summary: ... ahead of planned dynamic HCC variant. This changes simplifies some logic while still enabling future code sharing between implementation variants. Detail: For complicated reasons, using a std::function parameter to `ConstApplyToEntriesRange` with a lambda argument does not play nice with templated HandleImpl. An explicit conversion to std::function would be needed for it to compile. Templating the function type is the easy work-around. Also made some functions from https://github.com/facebook/rocksdb/issues/11572 private as recommended Pull Request resolved: https://github.com/facebook/rocksdb/pull/11609 Test Plan: existing tests Reviewed By: jowlyzhang Differential Revision: D47407415 Pulled By: pdillinger fbshipit-source-id: 0f65954db16335999b78fb7d2563ec627624cef0 --- cache/clock_cache.cc | 104 +++++++++++++++++++++---------------------- cache/clock_cache.h | 75 +++++++++++++++---------------- 2 files changed, 86 insertions(+), 93 deletions(-) diff --git a/cache/clock_cache.cc b/cache/clock_cache.cc index 4d4faeb30..e5a650d86 100644 --- a/cache/clock_cache.cc +++ b/cache/clock_cache.cc @@ -289,6 +289,46 @@ bool TryInsert(const ClockHandleBasicData& proto, ClockHandle& h, return b; } +template +void ConstApplyToEntriesRange(Func /*const HandleImpl& -> void*/ func, + const HandleImpl* begin, const HandleImpl* end, + bool apply_if_will_be_deleted) { + uint64_t check_state_mask = ClockHandle::kStateShareableBit; + if (!apply_if_will_be_deleted) { + check_state_mask |= ClockHandle::kStateVisibleBit; + } + + for (const HandleImpl* h = begin; h < end; ++h) { + // Note: to avoid using compare_exchange, we have to be extra careful. + uint64_t old_meta = h->meta.load(std::memory_order_relaxed); + // Check if it's an entry visible to lookups + if ((old_meta >> ClockHandle::kStateShift) & check_state_mask) { + // Increment acquire counter. Note: it's possible that the entry has + // completely changed since we loaded old_meta, but incrementing acquire + // count is always safe. (Similar to optimistic Lookup here.) + old_meta = h->meta.fetch_add(ClockHandle::kAcquireIncrement, + std::memory_order_acquire); + // Check whether we actually acquired a reference. + if ((old_meta >> ClockHandle::kStateShift) & + ClockHandle::kStateShareableBit) { + // Apply func if appropriate + if ((old_meta >> ClockHandle::kStateShift) & check_state_mask) { + func(*h); + } + // Pretend we never took the reference + h->meta.fetch_sub(ClockHandle::kAcquireIncrement, + std::memory_order_release); + // No net change, so don't need to check for overflow + } else { + // For other states, incrementing the acquire counter has no effect + // so we don't need to undo it. Furthermore, we cannot safely undo + // it because we did not acquire a read reference to lock the + // entry in a Shareable state. + } + } + } +} + } // namespace void ClockHandleBasicData::FreeData(MemoryAllocator* allocator) const { @@ -944,47 +984,6 @@ void HyperClockTable::Erase(const UniqueId64x2& hashed_key) { [&](HandleImpl* /*h*/, bool /*is_last*/) {}); } -void HyperClockTable::ConstApplyToEntriesRange( - std::function func, size_t index_begin, - size_t index_end, bool apply_if_will_be_deleted) const { - uint64_t check_state_mask = ClockHandle::kStateShareableBit; - if (!apply_if_will_be_deleted) { - check_state_mask |= ClockHandle::kStateVisibleBit; - } - - for (size_t i = index_begin; i < index_end; i++) { - HandleImpl& h = array_[i]; - - // Note: to avoid using compare_exchange, we have to be extra careful. - uint64_t old_meta = h.meta.load(std::memory_order_relaxed); - // Check if it's an entry visible to lookups - if ((old_meta >> ClockHandle::kStateShift) & check_state_mask) { - // Increment acquire counter. Note: it's possible that the entry has - // completely changed since we loaded old_meta, but incrementing acquire - // count is always safe. (Similar to optimistic Lookup here.) - old_meta = h.meta.fetch_add(ClockHandle::kAcquireIncrement, - std::memory_order_acquire); - // Check whether we actually acquired a reference. - if ((old_meta >> ClockHandle::kStateShift) & - ClockHandle::kStateShareableBit) { - // Apply func if appropriate - if ((old_meta >> ClockHandle::kStateShift) & check_state_mask) { - func(h); - } - // Pretend we never took the reference - h.meta.fetch_sub(ClockHandle::kAcquireIncrement, - std::memory_order_release); - // No net change, so don't need to check for overflow - } else { - // For other states, incrementing the acquire counter has no effect - // so we don't need to undo it. Furthermore, we cannot safely undo - // it because we did not acquire a read reference to lock the - // entry in a Shareable state. - } - } - } -} - void HyperClockTable::EraseUnRefEntries() { for (size_t i = 0; i <= this->length_bits_mask_; i++) { HandleImpl& h = array_[i]; @@ -1150,35 +1149,32 @@ void ClockCacheShard::ApplyToSomeEntries( size_t charge, const Cache::CacheItemHelper* helper)>& callback, size_t average_entries_per_lock, size_t* state) { - // The state is essentially going to be the starting hash, which works - // nicely even if we resize between calls because we use upper-most - // hash bits for table indexes. - size_t length_bits = table_.GetLengthBits(); + // The state will be a simple index into the table. Even with a dynamic + // hyper clock cache, entries will generally stay in their existing + // slots, so we don't need to be aware of the high-level organization + // that makes lookup efficient. size_t length = table_.GetTableSize(); assert(average_entries_per_lock > 0); - // Assuming we are called with same average_entries_per_lock repeatedly, - // this simplifies some logic (index_end will not overflow). - assert(average_entries_per_lock < length || *state == 0); - size_t index_begin = *state >> (sizeof(size_t) * 8u - length_bits); + size_t index_begin = *state; size_t index_end = index_begin + average_entries_per_lock; if (index_end >= length) { // Going to end. index_end = length; *state = SIZE_MAX; } else { - *state = index_end << (sizeof(size_t) * 8u - length_bits); + *state = index_end; } auto hash_seed = table_.GetHashSeed(); - table_.ConstApplyToEntriesRange( + ConstApplyToEntriesRange( [callback, hash_seed](const HandleImpl& h) { UniqueId64x2 unhashed; callback(ReverseHash(h.hashed_key, &unhashed, hash_seed), h.value, h.GetTotalCharge(), h.helper); }, - index_begin, index_end, false); + table_.HandlePtr(index_begin), table_.HandlePtr(index_end), false); } int HyperClockTable::CalcHashBits( @@ -1335,7 +1331,7 @@ size_t ClockCacheShard
::GetPinnedUsage() const { size_t table_pinned_usage = 0; const bool charge_metadata = metadata_charge_policy_ == kFullChargeCacheMetadata; - table_.ConstApplyToEntriesRange( + ConstApplyToEntriesRange( [&table_pinned_usage, charge_metadata](const HandleImpl& h) { uint64_t meta = h.meta.load(std::memory_order_relaxed); uint64_t refcount = GetRefcount(meta); @@ -1348,7 +1344,7 @@ size_t ClockCacheShard
::GetPinnedUsage() const { } } }, - 0, table_.GetTableSize(), true); + table_.HandlePtr(0), table_.HandlePtr(table_.GetTableSize()), true); return table_pinned_usage + table_.GetStandaloneUsage(); } diff --git a/cache/clock_cache.h b/cache/clock_cache.h index 992083e4f..fee2eb11a 100644 --- a/cache/clock_cache.h +++ b/cache/clock_cache.h @@ -371,8 +371,8 @@ struct ClockHandle : public ClockHandleBasicData { static constexpr uint8_t kMaxCountdown = kHighCountdown; // TODO: make these coundown values tuning parameters for eviction? - // See above - std::atomic meta{}; + // See above. Mutable for read reference counting. + mutable std::atomic meta{}; // Whether this is a "deteched" handle that is independently allocated // with `new` (so must be deleted with `delete`). @@ -397,42 +397,12 @@ class BaseClockTable { eviction_callback_(*eviction_callback), hash_seed_(*hash_seed) {} - // Creates a "standalone" handle for returning from an Insert operation that - // cannot be completed by actually inserting into the table. - // Updates `standalone_usage_` but not `usage_` nor `occupancy_`. - template - HandleImpl* StandaloneInsert(const ClockHandleBasicData& proto); - template typename Table::HandleImpl* CreateStandalone(ClockHandleBasicData& proto, size_t capacity, bool strict_capacity_limit, bool allow_uncharged); - // Helper for updating `usage_` for new entry with given `total_charge` - // and evicting if needed under strict_capacity_limit=true rules. This - // means the operation might fail with Status::MemoryLimit. If - // `need_evict_for_occupancy`, then eviction of at least one entry is - // required, and the operation should fail if not possible. - // NOTE: Otherwise, occupancy_ is not managed in this function - template - Status ChargeUsageMaybeEvictStrict(size_t total_charge, size_t capacity, - bool need_evict_for_occupancy, - typename Table::InsertState& state); - - // Helper for updating `usage_` for new entry with given `total_charge` - // and evicting if needed under strict_capacity_limit=false rules. This - // means that updating `usage_` always succeeds even if forced to exceed - // capacity. If `need_evict_for_occupancy`, then eviction of at least one - // entry is required, and the operation should return false if such eviction - // is not possible. `usage_` is not updated in that case. Otherwise, returns - // true, indicating success. - // NOTE: occupancy_ is not managed in this function - template - bool ChargeUsageMaybeEvictNonStrict(size_t total_charge, size_t capacity, - bool need_evict_for_occupancy, - typename Table::InsertState& state); - template Status Insert(const ClockHandleBasicData& proto, typename Table::HandleImpl** handle, Cache::Priority priority, @@ -459,7 +429,38 @@ class BaseClockTable { void TEST_ReleaseNMinus1(ClockHandle* handle, size_t n); #endif - protected: + private: // fns + // Creates a "standalone" handle for returning from an Insert operation that + // cannot be completed by actually inserting into the table. + // Updates `standalone_usage_` but not `usage_` nor `occupancy_`. + template + HandleImpl* StandaloneInsert(const ClockHandleBasicData& proto); + + // Helper for updating `usage_` for new entry with given `total_charge` + // and evicting if needed under strict_capacity_limit=true rules. This + // means the operation might fail with Status::MemoryLimit. If + // `need_evict_for_occupancy`, then eviction of at least one entry is + // required, and the operation should fail if not possible. + // NOTE: Otherwise, occupancy_ is not managed in this function + template + Status ChargeUsageMaybeEvictStrict(size_t total_charge, size_t capacity, + bool need_evict_for_occupancy, + typename Table::InsertState& state); + + // Helper for updating `usage_` for new entry with given `total_charge` + // and evicting if needed under strict_capacity_limit=false rules. This + // means that updating `usage_` always succeeds even if forced to exceed + // capacity. If `need_evict_for_occupancy`, then eviction of at least one + // entry is required, and the operation should return false if such eviction + // is not possible. `usage_` is not updated in that case. Otherwise, returns + // true, indicating success. + // NOTE: occupancy_ is not managed in this function + template + bool ChargeUsageMaybeEvictNonStrict(size_t total_charge, size_t capacity, + bool need_evict_for_occupancy, + typename Table::InsertState& state); + + protected: // data // We partition the following members into different cache lines // to avoid false sharing among Lookup, Release, Erase and Insert // operations in ClockCacheShard. @@ -536,18 +537,14 @@ class HyperClockTable : public BaseClockTable { void Erase(const UniqueId64x2& hashed_key); - void ConstApplyToEntriesRange(std::function func, - size_t index_begin, size_t index_end, - bool apply_if_will_be_deleted) const; - void EraseUnRefEntries(); size_t GetTableSize() const { return size_t{1} << length_bits_; } - int GetLengthBits() const { return length_bits_; } - size_t GetOccupancyLimit() const { return occupancy_limit_; } + const HandleImpl* HandlePtr(size_t idx) const { return &array_[idx]; } + #ifndef NDEBUG size_t& TEST_MutableOccupancyLimit() const { return const_cast(occupancy_limit_);