Skip to content

Commit 1b79ed7

Browse files
authored
CachingPageTracer Optimization (#172)
* Initial commit, WIP * Started adding tests * Finished tests * Addressed first set of review feedback * Addressed second set of feedback
1 parent 711729a commit 1b79ed7

16 files changed

Lines changed: 880 additions & 98 deletions

src/llfs/api_types.hpp

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,10 @@ BATT_STRONG_TYPEDEF(usize, BufferCount);
8181
*/
8282
BATT_STRONG_TYPEDEF(usize, BufferSize);
8383

84+
/** \brief True if a page contains outgoing references to other pages.
85+
*/
86+
BATT_STRONG_TYPEDEF(bool, HasOutgoingRefs);
87+
8488
} // namespace llfs
8589

8690
#endif // LLFS_API_TYPES_HPP

src/llfs/committable_page_cache_job.cpp

Lines changed: 23 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -88,11 +88,8 @@ usize CommittablePageCacheJob::new_page_count() const noexcept
8888
//
8989
BoxedSeq<PageId> CommittablePageCacheJob::deleted_page_ids() const
9090
{
91-
return as_seq(this->job_->get_deleted_pages().begin(), this->job_->get_deleted_pages().end()) //
92-
| seq::map([](const auto& kv_pair) -> PageId {
93-
return kv_pair.first;
94-
}) //
95-
| seq::boxed();
91+
return BoxedSeq<PageId>{
92+
as_seq(this->job_->get_deleted_pages().begin(), this->job_->get_deleted_pages().end())};
9693
}
9794

