feat: add iterators

src/core/intrusive_string_list.cc

@@ -5,6 +5,6 @@
 #include "intrusive_string_list.h"
 
 namespace dfly {
-ISLEntry IntrusiveStringList::Iterator::end_;
+ISLEntry IntrusiveStringList::end_;
 
 }  // namespace dfly

src/core/intrusive_string_list.h

@@ -196,7 +196,7 @@ class UniqueISLEntry : private ISLEntry {
 
 class IntrusiveStringList {
  public:
-  class Iterator {
+  class iterator {
    public:
     using iterator_category = std::forward_iterator_tag;
     using difference_type = std::ptrdiff_t;
@@ -204,7 +204,7 @@ class IntrusiveStringList {
     using pointer = ISLEntry*;
     using reference = ISLEntry&;
 
-    Iterator(ISLEntry prev = end_.FakePrev()) : prev_(prev) {
+    iterator(ISLEntry prev = end_.FakePrev()) : prev_(prev) {
       DCHECK(prev);
     }
 
@@ -226,7 +226,7 @@ class IntrusiveStringList {
       return prev_.Next().HasExpiry();
     }
 
-    Iterator& operator++() {
+    iterator& operator++() {
       prev_ = prev_.Next();
       return *this;
     }
@@ -243,9 +243,16 @@ class IntrusiveStringList {
       return prev_.Next();
     }
 
+    bool operator==(const iterator& r) {
+      return prev_.Next() == r.prev_.Next();
+    }
+
+    bool operator!=(const iterator& r) {
+      return !operator==(r);
+    }
+
    private:
     ISLEntry prev_;
-    static ISLEntry end_;
   };
 
   ~IntrusiveStringList() {
@@ -262,10 +269,14 @@ class IntrusiveStringList {
     r.start_ = {};
   }
 
-  Iterator begin() {
+  iterator begin() {
     return start_.FakePrev();
   }
 
+  static iterator end() {
+    return end_.FakePrev();
+  }
+
   ISLEntry Insert(ISLEntry e) {
     e.SetNext(start_);
     start_ = e;
@@ -286,7 +297,7 @@ class IntrusiveStringList {
   }
 
   bool Empty() {
-    return start_;
+    return !start_;
   }
 
   // TODO consider to wrap ISLEntry to prevent usage out of the list
@@ -295,7 +306,7 @@ class IntrusiveStringList {
   }
 
   // TODO consider to wrap ISLEntry to prevent usage out of the list
-  IntrusiveStringList::Iterator Find(std::string_view str) {
+  IntrusiveStringList::iterator Find(std::string_view str) {
     auto it = begin();
     for (; it && it->Key() != str; ++it)
       ;
@@ -350,6 +361,7 @@ class IntrusiveStringList {
 
  private:
   ISLEntry start_;
+  static ISLEntry end_;
 };
 
 }  // namespace dfly

src/core/intrusive_string_set.h

@@ -24,13 +24,16 @@ class IntrusiveStringSet {
    public:
     using iterator_category = std::forward_iterator_tag;
     using difference_type = std::ptrdiff_t;
-    using value_type = std::string_view;
+    using value_type = ISLEntry;
-    using pointer = std::string_view*;
+    using pointer = ISLEntry*;
-    using reference = std::string_view&;
+    using reference = ISLEntry&;
 
-    iterator(Buckets::iterator it,
+    iterator(Buckets::iterator it, Buckets::iterator end, IntrusiveStringList::iterator entry)
-             IntrusiveStringList::Iterator entry = IntrusiveStringList::Iterator())
+        : buckets_it_(it), end_(end), entry_(entry) {
-        : buckets_it_(it), entry_(entry) {
+    }
+
+    iterator(Buckets::iterator it, Buckets::iterator end) : buckets_it_(it), end_(end) {
+      SetEntryIt();
     }
 
     // uint32_t ExpiryTime() const {
@@ -47,29 +50,56 @@ class IntrusiveStringSet {
 
     // void Advance();
 
+    iterator& operator++() {
+      // TODO add expiration logic
+      if (entry_)
+        ++entry_;
+      if (!entry_) {
+        ++buckets_it_;
+        SetEntryIt();
+      }
+      return *this;
+    }
+
     bool operator==(const iterator& r) const {
-      return buckets_it_ == r.buckets_it_;
+      return buckets_it_ == r.buckets_it_ && entry_ == r.entry_;
     }
 
     bool operator!=(const iterator& r) const {
       return !operator==(r);
     }
 
-    IntrusiveStringList::Iterator::value_type operator*() {
+    IntrusiveStringList::iterator::value_type operator*() {
       return *entry_;
     }
 
-    IntrusiveStringList::Iterator operator->() {
+    IntrusiveStringList::iterator operator->() {
       return entry_;
     }
 
+   private:
+    // find valid entry_ iterator starting from buckets_it_ and set it
+    void SetEntryIt() {
+      for (; buckets_it_ != end_; ++buckets_it_) {
+        if (!buckets_it_->Empty()) {
+          entry_ = buckets_it_->begin();
+          break;
+        }
+      }
+    }
+
    private:
     Buckets::iterator buckets_it_;
-    IntrusiveStringList::Iterator entry_;
+    Buckets::iterator end_;
+    IntrusiveStringList::iterator entry_;
   };
 
+  iterator begin() {
+    return iterator(entries_.begin(), entries_.end());
+  }
+
   iterator end() {
-    return iterator(entries_.end());
+    return iterator(entries_.end(), entries_.end());
   }
 
   explicit IntrusiveStringSet(PMR_NS::memory_resource* mr = PMR_NS::get_default_resource())
@@ -182,11 +212,11 @@ class IntrusiveStringSet {
 
   iterator Find(std::string_view member) {
     if (entries_.empty())
-      return iterator(entries_.end());
+      return end();
     auto bucket_id = BucketId(Hash(member));
     auto entry_it = entries_.begin() + bucket_id;
     auto res = entry_it->Find(member);
-    return iterator(res ? entry_it : entries_.end(), res);
+    return iterator(res ? entry_it : entries_.end(), entries_.end(), res);
   }
 
   bool Contains(std::string_view member) {

src/core/intrusive_string_set_test.cc

@@ -75,6 +75,18 @@ class IntrusiveStringSetTest : public ::testing::Test {
   mt19937 generator_;
 };
 
+static string random_string(mt19937& rand, unsigned len) {
+  const string_view alpanum = "1234567890abcdefghijklmnopqrstuvwxyz";
+  string ret;
+  ret.reserve(len);
+
+  for (size_t i = 0; i < len; ++i) {
+    ret += alpanum[rand() % alpanum.size()];
+  }
+
+  return ret;
+}
+
 TEST_F(IntrusiveStringSetTest, IntrusiveStringListTest) {
   IntrusiveStringList isl;
   ISLEntry test = isl.Emplace("0123456789");
@@ -208,18 +220,6 @@ TEST_F(IntrusiveStringSetTest, DisplacedBug) {
   ss_->Add("HPq");
 }
 
-static string random_string(mt19937& rand, unsigned len) {
-  const string_view alpanum = "1234567890abcdefghijklmnopqrstuvwxyz";
-  string ret;
-  ret.reserve(len);
-
-  for (size_t i = 0; i < len; ++i) {
-    ret += alpanum[rand() % alpanum.size()];
-  }
-
-  return ret;
-}
-
 TEST_F(IntrusiveStringSetTest, Resizing) {
   constexpr size_t num_strs = 4096;
   unordered_set<string> strs;
@@ -434,33 +434,33 @@ TEST_F(IntrusiveStringSetTest, Pop) {
   DCHECK(to_insert.empty());
 }
 
-// TEST_F(IntrusiveStringSetTest, Iteration) {
+TEST_F(IntrusiveStringSetTest, Iteration) {
-//   ss_->Add("foo");
+  ss_->Add("foo");
-//   for (const sds ptr : *ss_) {
+  for (const auto& ptr : *ss_) {
-//     LOG(INFO) << ptr;
+    LOG(INFO) << ptr;
-//   }
+  }
-//   ss_->Clear();
+  ss_->Clear();
-//   constexpr size_t num_items = 8192;
+  constexpr size_t num_items = 8192;
-//   unordered_set<string> to_insert;
+  unordered_set<string> to_insert;
 
-//   while (to_insert.size() != num_items) {
+  while (to_insert.size() != num_items) {
-//     auto str = random_string(generator_, 10);
+    auto str = random_string(generator_, 10);
-//     if (to_insert.count(str)) {
+    if (to_insert.count(str)) {
-//       continue;
+      continue;
-//     }
+    }
 
-//     to_insert.insert(str);
+    to_insert.insert(str);
-//     EXPECT_TRUE(ss_->Add(str));
+    EXPECT_TRUE(ss_->Add(str));
-//   }
+  }
 
-//   for (const sds ptr : *ss_) {
+  for (const auto& ptr : *ss_) {
-//     string str{ptr, sdslen(ptr)};
+    std::string str(ptr.Key());
-//     EXPECT_TRUE(to_insert.count(str));
+    EXPECT_TRUE(to_insert.count(str));
-//     to_insert.erase(str);
+    to_insert.erase(str);
-//   }
+  }
 
-//   EXPECT_EQ(to_insert.size(), 0);
+  EXPECT_EQ(to_insert.size(), 0);
-// }
+}
 
 TEST_F(IntrusiveStringSetTest, SetFieldExpireHasExpiry) {
   EXPECT_TRUE(ss_->Add("k1", 100));