Skip to content

Commit

Permalink
Add more doc, tests for PageCacheSlot.
Browse files Browse the repository at this point in the history
  • Loading branch information
@tonyastolfi
tonyastolfi committed Feb 26, 2024
1 parent 8ed6476 commit 650249a
Show file tree
Hide file tree
Showing 9 changed files with 623 additions and 42 deletions.
4 changes: 2 additions & 2 deletions doc/proposals/lock_free_cache.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<K, V>` and `CacheSlot<K, V>`, replacing these with concrete classes that explicitly name `llfs::PageId` as the key type, and `boost::intrusive_ptr<batt::Latch<std::shared_ptr<const llfs::PageView>>>` as the value type.
1. Remove the generality of `Cache<K, V>` and `CacheSlot<K, V>`, replacing these with concrete classes that explicitly name `llfs::PageId` as the key type, and `batt::Latch<std::shared_ptr<const llfs::PageView>>` as the value type.
2. Replace `CacheSlot<K, V>` with `llfs::PageCacheSlot`, as described below
3. Replace `Cache<K, V>` with two types that separate the concerns currently both handled inside `Cache<K, V>`:
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<Latch<std::shared_ptr<const PageView>>>`.
15 changes: 9 additions & 6 deletions src/llfs/page_cache.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
Expand Down Expand Up @@ -643,12 +645,13 @@ void PageCache::async_load_page_into_slot(const PageCacheSlot::PinnedRef& pinned
std::shared_ptr<const PageBuffer>& 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;
{
Expand Down
50 changes: 46 additions & 4 deletions src/llfs/page_cache.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<PageCacheSlot::Pool>&& slot_pool) noexcept
Expand All @@ -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;
};

Expand Down Expand Up @@ -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<PinnedPage> put_view(std::shared_ptr<const PageView>&& 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;
Expand Down Expand Up @@ -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<PageCacheSlot::PinnedRef> find_page_in_cache(
PageId page_id, const Optional<PageLayoutId>& 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<PageLayoutId>& required_layout,
OkIfNotFound ok_if_not_found);
Expand Down
83 changes: 66 additions & 17 deletions src/llfs/page_cache_slot.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -40,6 +40,7 @@ namespace llfs {
class PageCacheSlot
{
public:
//==#==========+==+=+=++=+++++++++++-+-+--+----- --- -- - - - -
// State Transition Diagram:
//
// ┌─────────┐
Expand All @@ -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 <llfs/page_cache_slot_pool.hpp>
class AtomicRef; // defined in <llfs/page_cache_slot_atomic_ref.hpp>
class PinnedRef; // defined in <llfs/page_cache_slot_pinned_ref.hpp>

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;
Expand All @@ -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;

//+++++++++++-+-+--+----- --- -- - - - -
Expand All @@ -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<std::shared_ptr<const PageView>>* value() noexcept;

Expand All @@ -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;

Expand Down Expand Up @@ -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();

//+++++++++++-+-+--+----- --- -- - - - -
Expand All @@ -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) {
Expand All @@ -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) {
Expand All @@ -284,6 +334,5 @@ inline void decrement_weak_ref(PageCacheSlot* slot)
//
#include <llfs/page_cache_slot_atomic_ref.hpp>
#include <llfs/page_cache_slot_pool.hpp>
//#include <llfs/page_cache_slot_ref.hpp>

#endif // LLFS_PAGE_CACHE_SLOT_HPP
Loading

0 comments on commit 650249a

Please sign in to comment.