From 650249a585fa6f4361d84f7ed50c66418e9793b3 Mon Sep 17 00:00:00 2001 From: Anthony Astolfi Date: Mon, 26 Feb 2024 12:05:25 -0500 Subject: [PATCH] Add more doc, tests for PageCacheSlot. --- doc/proposals/lock_free_cache.md | 4 +- src/llfs/page_cache.cpp | 15 +- src/llfs/page_cache.hpp | 50 +++- src/llfs/page_cache_slot.hpp | 83 ++++-- src/llfs/page_cache_slot.test.cpp | 427 ++++++++++++++++++++++++++++++ src/llfs/page_id_slot.cpp | 37 ++- src/llfs/page_id_slot.hpp | 33 ++- src/llfs/page_loader.cpp | 11 + src/llfs/page_loader.hpp | 5 + 9 files changed, 623 insertions(+), 42 deletions(-) create mode 100644 src/llfs/page_cache_slot.test.cpp diff --git a/doc/proposals/lock_free_cache.md b/doc/proposals/lock_free_cache.md index aa6b3c2..2809652 100644 --- a/doc/proposals/lock_free_cache.md +++ b/doc/proposals/lock_free_cache.md @@ -141,9 +141,9 @@ This state machine mechanism essentially implements a non-blocking reader/writer We propose the following changes to the existing design: -1. Remove the generality of `Cache` and `CacheSlot`, replacing these with concrete classes that explicitly name `llfs::PageId` as the key type, and `boost::intrusive_ptr>>` as the value type. +1. Remove the generality of `Cache` and `CacheSlot`, replacing these with concrete classes that explicitly name `llfs::PageId` as the key type, and `batt::Latch>` as the value type. 2. Replace `CacheSlot` with `llfs::PageCacheSlot`, as described below 3. Replace `Cache` with two types that separate the concerns currently both handled inside `Cache`: 1. `llfs::PageDeviceCache` implements a per-`PageDevice` physical-page-index to cache slot index (using an array of atomic `u64` values) 2. `llfs::PageCacheSlot::Pool` implements a shared pool of cache slots; one pool can be shared among many `PageDeviceCache` objects - +4. Simplify the design of the `PageCacheSlot` state update mechanism; we don't really need two counters for increase and decrease of pin count, and we can also avoid heap-allocating the `Latch` object in favor of using `Optional>>`. diff --git a/src/llfs/page_cache.cpp b/src/llfs/page_cache.cpp index 092a034..75ed8db 100644 --- a/src/llfs/page_cache.cpp +++ b/src/llfs/page_cache.cpp @@ -536,7 +536,9 @@ void PageCache::purge(PageId page_id, u64 callers, u64 job_id) PageCache::PageDeviceEntry* PageCache::get_device_for_page(PageId page_id) { const page_device_id_int device_id = PageIdFactory::get_device_id(page_id); - BATT_CHECK_LT(device_id, this->page_devices_.size()); + if (BATT_HINT_FALSE(device_id >= this->page_devices_.size())) { + return nullptr; + } return this->page_devices_[device_id].get(); } @@ -643,12 +645,13 @@ void PageCache::async_load_page_into_slot(const PageCacheSlot::PinnedRef& pinned std::shared_ptr& page_data = *result; p_metrics->total_bytes_read.add(page_data->size()); - const PageLayoutId layout_id = [&] { - if (required_layout) { - return *required_layout; + PageLayoutId layout_id = get_page_header(*page_data).layout_id; + if (required_layout) { + if (*required_layout != layout_id) { + latch->set_value(::llfs::make_status(StatusCode::kPageHeaderBadLayoutId)); + return; } - return get_page_header(*page_data).layout_id; - }(); + } PageReader reader_for_layout; { diff --git a/src/llfs/page_cache.hpp b/src/llfs/page_cache.hpp index 502d25b..05e6d9f 100644 --- a/src/llfs/page_cache.hpp +++ b/src/llfs/page_cache.hpp @@ -103,6 +103,8 @@ class PageCache : public PageLoader int line; }; + /** \brief All the per-PageDevice state for a single device in the storage pool. + */ struct PageDeviceEntry { explicit PageDeviceEntry(PageArena&& arena, boost::intrusive_ptr&& slot_pool) noexcept @@ -111,7 +113,13 @@ class PageCache : public PageLoader { } + /** \brief The PageDevice and PageAllocator. + */ PageArena arena; + + /** \brief A per-device page cache; shares a PageCacheSlot::Pool with all other PageDeviceEntry + * objects that have the same page size. + */ PageDeviceCache cache; }; @@ -214,12 +222,14 @@ class PageCache : public PageLoader // //==#==========+==+=+=++=+++++++++++-+-+--+----- --- -- - - - - - // Insert a newly built PageView into the cache. - // + //----- --- -- - - - - + /** \brief Inserts a newly built PageView into the cache. + */ StatusOr put_view(std::shared_ptr&& view, u64 callers, u64 job_id); - // Remove all cached data for the specified page. - // + //----- --- -- - - - - + /** \brief Removes all cached data for the specified page. + */ void purge(PageId id_val, u64 callers, u64 job_id); bool page_might_contain_key(PageId id, const KeyView& key) const; @@ -258,11 +268,43 @@ class PageCache : public PageLoader //+++++++++++-+-+--+----- --- -- - - - - + //----- --- -- - - - - + /** \brief Returns the PageDeviceEntry for the device that owns the given page. + * + * If the specified device (in the most-significant bits of `page_id`) isn't known by this + * PageCache, returns nullptr. + */ PageDeviceEntry* get_device_for_page(PageId page_id); + //----- --- -- - - - - + /** \brief Attempts to find the specified page (`page_id`) in the cache; if successful, the cache + * slot is pinned (so it can't be evicted) and a pinned reference is returned. Otherwise, we + * attempt to load the page. + * + * If the given page is not in-cache and a cache slot can't be evicted/allocated (because there + * are too many pinned pages), then this function returns llfs::StatusCode::kCacheSlotsFull. + * + * \param page_id The page to load + * + * \param required_layout If specified, then the layout of the page is checked and if it doesn't + * match the given identifier, llfs::StatusCode::kPageHeaderBadLayoutId is returned. + * + * \param ok_if_not_found Controls whether page-not-found log messages (WARNING) are emitted if + * the page isn't found; ok_if_not_found == false -> emit log warnings, ... == true -> don't + */ batt::StatusOr find_page_in_cache( PageId page_id, const Optional& required_layout, OkIfNotFound ok_if_not_found); + //----- --- -- - - - - + /** \brief Populates the passed PageCacheSlot asynchronously by attempting to read the page from + * storage and setting the Latch value of the slot. + * + * \param required_layout If specified, then the layout of the page is checked and if it doesn't + * match the given identifier, the Latch is set to llfs::StatusCode::kPageHeaderBadLayoutId. + * + * \param ok_if_not_found Controls whether page-not-found log messages (WARNING) are emitted if + * the page isn't found; ok_if_not_found == false -> emit log warnings, ... == true -> don't + */ void async_load_page_into_slot(const PageCacheSlot::PinnedRef& pinned_slot, const Optional& required_layout, OkIfNotFound ok_if_not_found); diff --git a/src/llfs/page_cache_slot.hpp b/src/llfs/page_cache_slot.hpp index a5c4f27..adae5d8 100644 --- a/src/llfs/page_cache_slot.hpp +++ b/src/llfs/page_cache_slot.hpp @@ -28,7 +28,7 @@ namespace llfs { -/** \brief A container for a single key/value pair in a Cache. +/** \brief A container for a single key/value pair in a PageDeviceCache. * * PageCacheSlot objects are always in one of four states: * - Invalid (initial) @@ -40,6 +40,7 @@ namespace llfs { class PageCacheSlot { public: + //==#==========+==+=+=++=+++++++++++-+-+--+----- --- -- - - - - // State Transition Diagram: // // ┌─────────┐ @@ -60,35 +61,68 @@ class PageCacheSlot // │ ┌─────────────────────────┐ // └─▶│ Valid + Filled + Pinned │ // └─────────────────────────┘ - // + //==#==========+==+=+=++=+++++++++++-+-+--+----- --- -- - - - - + //==#==========+==+=+=++=+++++++++++-+-+--+----- --- -- - - - - + // State integer bit layout: + // + // ┌─────────────────┬────────────────────────────────────────────────────┬─┐ + // │Overflow (8 bits)│ Pin Count (47 bits) │ │ + // └─────────────────┴────────────────────────────────────────────────────┴─┘ + // ▲ + // Valid? (1 bit)───────┘ + //==#==========+==+=+=++=+++++++++++-+-+--+----- --- -- - - - - + + /** \brief The byte offset of the Pin Count within the state integer. + */ static constexpr usize kPinCountShift = 1; + + /** \brief The number of unused (most-significant) bits; used to detect integer overflow. + */ static constexpr usize kOverflowBits = 8; + /** \brief The amount to add/subtract to the state integer to increment or decrement the pin + * count. + */ static constexpr u64 kPinCountDelta = u64{1} << kPinCountShift; + + /** \brief Used to detect pin count integer overflow; should always be zero. + */ static constexpr u64 kOverflowMask = ((u64{1} << kOverflowBits) - 1) << (64 - kOverflowBits); + + /** \brief The Valid bit. + */ static constexpr u64 kValidMask = 1; //+++++++++++-+-+--+----- --- -- - - - - - class Pool; - class AtomicRef; - class PinnedRef; + // Forward-declarations of member types. + // + class Pool; // defined in + class AtomicRef; // defined in + class PinnedRef; // defined in using Self = PageCacheSlot; //+++++++++++-+-+--+----- --- -- - - - - + /** \brief Returns the pin count bit field within the passed state integer. + */ static constexpr u32 get_pin_count(u64 state) { return state >> kPinCountShift; } + /** \brief Returns true iff the pin count of `state` is non-zero, indicating the slot is in-use + * and must not be evicted or modified. + */ static constexpr bool is_pinned(u64 state) { return Self::get_pin_count(state) != 0; } + /** \brief Returns true iff the Valid? bit of `state` is set. + */ static constexpr bool is_valid(u64 state) { return (state & kValidMask) != 0; @@ -101,8 +135,13 @@ class PageCacheSlot //+++++++++++-+-+--+----- --- -- - - - - + /** \brief Constructs a new PageCacheSlot owned by the passed Pool. + */ explicit PageCacheSlot(Pool& pool) noexcept; + /** \brief Destroys the cache slot; ref count and pin count MUST both be zero when the slot is + * destroyed, or we will panic. + */ ~PageCacheSlot() noexcept; //+++++++++++-+-+--+----- --- -- - - - - @@ -115,13 +154,18 @@ class PageCacheSlot */ usize index() const noexcept; - /** \brief Returns the current key held in the slot, if valid; if the slot is invalid, returned - * value is undefined. + /** \brief Returns the current key held in the slot, if valid; if the slot is invalid, the + * returned value is undefined. */ PageId key() const; /** Returns the current value held in the slot, if valid; if the slot is invalid, behavior is * undefined. + * + * Most callers of this function must not modify the returned Latch object. There is one + * exception to this rule: the caller who most recently called `fill` to transition the slot state + * from 'Invalid' to 'Valid + Filled' is required to set the Latch value, which broadcasts to all + * other observers of this slot that the page has been loaded. */ batt::Latch>* value() noexcept; @@ -133,7 +177,9 @@ class PageCacheSlot /** \brief Returns the current (weak/non-pinning) reference count. * - * Do not confuse this with the pin count! + * Do not confuse this with the pin count! A non-zero ref count keeps the PageCacheSlot (and by + * extension the pool that owns it) in scope, but it does not prevent the slot from being evicted + * and refilled. Think of this as a weak reference count. */ u64 ref_count() const noexcept; @@ -231,20 +277,20 @@ class PageCacheSlot //+++++++++++-+-+--+----- --- -- - - - - private: - // The implementation of acquire_pin; returns true iff successful. - // + /** \brief The implementation of acquire_pin; returns true iff successful. + */ bool acquire_pin_impl(PageId key) noexcept; - // Invoked when the ref count goes from 0 -> 1. - // + /** \brief Invoked when the ref count goes from 0 -> 1. + */ void notify_first_ref_acquired(); - // Invoked when the ref count goes from 1 -> 0. - // + /** \brief Invoked when the ref count goes from 1 -> 0. + */ void notify_last_ref_released(); - // Sets the valid bit; Panic if the previous state was not Invalid. - // + /** \brief Sets the valid bit; Panic if the previous state was not Invalid. + */ void set_valid(); //+++++++++++-+-+--+----- --- -- - - - - @@ -261,6 +307,8 @@ class PageCacheSlot namespace detail { +/** \brief Calls slot->add_ref() iff slot is not nullptr. + */ inline PageCacheSlot* increment_weak_ref(PageCacheSlot* slot) { if (slot) { @@ -269,6 +317,8 @@ inline PageCacheSlot* increment_weak_ref(PageCacheSlot* slot) return slot; } +/** \brief Calls slot->remove_ref() iff slot is not nullptr. + */ inline void decrement_weak_ref(PageCacheSlot* slot) { if (slot) { @@ -284,6 +334,5 @@ inline void decrement_weak_ref(PageCacheSlot* slot) // #include #include -//#include #endif // LLFS_PAGE_CACHE_SLOT_HPP diff --git a/src/llfs/page_cache_slot.test.cpp b/src/llfs/page_cache_slot.test.cpp new file mode 100644 index 0000000..0660603 --- /dev/null +++ b/src/llfs/page_cache_slot.test.cpp @@ -0,0 +1,427 @@ +//#=##=##=#==#=#==#===#+==#+==========+==+=+=+=+=+=++=+++=+++++=-++++=-+++++++++++ +// +// Part of the LLFS Project, under Apache License v2.0. +// See https://www.apache.org/licenses/LICENSE-2.0 for license information. +// SPDX short identifier: Apache-2.0 +// +//+++++++++++-+-+--+----- --- -- - - - - + +#include +// +#include + +#include +#include + +#include + +namespace { + +// Test Plan: +// +// 1. Create slots with different index values in a Pool; verify index() +// - also verify initial is_valid state +// 2. ref_count test - add_ref/remove_ref should affect the ref_count, and also the use count of +// the pool but only in the case of add: 0 -> 1 and remove: 1 -> 0. +// 3. State transition test +// a. Invalid --(clear)--> Valid + Cleared +// b. Invalid --(fill)--> Valid + Filled +// c. Valid + Cleared --(evict)--> Invalid +// d. Valid + Filled --(evict)--> Invalid +// e. Valid + Filled --(acquire_pin)--> Valid + Filled + Pinned +// f. Valid + Filled + Pinned --(acquire_pin)-- >Valid + Filled + Pinned +// g. Valid + Filled + Pinned --(release_pin)-- >Valid + Filled + Pinned +// h. Valid + Filled + Pinned --(release_pin)-- >Valid + Filled +// 4. extend_pin increases pin count +// a. success if already > 0 +// b. panic otherwise +// 5. evict fails if pin count != 0 +// 6. evict_if_key_equals +// a. success +// b. fail because pin count != 0 +// c. fail because key is wrong +// 7. fill fails when state is not Invalid: +// a. Valid + Filled +// b. Valid + Cleared +// c. Valid + Filled + Pinned +// 8. update_latest_use +// 9. set_obsolete_hint +// + +using namespace llfs::int_types; + +constexpr usize kNumTestSlots = 4; +const std::string kTestPoolName = "Test PageCacheSlot Pool"; + +class PageCacheSlotTest : public ::testing::Test +{ + public: + boost::intrusive_ptr pool_ = llfs::PageCacheSlot::Pool::make_new( + /*n_slots=*/kNumTestSlots, batt::make_copy(kTestPoolName)); +}; + +//==#==========+==+=+=++=+++++++++++-+-+--+----- --- -- - - - - +// 1. Create slots with different index values in a Pool; verify index() +// - also verify initial is_valid state +// +TEST_F(PageCacheSlotTest, CreateSlots) +{ + for (usize i = 0; i < kNumTestSlots; ++i) { + llfs::PageCacheSlot* slot = this->pool_->allocate(); + + ASSERT_NE(slot, nullptr); + EXPECT_EQ(slot, this->pool_->get_slot(i)); + EXPECT_EQ(slot->index(), i); + EXPECT_EQ(this->pool_->index_of(slot), i); + EXPECT_FALSE(slot->is_valid()); + + if (i == 0) { + EXPECT_DEATH(slot->value(), ".*Assert.*failed:.*this->value_.*==.*true.*"); + } + + EXPECT_FALSE(slot->key().is_valid()); + EXPECT_EQ(slot->ref_count(), 0u); + EXPECT_EQ(slot->pin_count(), 0u); + } +} + +//==#==========+==+=+=++=+++++++++++-+-+--+----- --- -- - - - - +// 2. ref_count test - add_ref/remove_ref should affect the ref_count, and also the use count of +// the pool but only in the case of add: 0 -> 1 and remove: 1 -> 0. +// +TEST_F(PageCacheSlotTest, AddRemoveRef) +{ + EXPECT_EQ(this->pool_->use_count(), 1u); + + llfs::PageCacheSlot* slot = this->pool_->allocate(); + + EXPECT_EQ(slot->ref_count(), 0u); + EXPECT_DEATH(slot->remove_ref(), "Assert.*failed:.*observed_count.*>.*0"); + + slot->add_ref(); + + EXPECT_EQ(slot->ref_count(), 1u); + EXPECT_EQ(this->pool_->use_count(), 2u); + + slot->add_ref(); + slot->add_ref(); + + EXPECT_EQ(slot->ref_count(), 3u); + EXPECT_EQ(this->pool_->use_count(), 2u); + + slot->remove_ref(); + + EXPECT_EQ(slot->ref_count(), 2u); + EXPECT_EQ(this->pool_->use_count(), 2u); + + slot->remove_ref(); + slot->remove_ref(); + + EXPECT_EQ(slot->ref_count(), 0u); + EXPECT_EQ(this->pool_->use_count(), 1u); +} + +//==#==========+==+=+=++=+++++++++++-+-+--+----- --- -- - - - - +// 3. State transition test +// a. Invalid --(clear)--> Valid + Cleared +// b. Invalid --(fill)--> Valid + Filled +// c. Valid + Cleared --(evict)--> Invalid +// d. Valid + Filled --(evict)--> Invalid +// e. Valid + Filled --(acquire_pin)--> Valid + Filled + Pinned +// f. Valid + Filled + Pinned --(acquire_pin)-- >Valid + Filled + Pinned +// g. Valid + Filled + Pinned --(release_pin)-- >Valid + Filled + Pinned +// h. Valid + Filled + Pinned --(release_pin)-- >Valid + Filled +// +TEST_F(PageCacheSlotTest, StateTransitions) +{ + llfs::PageCacheSlot* slot = this->pool_->allocate(); + + EXPECT_FALSE(slot->is_valid()); + + // a. Invalid --(clear)--> Valid + Cleared + // + slot->clear(); + EXPECT_TRUE(slot->is_valid()); + + // c. Valid + Cleared --(evict)--> Invalid + // + EXPECT_TRUE(slot->evict()); + EXPECT_FALSE(slot->is_valid()); + + // b. Invalid --(fill)--> Valid + Filled + // + { + llfs::PageCacheSlot::PinnedRef pinned_ref = slot->fill(llfs::PageId{1}); + + EXPECT_TRUE(slot->is_valid()); + EXPECT_EQ(slot->key(), llfs::PageId{1}); + EXPECT_NE(slot->value(), nullptr); + EXPECT_TRUE(pinned_ref); + EXPECT_EQ(pinned_ref.slot(), slot); + EXPECT_EQ(pinned_ref.key(), slot->key()); + EXPECT_EQ(pinned_ref.value(), slot->value()); + EXPECT_EQ(pinned_ref.get(), slot->value()); + EXPECT_EQ(pinned_ref.pin_count(), 1u); + EXPECT_EQ(slot->pin_count(), 1u); + EXPECT_EQ(pinned_ref.ref_count(), 1u); + EXPECT_EQ(slot->ref_count(), 1u); + + // f. Valid + Filled + Pinned --(acquire_pin)-- >Valid + Filled + Pinned + // + llfs::PageCacheSlot::PinnedRef ref2 = pinned_ref; + + EXPECT_TRUE(ref2); + EXPECT_EQ(ref2.slot(), slot); + EXPECT_EQ(pinned_ref.pin_count(), 2u); + EXPECT_EQ(slot->pin_count(), 2u); + EXPECT_EQ(pinned_ref.ref_count(), 1u); + EXPECT_EQ(slot->ref_count(), 1u); + + llfs::PageCacheSlot::PinnedRef ref3; + + EXPECT_FALSE(ref3); + + ref3 = ref2; + + EXPECT_TRUE(ref3); + EXPECT_EQ(ref3.slot(), slot); + EXPECT_EQ(pinned_ref.pin_count(), 3u); + EXPECT_EQ(slot->pin_count(), 3u); + EXPECT_EQ(pinned_ref.ref_count(), 1u); + EXPECT_EQ(slot->ref_count(), 1u); + + { + llfs::PageCacheSlot::PinnedRef ref4 = std::move(ref2); + + EXPECT_FALSE(ref2); + EXPECT_EQ(ref4.slot(), slot); + EXPECT_TRUE(ref4); + EXPECT_EQ(pinned_ref.pin_count(), 3u); + EXPECT_EQ(slot->pin_count(), 3u); + EXPECT_EQ(pinned_ref.ref_count(), 1u); + EXPECT_EQ(slot->ref_count(), 1u); + + { + llfs::PageCacheSlot::PinnedRef ref5; + + EXPECT_FALSE(ref5); + + ref5 = std::move(ref3); + + EXPECT_EQ(ref5.slot(), slot); + EXPECT_FALSE(ref3); + EXPECT_TRUE(ref5); + EXPECT_EQ(pinned_ref.pin_count(), 3u); + EXPECT_EQ(slot->pin_count(), 3u); + EXPECT_EQ(pinned_ref.ref_count(), 1u); + EXPECT_EQ(slot->ref_count(), 1u); + } + // + // g. Valid + Filled + Pinned --(release_pin)-- >Valid + Filled + Pinned + + EXPECT_EQ(pinned_ref.pin_count(), 2u); + EXPECT_EQ(slot->pin_count(), 2u); + EXPECT_EQ(pinned_ref.ref_count(), 1u); + EXPECT_EQ(slot->ref_count(), 1u); + } + + EXPECT_EQ(pinned_ref.pin_count(), 1u); + EXPECT_EQ(slot->pin_count(), 1u); + EXPECT_EQ(pinned_ref.ref_count(), 1u); + EXPECT_EQ(slot->ref_count(), 1u); + } + // + // h. Valid + Filled + Pinned --(release_pin)-- >Valid + Filled + + //----- --- -- - - - - + + // e. Valid + Filled --(acquire_pin)--> Valid + Filled + Pinned + // + EXPECT_EQ(slot->pin_count(), 0u); + EXPECT_EQ(slot->ref_count(), 0u); + EXPECT_TRUE(slot->is_valid()); + { + llfs::PageCacheSlot::PinnedRef pinned_ref = + slot->acquire_pin(llfs::PageId{}, /*ignore_key=*/true); + + EXPECT_TRUE(pinned_ref); + EXPECT_TRUE(slot->is_valid()); + EXPECT_EQ(slot->pin_count(), 1u); + } + EXPECT_EQ(slot->pin_count(), 0u); + EXPECT_EQ(slot->ref_count(), 0u); + EXPECT_TRUE(slot->is_valid()); + { + llfs::PageCacheSlot::PinnedRef pinned_ref = + slot->acquire_pin(llfs::PageId{1}, /*ignore_key=*/false); + + EXPECT_TRUE(pinned_ref); + EXPECT_TRUE(slot->is_valid()); + EXPECT_EQ(slot->pin_count(), 1u); + } + EXPECT_EQ(slot->pin_count(), 0u); + EXPECT_EQ(slot->ref_count(), 0u); + EXPECT_TRUE(slot->is_valid()); + { + // Try to acquire pin using the wrong PageId; expect to fail. + // + llfs::PageCacheSlot::PinnedRef pinned_ref = + slot->acquire_pin(llfs::PageId{2}, /*ignore_key=*/false); + + EXPECT_FALSE(pinned_ref); + EXPECT_TRUE(slot->is_valid()); + EXPECT_EQ(slot->pin_count(), 0u); + } + + // b. Invalid --(fill)--> Valid + Filled + // + EXPECT_TRUE(slot->evict()); + EXPECT_FALSE(slot->is_valid()); +} + +//==#==========+==+=+=++=+++++++++++-+-+--+----- --- -- - - - - +// 4. extend_pin increases pin count +// a. success if already > 0 +// +TEST_F(PageCacheSlotTest, ExtendPinSuccess) +{ + llfs::PageCacheSlot* slot = this->pool_->allocate(); + + llfs::PageCacheSlot::PinnedRef pinned_ref = slot->fill(llfs::PageId{1}); + + EXPECT_EQ(slot->pin_count(), 1u); + + slot->extend_pin(); + + EXPECT_EQ(slot->pin_count(), 2u); + + slot->release_pin(); + + EXPECT_EQ(slot->pin_count(), 1u); +} + +//==#==========+==+=+=++=+++++++++++-+-+--+----- --- -- - - - - +// 4. extend_pin increases pin count +// b. panic otherwise +// +TEST_F(PageCacheSlotTest, ExtendPinDeath) +{ + llfs::PageCacheSlot* slot = this->pool_->allocate(); + + EXPECT_DEATH(slot->extend_pin(), "Assert.*failed:.*is.*pinned"); +} + +//==#==========+==+=+=++=+++++++++++-+-+--+----- --- -- - - - - +// 5. evict fails if pin count != 0 +// +TEST_F(PageCacheSlotTest, EvictFailure) +{ + llfs::PageCacheSlot* slot = this->pool_->allocate(); + + llfs::PageCacheSlot::PinnedRef pinned_ref = slot->fill(llfs::PageId{1}); + + EXPECT_FALSE(slot->evict()); +} + +//==#==========+==+=+=++=+++++++++++-+-+--+----- --- -- - - - - +// 6. evict_if_key_equals +// a. success +// +TEST_F(PageCacheSlotTest, EvictIfKeyEqualsSuccess) +{ + llfs::PageCacheSlot* slot = this->pool_->allocate(); + { + llfs::PageCacheSlot::PinnedRef pinned_ref = slot->fill(llfs::PageId{1}); + } + + EXPECT_TRUE(slot->evict_if_key_equals(llfs::PageId{1})); + EXPECT_FALSE(slot->is_valid()); +} + +//==#==========+==+=+=++=+++++++++++-+-+--+----- --- -- - - - - +// 6. evict_if_key_equals +// b. fail because pin count != 0 +// +TEST_F(PageCacheSlotTest, EvictIfKeyEqualsFailurePinned) +{ + llfs::PageCacheSlot* slot = this->pool_->allocate(); + { + llfs::PageCacheSlot::PinnedRef pinned_ref = slot->fill(llfs::PageId{1}); + + EXPECT_FALSE(slot->evict_if_key_equals(llfs::PageId{1})); + } + EXPECT_TRUE(slot->is_valid()); +} + +//==#==========+==+=+=++=+++++++++++-+-+--+----- --- -- - - - - +// 6. evict_if_key_equals +// c. fail because key is wrong +// +TEST_F(PageCacheSlotTest, EvictIfKeyEqualsFailureWrongKey) +{ + llfs::PageCacheSlot* slot = this->pool_->allocate(); + { + llfs::PageCacheSlot::PinnedRef pinned_ref = slot->fill(llfs::PageId{1}); + } + + EXPECT_FALSE(slot->evict_if_key_equals(llfs::PageId{2})); + EXPECT_TRUE(slot->is_valid()); +} + +//==#==========+==+=+=++=+++++++++++-+-+--+----- --- -- - - - - +// 7. fill fails when state is not Invalid: +// a. Valid + Filled +// c. Valid + Filled + Pinned +// +TEST_F(PageCacheSlotTest, FillFailureAlreadyFilled) +{ + llfs::PageCacheSlot* slot = this->pool_->allocate(); + { + llfs::PageCacheSlot::PinnedRef pinned_ref = slot->fill(llfs::PageId{1}); + + EXPECT_EQ(slot->pin_count(), 1u); + EXPECT_TRUE(pinned_ref); + EXPECT_DEATH(slot->fill(llfs::PageId{2}), "Assert.*fail.*is.*valid"); + } + EXPECT_EQ(slot->pin_count(), 0u); + EXPECT_DEATH(slot->fill(llfs::PageId{2}), "Assert.*fail.*is.*valid"); +} + +//==#==========+==+=+=++=+++++++++++-+-+--+----- --- -- - - - - +// 7. fill fails when state is not Invalid: +// b. Valid + Cleared +// +TEST_F(PageCacheSlotTest, FillFailureCleared) +{ + llfs::PageCacheSlot* slot = this->pool_->allocate(); + + EXPECT_FALSE(slot->is_valid()); + + slot->clear(); + + EXPECT_TRUE(slot->is_valid()); + EXPECT_DEATH(slot->fill(llfs::PageId{2}), "Assert.*fail.*is.*valid"); + EXPECT_DEATH(slot->clear(), "Assert.*fail.*is.*valid"); +} + +//==#==========+==+=+=++=+++++++++++-+-+--+----- --- -- - - - - +// 8. update_latest_use +// 9. set_obsolete_hint +// +TEST_F(PageCacheSlotTest, LatestUse) +{ + llfs::PageCacheSlot* slot = this->pool_->allocate(); + + i64 t0 = slot->get_latest_use(); + slot->update_latest_use(); + i64 t1 = slot->get_latest_use(); + + EXPECT_GT(t1 - t0, 0); + + slot->set_obsolete_hint(); + i64 t2 = slot->get_latest_use(); + + EXPECT_LT(t2 - t1, 0); +} + +} // namespace diff --git a/src/llfs/page_id_slot.cpp b/src/llfs/page_id_slot.cpp index e5f22b2..4f8be0c 100644 --- a/src/llfs/page_id_slot.cpp +++ b/src/llfs/page_id_slot.cpp @@ -27,21 +27,21 @@ namespace llfs { //==#==========+==+=+=++=+++++++++++-+-+--+----- --- -- - - - - // -batt::StatusOr PageIdSlot::load_through(PageLoader& loader, - const Optional& required_layout, - PinPageToJob pin_page_to_job, - OkIfNotFound ok_if_not_found) const noexcept +/*static*/ batt::StatusOr PageIdSlot::load_through_impl( + PageCacheSlot::AtomicRef& cache_slot_ref, PageLoader& loader, + const Optional& required_layout, PinPageToJob pin_page_to_job, + OkIfNotFound ok_if_not_found, PageId page_id) noexcept { { - batt::StatusOr pinned = this->try_pin(); + batt::StatusOr pinned = Self::try_pin_impl(cache_slot_ref, page_id); if (pinned.ok()) { return pinned; } } batt::StatusOr pinned = loader.get_page_with_layout_in_job( - this->page_id, required_layout, pin_page_to_job, ok_if_not_found); + page_id, required_layout, pin_page_to_job, ok_if_not_found); if (pinned.ok()) { - this->cache_slot_ref = pinned->get_cache_slot(); + cache_slot_ref = pinned->get_cache_slot(); } return pinned; @@ -49,11 +49,12 @@ batt::StatusOr PageIdSlot::load_through(PageLoader& loader, //==#==========+==+=+=++=+++++++++++-+-+--+----- --- -- - - - - // -batt::StatusOr PageIdSlot::try_pin() const noexcept +/*static*/ batt::StatusOr PageIdSlot::try_pin_impl( + PageCacheSlot::AtomicRef& cache_slot_ref, PageId page_id) noexcept { PageIdSlot::metrics().load_total_count.fetch_add(1); - PageCacheSlot::PinnedRef cache_slot = this->cache_slot_ref.pin(this->page_id); + PageCacheSlot::PinnedRef cache_slot = cache_slot_ref.pin(page_id); if (!cache_slot) { PageIdSlot::metrics().load_slot_miss_count.fetch_add(1); return make_status(StatusCode::kPinFailedPageEvicted); @@ -67,4 +68,22 @@ batt::StatusOr PageIdSlot::try_pin() const noexcept return PinnedPage{page_view->get(), std::move(cache_slot)}; } +//==#==========+==+=+=++=+++++++++++-+-+--+----- --- -- - - - - +// +batt::StatusOr PageIdSlot::load_through(PageLoader& loader, + const Optional& required_layout, + PinPageToJob pin_page_to_job, + OkIfNotFound ok_if_not_found) const noexcept +{ + return Self::load_through_impl(this->cache_slot_ref, loader, required_layout, pin_page_to_job, + ok_if_not_found, this->page_id); +} + +//==#==========+==+=+=++=+++++++++++-+-+--+----- --- -- - - - - +// +batt::StatusOr PageIdSlot::try_pin() const noexcept +{ + return Self::try_pin_impl(this->cache_slot_ref, this->page_id); +} + } // namespace llfs diff --git a/src/llfs/page_id_slot.hpp b/src/llfs/page_id_slot.hpp index 1c36f7c..a6e7cd8 100644 --- a/src/llfs/page_id_slot.hpp +++ b/src/llfs/page_id_slot.hpp @@ -44,14 +44,14 @@ bool bool_from(PinPageToJob pin_page, bool default_value); struct PageIdSlot { using Self = PageIdSlot; - using AtomicCacheSlotRefT = PageCacheSlot::AtomicRef; - struct Metrics { CountMetric load_total_count; CountMetric load_slot_hit_count; CountMetric load_slot_miss_count; }; + //+++++++++++-+-+--+----- --- -- - - - - + static Metrics& metrics() { static Metrics m_; @@ -68,8 +68,33 @@ struct PageIdSlot { static Self from_pinned_page(const PinnedPage& pinned); + /** \brief Attempts to pin the passed cache slot using the specified `page_id`; if this fails, + * then falls back on loading the page from the `loader`, updating `cache_slot_ref` if successful. + */ + static batt::StatusOr load_through_impl(PageCacheSlot::AtomicRef& cache_slot_ref, + PageLoader& loader, + const Optional& required_layout, + PinPageToJob pin_page_to_job, + OkIfNotFound ok_if_not_found, + PageId page_id) noexcept; + + /** \brief Attempts to pin the slot using the specified page_id. + * + * If pin succeeded, but the page failed to load into the slot when it was originally added to the + * cache, then the page load error status code is returned. + * + * \return The PinnedPage if successful, llfs::StatusCode::kPinFailedPageEvicted otherwise (unless + * load error; see above) + */ + static batt::StatusOr try_pin_impl(PageCacheSlot::AtomicRef& cache_slot_ref, + PageId page_id) noexcept; + + //+++++++++++-+-+--+----- --- -- - - - - + PageId page_id; - mutable AtomicCacheSlotRefT cache_slot_ref; + mutable PageCacheSlot::AtomicRef cache_slot_ref; + + //+++++++++++-+-+--+----- --- -- - - - - operator PageId() const { @@ -90,7 +115,7 @@ struct PageIdSlot { { if (BATT_HINT_TRUE(id != this->page_id)) { this->page_id = id; - this->cache_slot_ref = AtomicCacheSlotRefT{}; + this->cache_slot_ref = PageCacheSlot::AtomicRef{}; } return *this; } diff --git a/src/llfs/page_loader.cpp b/src/llfs/page_loader.cpp index 3b5a70c..583853f 100644 --- a/src/llfs/page_loader.cpp +++ b/src/llfs/page_loader.cpp @@ -100,4 +100,15 @@ StatusOr PageLoader::get_page(PageId page_id, OkIfNotFound ok_if_not return this->get_page_with_layout(page_id, /*required_layout=*/None, ok_if_not_found); } +//==#==========+==+=+=++=+++++++++++-+-+--+----- --- -- - - - - +// +StatusOr PageLoader::get_page_slot_ref_with_layout_in_job( + PageId page_id, PageCacheSlot::AtomicRef& slot_ref, + const Optional& required_layout, PinPageToJob pin_page_to_job, + OkIfNotFound ok_if_not_found) +{ + return PageIdSlot::load_through_impl(slot_ref, *this, required_layout, pin_page_to_job, + ok_if_not_found, page_id); +} + } // namespace llfs diff --git a/src/llfs/page_loader.hpp b/src/llfs/page_loader.hpp index e48f5b5..da1272a 100644 --- a/src/llfs/page_loader.hpp +++ b/src/llfs/page_loader.hpp @@ -64,6 +64,11 @@ class PageLoader virtual StatusOr get_page(PageId page_id, OkIfNotFound ok_if_not_found); + virtual StatusOr get_page_slot_ref_with_layout_in_job( + PageId page_id, PageCacheSlot::AtomicRef& slot_ref, + const Optional& required_layout, PinPageToJob pin_page_to_job, + OkIfNotFound ok_if_not_found); + protected: PageLoader() = default; };