9895
//==#==========+==+=+=++=+++++++++++-+-+--+----- --- -- - - - -
@@ -482,7 +479,7 @@ Status CommittablePageCacheJob::await_ref_count_updates(const PageRefCountUpdate
482479

483480
//==#==========+==+=+=++=+++++++++++-+-+--+----- --- -- - - - -
484481
//
485-
auto CommittablePageCacheJob::get_page_ref_count_updates(u64 /*callers*/) const
482+
auto CommittablePageCacheJob::get_page_ref_count_updates(u64 /*callers*/)
486483
-> StatusOr<PageRefCountUpdates>
487484
{
488485
std::unordered_map<PageId, i32, PageId::Hash> ref_count_delta = this->job_->get_root_set_delta();
@@ -512,19 +509,22 @@ auto CommittablePageCacheJob::get_page_ref_count_updates(u64 /*callers*/) const
512509
// Trace deleted pages non-recursively, decrementing the ref counts of all pages they directly
513510
// reference.
514511
//
515-
for (const auto& p : this->job_->get_deleted_pages()) {
516-
// Sanity check; deleted pages should have a ref_count_delta of kRefCount_1_to_0.
512+
LoadingPageTracer loading_tracer{loader, /*ok_if_not_found=*/true};
513+
CachingPageTracer caching_tracer{this->job_->cache().devices_by_id(), loading_tracer};
514+
for (const PageId& deleted_page_id : this->job_->get_deleted_pages()) {
515+
// Decrement ref counts.
517516
//
518-
const PageId deleted_page_id = p.first;
519-
{
520-
auto iter = ref_count_delta.find(deleted_page_id);
521-
BATT_CHECK_NE(iter, ref_count_delta.end());
522-
BATT_CHECK_EQ(iter->second, kRefCount_1_to_0);
517+
batt::StatusOr<batt::BoxedSeq<PageId>> outgoing_refs =
518+
caching_tracer.trace_page_refs(deleted_page_id);
519+
if (outgoing_refs.status() == batt::StatusCode::kNotFound) {
520+
this->not_found_deleted_pages_.insert(deleted_page_id);
521+
continue;
523522
}
523+
BATT_REQUIRE_OK(outgoing_refs);
524524

525-
// Decrement ref counts.
526-
//
527-
p.second->trace_refs() | seq::for_each([&ref_count_delta, deleted_page_id](PageId id) {
525+
ref_count_delta[deleted_page_id] = kRefCount_1_to_0;
526+
527+
*outgoing_refs | seq::for_each([&ref_count_delta, deleted_page_id](PageId id) {
528528
if (id) {
529529
LLFS_VLOG(1) << " decrementing ref count for page " << id
530530
<< " (because it was referenced from deleted page " << deleted_page_id << ")";
@@ -596,13 +596,13 @@ Status CommittablePageCacheJob::drop_deleted_pages(u64 callers)
596596
{
597597
LLFS_VLOG(1) << "commit(PageCacheJob): dropping deleted pages";
598598

599-
const auto& deleted_pages = this->job_->get_deleted_pages();
600-
601-
return parallel_drop_pages(as_seq(deleted_pages.begin(), deleted_pages.end()) //
602-
| seq::map([](const auto& kv_pair) -> PageId {
603-
return kv_pair.first;
604-
}) //
605-
| seq::collect_vec(),
599+
// From the set of all deleted pages, filter out those that were not found during the attempt to
600+
// trace their outgoing refs.
601+
//
602+
return parallel_drop_pages(this->deleted_page_ids() | seq::filter([this](const PageId& id) {
603+
auto iter = this->not_found_deleted_pages_.find(id);
604+
return iter == this->not_found_deleted_pages_.end();
605+
}) | seq::collect_vec(),
606606
this->job_->cache(), this->job_->job_id, callers);
607607
}
608608

src/llfs/committable_page_cache_job.hpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -193,7 +193,7 @@ class CommittablePageCacheJob
193193

194194
Status write_new_pages();
195195

196-
StatusOr<PageRefCountUpdates> get_page_ref_count_updates(u64 callers) const;
196+
StatusOr<PageRefCountUpdates> get_page_ref_count_updates(u64 callers);
197197

198198
StatusOr<DeadPages> start_ref_count_updates(const JobCommitParams& params,
199199
PageRefCountUpdates& updates, u64 callers);
@@ -212,6 +212,7 @@ class CommittablePageCacheJob
212212
boost::intrusive_ptr<FinalizedJobTracker> tracker_;
213213
PageRefCountUpdates ref_count_updates_;
214214
std::unique_ptr<WriteNewPagesContext> write_new_pages_context_;
215+
std::unordered_set<PageId, PageId::Hash> not_found_deleted_pages_;
215216
};
216217

217218
/** \brief Write all changes in `job` to durable storage. This is guaranteed to be atomic.
Lines changed: 99 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,99 @@
1+
//#=##=##=#==#=#==#===#+==#+==========+==+=+=+=+=+=++=+++=+++++=-++++=-+++++++++++
2+
//
3+
// Part of the LLFS Project, under Apache License v2.0.
4+
// See https://www.apache.org/licenses/LICENSE-2.0 for license information.
5+
// SPDX short identifier: Apache-2.0
6+
//
7+
//+++++++++++-+-+--+----- --- -- - - - -
8+
#include <llfs/no_outgoing_refs_cache.hpp>
9+
10+
namespace llfs {
11+
12+
//==#==========+==+=+=++=+++++++++++-+-+--+----- --- -- - - - -
13+
//
14+
NoOutgoingRefsCache::NoOutgoingRefsCache(const PageIdFactory& page_ids) noexcept
15+
: page_ids_{page_ids}
16+
, cache_(this->page_ids_.get_physical_page_count().value())
17+
{
18+
}
19+
20+
//==#==========+==+=+=++=+++++++++++-+-+--+----- --- -- - - - -
21+
//
22+
void NoOutgoingRefsCache::set_page_state(PageId page_id,
23+
HasOutgoingRefs new_has_outgoing_refs_state) noexcept
24+
{
25+
BATT_CHECK_EQ(PageIdFactory::get_device_id(page_id), this->page_ids_.get_device_id());
26+
const u64 physical_page = this->page_ids_.get_physical_page(page_id);
27+
const page_generation_int generation = this->page_ids_.get_generation(page_id);
28+
BATT_CHECK_LT((usize)physical_page, this->cache_.size());
29+
30+
u64 new_cache_entry = generation << this->kGenerationShift;
31+
32+
// Set the "valid" bit to 1.
33+
//
34+
new_cache_entry |= this->kValidBitMask;
35+
36+
// If new_has_outgoing_refs_state has value false, page_id has no outgoing references.
37+
//
38+
if (!new_has_outgoing_refs_state) {
39+
// Set the "has no outgoing references" bit to 1.
40+
//
41+
new_cache_entry |= this->kHasNoOutgoingRefsBitMask;
42+
}
43+
44+
u64 old_cache_entry = this->cache_[physical_page].exchange(new_cache_entry);
45+
46+
// Two sanity checks:
47+
// 1) We are not going backwards in generation.
48+
// 2) If the cache entry is set of the same generation multiple times, the same value should be
49+
// set.
50+
//
51+
page_generation_int old_generation = old_cache_entry >> this->kGenerationShift;
52+
BATT_CHECK_GE(generation, old_generation);
53+
if (generation == old_generation) {
54+
BATT_CHECK_EQ(new_cache_entry, old_cache_entry);
55+
}
56+
}
57+
58+
//==#==========+==+=+=++=+++++++++++-+-+--+----- --- -- - - - -
59+
//
60+
batt::BoolStatus NoOutgoingRefsCache::has_outgoing_refs(PageId page_id) const noexcept
61+
{
62+
BATT_CHECK_EQ(PageIdFactory::get_device_id(page_id), this->page_ids_.get_device_id());
63+
const u64 physical_page = this->page_ids_.get_physical_page(page_id);
64+
const page_generation_int generation = this->page_ids_.get_generation(page_id);
65+
BATT_CHECK_LT((usize)physical_page, this->cache_.size());
66+
67+
u64 current_cache_entry = this->cache_[physical_page].load();
68+
page_generation_int stored_generation = current_cache_entry >> this->kGenerationShift;
69+
u64 outgoing_refs_status = current_cache_entry & this->kOutgoingRefsStatusBitsMask;
70+
71+
// If the generation that is currently stored in the cache is not the same as the generation we
72+
// are querying for, this cache entry is invalid. Thus, we return a "unknown" status.
73+
//
74+
if (stored_generation != generation) {
75+
return batt::BoolStatus::kUnknown;
76+
}
77+
78+
switch (outgoing_refs_status) {
79+
case 0:
80+
// Bit status 00, not traced yet.
81+
//
82+
return batt::BoolStatus::kUnknown;
83+
case 1:
84+
BATT_PANIC() << "The lower two outgoing refs bits in a cache entry can never be 01!";
85+
BATT_UNREACHABLE();
86+
case 2:
87+
// Bit status 10, has outgoing refs.
88+
//
89+
return batt::BoolStatus::kTrue;
90+
case 3:
91+
// Bit status 11, no outgoing refs.
92+
//
93+
return batt::BoolStatus::kFalse;
94+
default:
95+
BATT_PANIC() << "Impossible outgoing refs bits state: " << outgoing_refs_status;
96+
BATT_UNREACHABLE();
97+
}
98+
}
99+
} // namespace llfs
Lines changed: 102 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,102 @@
1+
//#=##=##=#==#=#==#===#+==#+==========+==+=+=+=+=+=++=+++=+++++=-++++=-+++++++++++
2+
//
3+
// Part of the LLFS Project, under Apache License v2.0.
4+
// See https://www.apache.org/licenses/LICENSE-2.0 for license information.
5+
// SPDX short identifier: Apache-2.0
6+
//
7+
//+++++++++++-+-+--+----- --- -- - - - -
8+
9+
#pragma once
10+
#ifndef LLFS_NO_OUTGOING_REFS_CACHE_HPP
11+
#define LLFS_NO_OUTGOING_REFS_CACHE_HPP
12+
13+
#include <llfs/api_types.hpp>
14+
#include <llfs/page_id_factory.hpp>
15+
16+
#include <batteries/bool_status.hpp>
17+
18+
#include <atomic>
19+
#include <vector>
20+
21+
namespace llfs {
22+
23+
//=#=#==#==#===============+=+=+=+=++=++++++++++++++-++-+--+-+----+---------------
24+
/** \brief A cache to store information about a page's outgoing references to other pages. Can be
25+
* used by an implementer of PageTracer as a way to organize and look up this information on the
26+
* PageDevice level. This cache is implemented as a vector of unsigned 64-bit integers, where every
27+
* element of the vector represents the outgoing refs "state" of a physical page in a PageDevice.
28+
* The lowest bit in an element represents if the page has no outgoing refs. The second lowest bit
29+
* represents the validity of the page's state to help determine if a page's outgoing refs have ever
30+
* been traced. The remaining upper 62 bits are used to store the generation of the physical page.
31+
*
32+
* Example cache entry: 0000000000000000000000000000000000000000000000000000000000000110
33+
* The upper 62 bits represent generation, which in this case is 1. The second lowest bit is the
34+
* validity bit, which is 1 here. This indicates that the page has been traced and therefore the
35+
* cache entry contains valid information for the page of this generation. The lowest bit is 0,
36+
* which indicates that the page has outgoing references to other pages.
37+
*/
38+
class NoOutgoingRefsCache
39+
{
40+
public:
41+
explicit NoOutgoingRefsCache(const PageIdFactory& page_ids) noexcept;
42+
43+
NoOutgoingRefsCache(const NoOutgoingRefsCache&) = delete;
44+
NoOutgoingRefsCache& operator=(const NoOutgoingRefsCache&) = delete;
45+
46+
//----- --- -- - - - -
47+
/** \brief Sets the two outgoing refs state bits for the given `page_id` based on
48+
* whether the page has outgoing refs or not, as indicated by `new_has_outgoing_refs_state`. This
49+
* function also updates the generation stored in the cache for the physical page associated with
50+
* `page_id`.
51+
*
52+
* @param page_id The id of the page whose outgoing refs information we are storing.
53+
*
54+
* @param new_has_outgoing_refs_state The status to store, indicating whether or not the page has
55+
* outgoing refs. A value of `false` indicates that the page does not have any outgoing refs, and
56+
* a value of `true` indicates that a page does have outgoing refs.
57+
*/
58+
void set_page_state(PageId page_id, HasOutgoingRefs new_has_outgoing_refs_state) noexcept;
59+
60+
//----- --- -- - - - -
61+
/** \brief Queries for whether or not the page with id `page_id` contains outgoing references to
62+
* other pages.
63+
*
64+
* \return Returns a `batt::BoolStatus` type, where `kFalse` indicates that the page has no
65+
* outgoing refs, `kTrue` indicates that the page does have outgoing refs, and `kUnknown`
66+
* indicates that the cache entry for the given page does not have any valid information stored in
67+
* it currently.
68+
*/
69+
batt::BoolStatus has_outgoing_refs(PageId page_id) const noexcept;
70+
71+
private:
72+
/** \brief A mask to retrieve the lowest two outgoing refs state bits.
73+
*/
74+
static constexpr u64 kOutgoingRefsStatusBitsMask = 0b11;
75+
76+
/** \brief A mask to access the "valid" bit of the two outgoing refs state bits. This is the
77+
* higher of the two bits.
78+
*/
79+
static constexpr u64 kValidBitMask = 0b10;
80+
81+
/** \brief A mask to access the "has no outgoing references" bit of the two outgoing refs state
82+
* bits. This is the lower of the two bits.
83+
*/
84+
static constexpr u64 kHasNoOutgoingRefsBitMask = 0b01;
85+
86+
/** \brief A constant representing the number of bits to shift a cache entry in order retrieve the
87+
* generation stored.
88+
*/
89+
static constexpr u8 kGenerationShift = 2;
90+
91+
/** \brief A PageIdFactory object to help resolve physical page and generation values from a
92+
* PageId object.
93+
*/
94+
const PageIdFactory page_ids_;
95+
96+
/** \brief The vector storing the cache entries, indexed by physical page id.
97+
*/
98+
std::vector<std::atomic<u64>> cache_;
99+
};
100+
} // namespace llfs
101+
102+
#endif // LLFS_NO_OUTGOING_REFS_CACHE_HPP

src/llfs/page_cache.cpp

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ namespace llfs {
2222

2323
//==#==========+==+=+=++=+++++++++++-+-+--+----- --- -- - - - -
2424
//
25-
usize get_page_size(const PageCache::PageDeviceEntry* entry)
25+
usize get_page_size(const PageDeviceEntry* entry)
2626
{
2727
return entry ? get_page_size(entry->arena) : 0;
2828
}
@@ -420,14 +420,21 @@ void PageCache::prefetch_hint(PageId page_id)
420420

421421
//==#==========+==+=+=++=+++++++++++-+-+--+----- --- -- - - - -
422422
//
423-
Slice<PageCache::PageDeviceEntry* const> PageCache::all_devices() const
423+
const std::vector<std::unique_ptr<PageDeviceEntry>>& PageCache::devices_by_id() const
424+
{
425+
return this->page_devices_;
426+
}
427+
428+
//==#==========+==+=+=++=+++++++++++-+-+--+----- --- -- - - - -
429+
//
430+
Slice<PageDeviceEntry* const> PageCache::all_devices() const
424431
{
425432
return as_slice(this->page_devices_by_page_size_);
426433
}
427434

428435
//==#==========+==+=+=++=+++++++++++-+-+--+----- --- -- - - - -
429436
//
430-
Slice<PageCache::PageDeviceEntry* const> PageCache::devices_with_page_size(usize size) const
437+
Slice<PageDeviceEntry* const> PageCache::devices_with_page_size(usize size) const
431438
{
432439
const usize size_log2 = batt::log2_ceil(size);
433440
BATT_CHECK_EQ(size, usize{1} << size_log2) << "page size must be a power of 2";
@@ -437,8 +444,7 @@ Slice<PageCache::PageDeviceEntry* const> PageCache::devices_with_page_size(usize
437444

438445
//==#==========+==+=+=++=+++++++++++-+-+--+----- --- -- - - - -
439446
//
440-
Slice<PageCache::PageDeviceEntry* const> PageCache::devices_with_page_size_log2(
441-
usize size_log2) const
447+
Slice<PageDeviceEntry* const> PageCache::devices_with_page_size_log2(usize size_log2) const
442448
{
443449
BATT_CHECK_LT(size_log2, kMaxPageSizeLog2);
444450

@@ -533,7 +539,7 @@ void PageCache::purge(PageId page_id, u64 callers, u64 job_id)
533539

534540
//==#==========+==+=+=++=+++++++++++-+-+--+----- --- -- - - - -
535541
//
536-
PageCache::PageDeviceEntry* PageCache::get_device_for_page(PageId page_id)
542+
PageDeviceEntry* PageCache::get_device_for_page(PageId page_id)
537543
{
538544
const page_device_id_int device_id = PageIdFactory::get_device_id(page_id);
539545
if (BATT_HINT_FALSE(device_id >= this->page_devices_.size())) {

0 commit comments

Comments
 (0)