refactor: improve performance

Author: BorysTheDev

src/core/intrusive_string_list.h

@@ -92,7 +92,8 @@ class ISLEntry {
     e.data_ = nullptr;
   }
 
-  ~ISLEntry() {
+  // consider manual removing, we waste a lot of time to check nullptr
+  inline ~ISLEntry() {
     Clear();
   }
 
@@ -102,33 +103,33 @@ class ISLEntry {
     return *this;
   }
 
-  bool Empty() const {
+  inline bool Empty() const {
     return !Raw();
   }
 
-  operator bool() const {
+  inline operator bool() const {
     return !Empty();
   }
 
-  bool IsVector() const {
+  inline bool IsVector() const {
     return (uptr() & kVectorBit) != 0;
   }
 
-  std::vector<ISLEntry>& AsVector() {
+  inline std::vector<ISLEntry>& AsVector() {
     return *reinterpret_cast<std::vector<ISLEntry>*>(Raw());
   }
 
-  std::string_view Key() const {
+  inline std::string_view Key() const {
     DCHECK(!IsVector());
     return {GetKeyData(), GetKeySize()};
   }
 
-  bool HasExpiry() const {
+  inline bool HasExpiry() const {
     return (uptr() & kExpiryBit) != 0;
   }
 
   // returns the expiry time of the current entry or UINT32_MAX if no expiry is set.
-  uint32_t GetExpiry() const {
+  inline uint32_t GetExpiry() const {
     std::uint32_t res = UINT32_MAX;
     if (HasExpiry()) {
       DCHECK(!IsVector());
@@ -142,7 +143,7 @@ class ISLEntry {
     return this;
   }
 
-  uint64_t GetExtendedHash() const {
+  inline uint64_t GetExtendedHash() const {
     return (uptr() & kExtHashShiftedMask) >> kExtHashShift;
   }
 
@@ -265,7 +266,7 @@ class ISLEntry {
   }
 
   // TODO refactor, because it's inefficient
-  uint32_t Insert(ISLEntry&& e) {
+  inline uint32_t Insert(ISLEntry&& e) {
     if (Empty()) {
       *this = std::move(e);
       return 0;
@@ -298,7 +299,8 @@ class ISLEntry {
     return AsVector().size();
   }
 
-  ISLEntry& operator[](uint32_t pos) {
+  // TODO remove, it is inefficient
+  inline ISLEntry& operator[](uint32_t pos) {
     DCHECK(!Empty());
     if (!IsVector()) {
       DCHECK(pos == 0);
@@ -358,7 +360,11 @@ class ISLEntry {
   }
 
  protected:
-  void Clear() {
+  inline void Clear() {
+    // TODO add optimization to avoid destructor calls during vector allocator
+    if (!data_)
+      return;
+
     if (IsVector()) {
       delete &AsVector();
     } else {
@@ -383,70 +389,45 @@ class ISLEntry {
     return size;
   }
 
-  uint64_t uptr() const {
+  inline uint64_t uptr() const {
     return uint64_t(data_);
   }
 
-  char* Raw() const {
+  inline char* Raw() const {
     return (char*)(uptr() & ~kTagMask);
   }
 
-  void SetExpiryBit(bool b) {
+  inline void SetExpiryBit(bool b) {
     if (b)
       data_ = (char*)(uptr() | kExpiryBit);
     else
       data_ = (char*)(uptr() & (~kExpiryBit));
   }
 
-  void SetVectorBit() {
+  inline void SetVectorBit() {
     data_ = (char*)(uptr() | kVectorBit);
   }
 
-  void SetSsoBit() {
+  inline void SetSsoBit() {
     data_ = (char*)(uptr() | kSsoBit);
   }
 
-  bool HasSso() const {
+  inline bool HasSso() const {
     return (uptr() & kSsoBit) != 0;
   }
 
-  size_t Size() {
+  inline size_t Size() {
     size_t key_field_size = HasSso() ? 1 : 4;
     size_t expiry_field_size = HasExpiry() ? 4 : 0;
     return expiry_field_size + key_field_size + GetKeySize();
   }
-  std::uint32_t GetExpirySize() const {
+
+  inline std::uint32_t GetExpirySize() const {
     return HasExpiry() ? sizeof(std::uint32_t) : 0;
   }
 
-  // TODO add optimization for big keys
   // memory daya layout [Expiry, key_size, key]
   char* data_ = nullptr;
 };
 
-// template <class T, std::enable_if_t<std::is_invocable_v<T, std::string_view>>* = nullptr>
-// bool Scan(const T& cb, uint32_t* set_size, uint32_t curr_time) {
-//   for (auto it = start_; it && it.ExpiryTime() <= curr_time; it = start_) {
-//     start_ = it.Next();
-//     ISLEntry::Destroy(it);
-//     (*set_size)--;
-//   }
-
-//   for (auto curr = start_, next = start_; curr; curr = next) {
-//     cb(curr.Key());
-//     next = curr.Next();
-//     for (auto tmp = next; tmp && tmp.ExpiryTime() <= curr_time; tmp = next) {
-//       (*set_size)--;
-//       next = next.Next();
-//       ISLEntry::Destroy(tmp);
-//     }
-//     curr.SetNext(next);
-//   }
-//   return start_;
-// }
-
-// size_t ObjMallocUsed() const {
-//   return obj_malloc_used_;
-// };
-
 }  // namespace dfly

src/core/intrusive_string_set.h

@@ -321,16 +321,17 @@ class IntrusiveStringSet {
     auto prev_size = 1 << prev_capacity_log;
     for (int64_t bucket_id = prev_size - 1; bucket_id >= 0; --bucket_id) {
       auto bucket = std::move(entries_[bucket_id]);
+      // TODO add optimization for package processing
       for (uint32_t pos = 0, size = bucket.ElementsNum(); pos < size; ++pos) {
+        // TODO operator [] is inefficient and it is better to avoid it
         if (bucket[pos]) {
           auto new_bucket_id =
               bucket[pos].Rehash(bucket_id, prev_capacity_log, capacity_log_, kShiftLog);
-          // TODO remove
-          // auto real_bucket_id = BucketId(Hash(bucket[i].Key()), capacity_log_);
-          // DCHECK(real_bucket_id == bucket_id);
 
+          // TODO add optimization for package processing
           new_bucket_id = FindEmptyAround(new_bucket_id);
 
+          // insert method is inefficient
           entries_[new_bucket_id]->Insert(std::move(bucket[pos]));
         }
       }

src/core/intrusive_string_set_test.cc

@@ -607,12 +607,12 @@ TEST_F(IntrusiveStringSetTest, Fill) {
   }
 }
 
-// TEST_F(IntrusiveStringSetTest, IterateEmpty) {
-//   for (const auto& s : *ss_) {
-//     // We're iterating to make sure there is no crash. However, if we got here, it's a bug
-//     CHECK(false) << "Found entry " << s << " in empty set";
-//   }
-// }
+TEST_F(IntrusiveStringSetTest, IterateEmpty) {
+  for (const auto& s : *ss_) {
+    // We're iterating to make sure there is no crash. However, if we got here, it's a bug
+    CHECK(false) << "Found entry " << s << " in empty set";
+  }
+}
 
 // size_t memUsed(IntrusiveStringSet& obj) {
 //   return obj.ObjMallocUsed() + obj.SetMallocUsed();
@@ -778,33 +778,33 @@ TEST_F(IntrusiveStringSetTest, Fill) {
 //     ->ArgNames({"elements", "Key Size"})
 //     ->ArgsProduct({{1000, 10000, 100000}, {10, 100, 1000}});
 
-// void BM_Grow(benchmark::State& state) {
-//   vector<string> strs;
-//   mt19937 generator(0);
-//   IntrusiveStringSet src;
-//   unsigned elems = 1 << 18;
-//   for (size_t i = 0; i < elems; ++i) {
-//     src.Add(random_string(generator, 16), UINT32_MAX);
-//     strs.push_back(random_string(generator, 16));
-//   }
-
-//   while (state.KeepRunning()) {
-//     state.PauseTiming();
-//     IntrusiveStringSet tmp;
-//     src.Fill(&tmp);
-//     CHECK_EQ(tmp.Capacity(), elems);
-//     state.ResumeTiming();
-//     for (const auto& str : strs) {
-//       tmp.Add(str);
-//       if (tmp.Capacity() > elems) {
-//         break;  // we grew
-//       }
-//     }
+void BM_Grow(benchmark::State& state) {
+  vector<string> strs;
+  mt19937 generator(0);
+  IntrusiveStringSet src;
+  unsigned elems = 1 << 18;
+  for (size_t i = 0; i < elems; ++i) {
+    src.Add(random_string(generator, 16), UINT32_MAX);
+    strs.push_back(random_string(generator, 16));
+  }
+
+  while (state.KeepRunning()) {
+    state.PauseTiming();
+    IntrusiveStringSet tmp;
+    src.Fill(&tmp);
+    CHECK_EQ(tmp.Capacity(), elems);
+    state.ResumeTiming();
+    for (const auto& str : strs) {
+      tmp.Add(str);
+      if (tmp.Capacity() > elems) {
+        break;  // we grew
+      }
+    }
 
-//     CHECK_GT(tmp.Capacity(), elems);
-//   }
-// }
-// BENCHMARK(BM_Grow);
+    CHECK_GT(tmp.Capacity(), elems);
+  }
+}
+BENCHMARK(BM_Grow);
 
 // unsigned total_wasted_memory = 0;