Skip to content

Commit

Permalink
Reduce the overhead of EntryOrDelete
Browse files Browse the repository at this point in the history
  • Loading branch information
@JaySon-Huang
JaySon-Huang committed Feb 21, 2022
1 parent 01251b0 commit e2cb6fe
Show file tree
Hide file tree
Showing 3 changed files with 40 additions and 40 deletions.
18 changes: 9 additions & 9 deletions dbms/src/Storages/Page/V3/PageDirectory.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ namespace PS::V3
* VersionedPageEntries methods *
********************************/

std::optional<std::shared_ptr<PageEntryV3>> VersionedPageEntries::getEntryNotSafe(UInt64 seq) const
std::shared_ptr<PageEntryV3> VersionedPageEntries::getEntryNotSafe(UInt64 seq) const
{
// entries are sorted by <ver, epoch>, find the first one less than <ver+1, 0>
if (auto iter = MapUtils::findLess(entries, PageVersionType(seq + 1));
Expand All @@ -56,10 +56,10 @@ std::optional<std::shared_ptr<PageEntryV3>> VersionedPageEntries::getEntryNotSaf
if (!iter->second.isDelete())
return iter->second.entryPtr();
}
return std::nullopt;
return nullptr;
}

std::optional<std::shared_ptr<PageEntryV3>> VersionedPageEntries::getEntry(UInt64 seq) const
std::shared_ptr<PageEntryV3> VersionedPageEntries::getEntry(UInt64 seq) const
{
auto page_lock = acquireLock();
return getEntryNotSafe(seq);
Expand Down Expand Up @@ -405,9 +405,9 @@ PageIDAndEntryV3 PageDirectory::get(PageId page_id, const PageDirectorySnapshotP
throw Exception(fmt::format("Entry [id={}] not exist!", page_id), ErrorCodes::PS_ENTRY_NOT_EXISTS);
}

if (auto entry = iter->second->getEntry(snap->sequence); entry)
if (auto entry = iter->second->getEntry(snap->sequence); entry != nullptr)
{
return PageIDAndEntryV3(page_id, *entry.value());
return PageIDAndEntryV3(page_id, *entry);
}

throw Exception(fmt::format("Entry [id={}] [seq={}] not exist!", page_id, snap->sequence), ErrorCodes::PS_ENTRY_NO_VALID_VERSION);
Expand Down Expand Up @@ -442,9 +442,9 @@ PageIDAndEntriesV3 PageDirectory::get(const PageIds & page_ids, const PageDirect
for (size_t idx = 0; idx < page_ids.size(); ++idx)
{
const auto & iter = iters[idx];
if (auto entry = iter->second->getEntry(snap->sequence); entry)
if (auto entry = iter->second->getEntry(snap->sequence); entry != nullptr)
{
id_entries.emplace_back(page_ids[idx], *entry.value());
id_entries.emplace_back(page_ids[idx], *entry);
}
else
throw Exception(fmt::format("Entry [id={}] [seq={}] at [idx={}] not exist!", page_ids[idx], snap->sequence, idx), ErrorCodes::PS_ENTRY_NO_VALID_VERSION);
Expand Down Expand Up @@ -549,10 +549,10 @@ void PageDirectory::apply(PageEntriesEdit && edit)
if (auto entry = (updating_locks.find(r.ori_page_id) != updating_locks.end()
? iter->second->getEntryNotSafe(last_sequence + 1)
: iter->second->getEntry(last_sequence + 1));
entry)
entry != nullptr)
{
// copy the entry shared ptr to be ref
std::shared_ptr<PageEntryV3> entry_ptr_copy = entry.value();
std::shared_ptr<PageEntryV3> entry_ptr_copy = entry;
updating_pages[idx]->createNewVersion(last_sequence + 1, std::move(entry_ptr_copy));
}
else
Expand Down
20 changes: 10 additions & 10 deletions dbms/src/Storages/Page/V3/PageDirectory.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -68,26 +68,26 @@ struct EntryOrDelete
{
public:
explicit EntryOrDelete(bool del)
: entry_or_delete(std::nullopt)
: entry_or_delete(nullptr)
{
assert(del == true);
}
explicit EntryOrDelete(std::shared_ptr<PageEntryV3> && entry)
: entry_or_delete(std::move(entry))
{
assert(entry_or_delete.value() != nullptr);
assert(entry_or_delete != nullptr);
}

inline bool isDelete() const { return !entry_or_delete.has_value(); }
inline bool isDelete() const { return entry_or_delete == nullptr; }

const std::shared_ptr<PageEntryV3> & entryPtr() const
inline const std::shared_ptr<PageEntryV3> & entryPtr() const
{
assert(!isDelete());
return entry_or_delete.value();
assert(entry_or_delete != nullptr);
return entry_or_delete;
}

private:
std::optional<std::shared_ptr<PageEntryV3>> entry_or_delete;
std::shared_ptr<PageEntryV3> entry_or_delete;
};

using PageLock = std::unique_ptr<std::lock_guard<std::mutex>>;
Expand Down Expand Up @@ -116,10 +116,10 @@ class VersionedPageEntries

// Return the shared_ptr to the entry that is visible by `seq`.
// If the entry is not visible (deleted or not exist for `seq`), then
// return `std::nullopt`.
std::optional<std::shared_ptr<PageEntryV3>> getEntry(UInt64 seq) const;
// return `nullptr`.
std::shared_ptr<PageEntryV3> getEntry(UInt64 seq) const;

std::optional<std::shared_ptr<PageEntryV3>> getEntryNotSafe(UInt64 seq) const;
std::shared_ptr<PageEntryV3> getEntryNotSafe(UInt64 seq) const;

std::shared_ptr<PageEntryV3> getLatestEntryNotSafe() const;

Expand Down
42 changes: 21 additions & 21 deletions dbms/src/Storages/Page/V3/tests/gtest_page_directory.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -499,54 +499,54 @@ TEST_F(VersionedEntriesTest, InsertGet)
INSERT_ENTRY(10);

// Insert some entries with version
ASSERT_FALSE(entries.getEntry(1).has_value());
ASSERT_TRUE(isSameEntry(**entries.getEntry(2), entry_v2));
ASSERT_TRUE(isSameEntry(**entries.getEntry(3), entry_v2));
ASSERT_TRUE(isSameEntry(**entries.getEntry(4), entry_v2));
ASSERT_FALSE(entries.getEntry(1));
ASSERT_TRUE(isSameEntry(*entries.getEntry(2), entry_v2));
ASSERT_TRUE(isSameEntry(*entries.getEntry(3), entry_v2));
ASSERT_TRUE(isSameEntry(*entries.getEntry(4), entry_v2));
for (UInt64 seq = 5; seq < 10; ++seq)
{
ASSERT_TRUE(isSameEntry(**entries.getEntry(seq), entry_v5));
ASSERT_TRUE(isSameEntry(*entries.getEntry(seq), entry_v5));
}
for (UInt64 seq = 10; seq < 20; ++seq)
{
ASSERT_TRUE(isSameEntry(**entries.getEntry(seq), entry_v10));
ASSERT_TRUE(isSameEntry(*entries.getEntry(seq), entry_v10));
}

// Insert some entries with version && gc epoch
INSERT_GC_ENTRY(2, 1);
INSERT_GC_ENTRY(5, 1);
INSERT_GC_ENTRY(5, 2);
ASSERT_FALSE(entries.getEntry(1).has_value());
ASSERT_TRUE(isSameEntry(**entries.getEntry(2), entry_gc_v2_1));
ASSERT_TRUE(isSameEntry(**entries.getEntry(3), entry_gc_v2_1));
ASSERT_TRUE(isSameEntry(**entries.getEntry(4), entry_gc_v2_1));
ASSERT_FALSE(entries.getEntry(1));
ASSERT_TRUE(isSameEntry(*entries.getEntry(2), entry_gc_v2_1));
ASSERT_TRUE(isSameEntry(*entries.getEntry(3), entry_gc_v2_1));
ASSERT_TRUE(isSameEntry(*entries.getEntry(4), entry_gc_v2_1));
for (UInt64 seq = 5; seq < 10; ++seq)
{
ASSERT_TRUE(isSameEntry(**entries.getEntry(seq), entry_gc_v5_2));
ASSERT_TRUE(isSameEntry(*entries.getEntry(seq), entry_gc_v5_2));
}
for (UInt64 seq = 10; seq < 20; ++seq)
{
ASSERT_TRUE(isSameEntry(**entries.getEntry(seq), entry_v10));
ASSERT_TRUE(isSameEntry(*entries.getEntry(seq), entry_v10));
}

// Insert delete. Can not get entry with seq >= delete_version.
// But it won't affect reading with old seq.
entries.createDelete(15);
ASSERT_FALSE(entries.getEntry(1).has_value());
ASSERT_TRUE(isSameEntry(**entries.getEntry(2), entry_gc_v2_1));
ASSERT_TRUE(isSameEntry(**entries.getEntry(3), entry_gc_v2_1));
ASSERT_TRUE(isSameEntry(**entries.getEntry(4), entry_gc_v2_1));
ASSERT_FALSE(entries.getEntry(1));
ASSERT_TRUE(isSameEntry(*entries.getEntry(2), entry_gc_v2_1));
ASSERT_TRUE(isSameEntry(*entries.getEntry(3), entry_gc_v2_1));
ASSERT_TRUE(isSameEntry(*entries.getEntry(4), entry_gc_v2_1));
for (UInt64 seq = 5; seq < 10; ++seq)
{
ASSERT_TRUE(isSameEntry(**entries.getEntry(seq), entry_gc_v5_2));
ASSERT_TRUE(isSameEntry(*entries.getEntry(seq), entry_gc_v5_2));
}
for (UInt64 seq = 10; seq < 15; ++seq)
{
ASSERT_TRUE(isSameEntry(**entries.getEntry(seq), entry_v10));
ASSERT_TRUE(isSameEntry(*entries.getEntry(seq), entry_v10));
}
for (UInt64 seq = 15; seq < 20; ++seq)
{
ASSERT_FALSE(entries.getEntry(seq).has_value());
ASSERT_FALSE(entries.getEntry(seq));
}
}

Expand Down Expand Up @@ -609,13 +609,13 @@ try
INSERT_ENTRY(5);

// Read with snapshot seq=2
ASSERT_TRUE(isSameEntry(entry_v2, **entries.getEntry(2)));
ASSERT_TRUE(isSameEntry(entry_v2, *entries.getEntry(2)));

// Mock that gc applied and insert <2, 1>
INSERT_GC_ENTRY(2, 1);

// Now we should read the entry <2, 1> with seq=2
ASSERT_TRUE(isSameEntry(entry_gc_v2_1, **entries.getEntry(2)));
ASSERT_TRUE(isSameEntry(entry_gc_v2_1, *entries.getEntry(2)));

// <2,0> get removed
auto removed_entries = gcAndGetRemovedEntries(2);
Expand Down

0 comments on commit e2cb6fe

Please sign in to comment.