diff --git a/src/core/CMakeLists.txt b/src/core/CMakeLists.txt index 6d12a74f4..d6604b8b8 100644 --- a/src/core/CMakeLists.txt +++ b/src/core/CMakeLists.txt @@ -21,7 +21,7 @@ add_subdirectory(json) set(SEARCH_LIB query_parser) add_library(dfly_core allocation_tracker.cc bloom.cc compact_object.cc dense_set.cc - dragonfly_core.cc extent_tree.cc + dragonfly_core.cc extent_tree.cc intrusive_string_list.cc interpreter.cc glob_matcher.cc mi_memory_resource.cc qlist.cc sds_utils.cc segment_allocator.cc score_map.cc small_string.cc sorted_map.cc task_queue.cc tx_queue.cc string_set.cc string_map.cc top_keys.cc detail/bitpacking.cc) diff --git a/src/core/intrusive_string_list.cc b/src/core/intrusive_string_list.cc new file mode 100644 index 000000000..abbc531d9 --- /dev/null +++ b/src/core/intrusive_string_list.cc @@ -0,0 +1,10 @@ +// Copyright 2025, DragonflyDB authors. All rights reserved. +// See LICENSE for licensing terms. +// + +#include "intrusive_string_list.h" + +namespace dfly { +ISLEntry IntrusiveStringList::Iterator::end_; + +} // namespace dfly diff --git a/src/core/intrusive_string_list.h b/src/core/intrusive_string_list.h index 48e301015..4d13127f8 100644 --- a/src/core/intrusive_string_list.h +++ b/src/core/intrusive_string_list.h @@ -4,16 +4,18 @@ #pragma once -#include +// #include #include #include +#include "base/logging.h" + extern "C" { #include "redis/zmalloc.h" } namespace dfly { - +// doesn't possess memory, it should be created and release manually class ISLEntry { friend class IntrusiveStringList; @@ -31,8 +33,17 @@ class ISLEntry { data_ = data; } - static void Destroy(ISLEntry entry) { - zfree(entry.Raw()); + ISLEntry(const ISLEntry& e) = default; + ISLEntry(ISLEntry&& e) { + data_ = e.data_; + e.data_ = nullptr; + } + + ISLEntry& operator=(const ISLEntry& e) = default; + ISLEntry& operator=(ISLEntry&& e) { + data_ = e.data_; + e.data_ = nullptr; + return *this; } operator bool() const { @@ -56,20 +67,14 @@ class ISLEntry { return res; } - void SetExpiryTime(uint32_t ttl_sec) { - if (HasExpiry()) { - auto* ttl_pos = Raw() + sizeof(char*); - std::memcpy(ttl_pos, &ttl_sec, sizeof(ttl_sec)); - } else { - ISLEntry new_entry = Create(Key(), ttl_sec); - std::swap(data_, new_entry.data_); - Destroy(new_entry); - } + // TODO consider another option to implement iterator + ISLEntry* operator->() { + return this; } - private: - static ISLEntry Create(std::string_view key, uint32_t ttl_sec = UINT32_MAX) { - char* next = nullptr; + protected: + static ISLEntry Create(std::string_view key, char* next = nullptr, + uint32_t ttl_sec = UINT32_MAX) { uint32_t key_size = key.size(); bool has_ttl = ttl_sec != UINT32_MAX; @@ -96,12 +101,20 @@ class ISLEntry { return res; } + static void Destroy(ISLEntry& entry) { + zfree(entry.Raw()); + } + ISLEntry Next() const { ISLEntry next; std::memcpy(&next.data_, Raw(), sizeof(next)); return next; } + ISLEntry FakePrev() { + return ISLEntry(reinterpret_cast(&data_)); + } + void SetNext(ISLEntry next) { std::memcpy(Raw(), &next, sizeof(next)); } @@ -135,6 +148,15 @@ class ISLEntry { return (uptr() & kTtlBit) != 0; } + [[nodiscard]] bool UpdateTtl(uint32_t ttl_sec) { + if (HasTtl()) { + auto* ttl_pos = Raw() + sizeof(char*); + std::memcpy(ttl_pos, &ttl_sec, sizeof(ttl_sec)); + return true; + } + return false; + } + std::uint32_t GetTtlSize() const { return HasTtl() ? sizeof(std::uint32_t) : 0; } @@ -145,8 +167,87 @@ class ISLEntry { char* data_ = nullptr; }; +class UniqueISLEntry : private ISLEntry { + public: + ~UniqueISLEntry() { + Destroy(*this); + } + + UniqueISLEntry() = default; + UniqueISLEntry(ISLEntry e) : ISLEntry(e) { + } + UniqueISLEntry(UniqueISLEntry&& e) = default; + UniqueISLEntry& operator=(UniqueISLEntry&& e) = default; + + using ISLEntry::ExpiryTime; + using ISLEntry::HasExpiry; + using ISLEntry::Key; + using ISLEntry::operator bool; + + ISLEntry Release() { + ISLEntry res = *this; + data_ = nullptr; + return res; + } + + private: + UniqueISLEntry(const UniqueISLEntry&) = delete; +}; + class IntrusiveStringList { public: + class Iterator { + public: + using iterator_category = std::forward_iterator_tag; + using difference_type = std::ptrdiff_t; + using value_type = ISLEntry; + using pointer = ISLEntry*; + using reference = ISLEntry&; + + Iterator(ISLEntry prev = end_.FakePrev()) : prev_(prev) { + DCHECK(prev); + } + + uint32_t ExpiryTime() const { + return prev_.Next().ExpiryTime(); + } + + void SetExpiryTime(uint32_t ttl_sec) { + auto entry = prev_.Next(); + + if (!entry.UpdateTtl(ttl_sec)) { + ISLEntry new_entry = ISLEntry::Create(entry.Key(), entry.Next().data_, ttl_sec); + ISLEntry::Destroy(entry); + prev_.SetNext(new_entry); + } + } + + bool HasExpiry() const { + return prev_.Next().HasExpiry(); + } + + Iterator& operator++() { + prev_ = prev_.Next(); + return *this; + } + + operator bool() const { + return prev_.Next(); + } + + value_type operator*() { + return prev_.Next(); + } + + value_type operator->() { + return prev_.Next(); + } + + private: + ISLEntry prev_; + static ISLEntry end_; + }; + ~IntrusiveStringList() { while (start_) { auto next = start_.Next(); @@ -161,14 +262,17 @@ class IntrusiveStringList { r.start_ = {}; } + Iterator begin() { + return start_.FakePrev(); + } + ISLEntry Insert(ISLEntry e) { e.SetNext(start_); start_ = e; return start_; } - // TODO rewrite, because it's dangerous operations, ISLEntry shouldn't returned without owner - [[nodiscard]] ISLEntry Pop(uint32_t curr_time) { + UniqueISLEntry Pop(uint32_t curr_time) { for (auto it = start_; it && it.ExpiryTime() < curr_time; it = start_) { start_ = it.Next(); ISLEntry::Destroy(it); @@ -187,13 +291,13 @@ class IntrusiveStringList { // TODO consider to wrap ISLEntry to prevent usage out of the list ISLEntry Emplace(std::string_view key, uint32_t ttl_sec = UINT32_MAX) { - return Insert(ISLEntry::Create(key, ttl_sec)); + return Insert(ISLEntry::Create(key, nullptr, ttl_sec)); } // TODO consider to wrap ISLEntry to prevent usage out of the list - ISLEntry Find(std::string_view str) const { - auto it = start_; - for (; it && it.Key() != str; it = it.Next()) + IntrusiveStringList::Iterator Find(std::string_view str) { + auto it = begin(); + for (; it && it->Key() != str; ++it) ; return it; } diff --git a/src/core/intrusive_string_set.h b/src/core/intrusive_string_set.h index a1a3b86e5..fd18d27d8 100644 --- a/src/core/intrusive_string_set.h +++ b/src/core/intrusive_string_set.h @@ -20,7 +20,7 @@ class IntrusiveStringSet { std::vector>; public: - class Iterator { + class iterator { public: using iterator_category = std::forward_iterator_tag; using difference_type = std::ptrdiff_t; @@ -28,14 +28,18 @@ class IntrusiveStringSet { using pointer = std::string_view*; using reference = std::string_view&; - Iterator(Buckets::iterator it, ISLEntry prev) : buckets_it_(it), prev_(prev) { + iterator(Buckets::iterator it, + IntrusiveStringList::Iterator entry = IntrusiveStringList::Iterator()) + : buckets_it_(it), entry_(entry) { } // uint32_t ExpiryTime() const { // return prev.Next()->ExpiryTime(); // } - // void SetExpiryTime(uint32_t ttl_sec); + void SetExpiryTime(uint32_t ttl_sec) { + entry_.SetExpiryTime(ttl_sec); + } // bool HasExpiry() const { // return curr_entry_.HasExpiry(); @@ -43,11 +47,31 @@ class IntrusiveStringSet { // void Advance(); + bool operator==(const iterator& r) const { + return buckets_it_ == r.buckets_it_; + } + + bool operator!=(const iterator& r) const { + return !operator==(r); + } + + IntrusiveStringList::Iterator::value_type operator*() { + return *entry_; + } + + IntrusiveStringList::Iterator operator->() { + return entry_; + } + private: Buckets::iterator buckets_it_; - ISLEntry prev_; + IntrusiveStringList::Iterator entry_; }; + iterator end() { + return iterator(entries_.end()); + } + explicit IntrusiveStringSet(PMR_NS::memory_resource* mr = PMR_NS::get_default_resource()) : entries_(mr) { } @@ -139,8 +163,7 @@ class IntrusiveStringSet { return entries_idx << (32 - capacity_log_); } - // return unowned ISLEntry, so it should be destroyed manually - [[nodiscard]] ISLEntry Pop() { + UniqueISLEntry Pop() { for (auto& bucket : entries_) { if (auto res = bucket.Pop(time_now_); res) { --size_; @@ -157,20 +180,17 @@ class IntrusiveStringSet { return entries_[bucket_id].Erase(str); } - ISLEntry Find(std::string_view member) const { + iterator Find(std::string_view member) { if (entries_.empty()) - return {}; + return iterator(entries_.end()); auto bucket_id = BucketId(Hash(member)); - auto res = entries_[bucket_id].Find(member); - if (!res) { - bucket_id = BucketId(Hash(member)); - res = entries_[bucket_id].Find(member); - } - return res; + auto entry_it = entries_.begin() + bucket_id; + auto res = entry_it->Find(member); + return iterator(res ? entry_it : entries_.end(), res); } - bool Contains(std::string_view member) const { - return Find(member); + bool Contains(std::string_view member) { + return Find(member) != end(); } // Returns the number of elements in the map. Note that it might be that some of these elements @@ -203,7 +223,7 @@ class IntrusiveStringSet { auto list = std::move(entries_[i]); for (auto entry = list.Pop(time_now_); entry; entry = list.Pop(time_now_)) { auto bucket_id = BucketId(Hash(entry.Key())); - entries_[bucket_id].Insert(entry); + entries_[bucket_id].Insert(entry.Release()); } } } diff --git a/src/core/intrusive_string_set_test.cc b/src/core/intrusive_string_set_test.cc index 5b53b1611..ec7f29495 100644 --- a/src/core/intrusive_string_set_test.cc +++ b/src/core/intrusive_string_set_test.cc @@ -87,15 +87,15 @@ TEST_F(IntrusiveStringSetTest, IntrusiveStringListTest) { test = isl.Emplace("23456789"); - EXPECT_EQ(isl.Find("0123456789").Key(), "0123456789"sv); - EXPECT_EQ(isl.Find("23456789").Key(), "23456789"sv); - EXPECT_EQ(isl.Find("123456789").Key(), "123456789"sv); - EXPECT_EQ(isl.Find("test"), ISLEntry()); + EXPECT_EQ(isl.Find("0123456789")->Key(), "0123456789"sv); + EXPECT_EQ(isl.Find("23456789")->Key(), "23456789"sv); + EXPECT_EQ(isl.Find("123456789")->Key(), "123456789"sv); + EXPECT_FALSE(isl.Find("test")); EXPECT_TRUE(isl.Erase("23456789")); - EXPECT_EQ(isl.Find("23456789"), ISLEntry()); + EXPECT_FALSE(isl.Find("23456789")); EXPECT_FALSE(isl.Erase("test")); - EXPECT_EQ(isl.Find("test"), ISLEntry()); + EXPECT_FALSE(isl.Find("test")); } TEST_F(IntrusiveStringSetTest, IntrusiveStringSetAddFindTest) { @@ -113,7 +113,7 @@ TEST_F(IntrusiveStringSetTest, IntrusiveStringSetAddFindTest) { for (const auto& s : test_set) { auto e = ss.Find(s); - EXPECT_EQ(e.Key(), s); + EXPECT_EQ(e->Key(), s); } EXPECT_EQ(ss.Capacity(), 16384); @@ -240,9 +240,9 @@ TEST_F(IntrusiveStringSetTest, Resizing) { for (auto j = strs.begin(); j != it; ++j) { const auto& str = *j; auto it = ss_->Find(str); - ASSERT_TRUE(it); - EXPECT_TRUE(it.HasExpiry()); - EXPECT_EQ(it.ExpiryTime(), ss_->time_now() + 1); + ASSERT_NE(it, ss_->end()); + EXPECT_TRUE(it->HasExpiry()); + EXPECT_EQ(it->ExpiryTime(), ss_->time_now() + 1); } } ++size; @@ -465,21 +465,21 @@ TEST_F(IntrusiveStringSetTest, Pop) { TEST_F(IntrusiveStringSetTest, SetFieldExpireHasExpiry) { EXPECT_TRUE(ss_->Add("k1", 100)); auto k = ss_->Find("k1"); - EXPECT_TRUE(k.HasExpiry()); - EXPECT_EQ(k.ExpiryTime(), 100); + EXPECT_TRUE(k->HasExpiry()); + EXPECT_EQ(k->ExpiryTime(), 100); k.SetExpiryTime(1); - EXPECT_TRUE(k.HasExpiry()); - EXPECT_EQ(k.ExpiryTime(), 1); + EXPECT_TRUE(k->HasExpiry()); + EXPECT_EQ(k->ExpiryTime(), 1); } -// TEST_F(IntrusiveStringSetTest, SetFieldExpireNoHasExpiry) { -// EXPECT_TRUE(ss_->Add("k1")); -// auto k = ss_->Find("k1"); -// EXPECT_FALSE(k.HasExpiry()); -// k.SetExpiryTime(10); -// EXPECT_TRUE(k.HasExpiry()); -// EXPECT_EQ(k.ExpiryTime(), 10); -// } +TEST_F(IntrusiveStringSetTest, SetFieldExpireNoHasExpiry) { + EXPECT_TRUE(ss_->Add("k1")); + auto k = ss_->Find("k1"); + EXPECT_FALSE(k->HasExpiry()); + k.SetExpiryTime(10); + EXPECT_TRUE(k->HasExpiry()); + EXPECT_EQ(k->ExpiryTime(), 10); +} // TEST_F(IntrusiveStringSetTest, Ttl) { // EXPECT_TRUE(ss_->Add("bla"sv, 1));