Skip to content

Commit 96a123c

Browse files
mkaruzaBorysTheDev
mkaruza
authored and
BorysTheDev
committed
feat(intrusive_string_set): Fixed set TTL tests (#5142)
Fix set TTL tests
1 parent fa7894a commit 96a123c

File tree

3 files changed

+40
-24
lines changed

3 files changed

+40
-24
lines changed

‎src/core/intrusive_string_list.h‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -262,6 +262,7 @@ class UniqueISLEntry : private ISLEntry {
262262

263263
class IntrusiveStringList {
264264
public:
265+
size_t obj_malloc_used_; // TODO: think how we can keep track of size
265266
class iterator {
266267
public:
267268
using iterator_category = std::forward_iterator_tag;
@@ -422,25 +423,27 @@ class IntrusiveStringList {
422423
return entry;
423424
}
424425

425-
bool Erase(std::string_view str) {
426+
bool Erase(std::string_view str, uint32_t* set_size) {
426427
auto cond = [str](const ISLEntry e) { return str == e.Key(); };
427-
return Erase(cond);
428+
return Erase(cond, set_size);
428429
}
429430

430431
template <class T, std::enable_if_t<std::is_invocable_v<T, ISLEntry>>* = nullptr>
431-
bool Erase(const T& cond) {
432+
bool Erase(const T& cond, uint32_t* set_size) {
432433
if (!start_) {
433434
return false;
434435
}
435436

436437
if (auto it = start_; cond(it)) {
438+
(*set_size)--;
437439
start_ = it.Next();
438440
ISLEntry::Destroy(it);
439441
return true;
440442
}
441443

442444
for (auto prev = start_, it = start_.Next(); it; prev = it, it = it.Next()) {
443445
if (cond(it)) {
446+
(*set_size)--;
444447
prev.SetNext(it.Next());
445448
ISLEntry::Destroy(it);
446449
return true;
@@ -493,7 +496,6 @@ class IntrusiveStringList {
493496
}
494497

495498
private:
496-
size_t obj_malloc_used_;
497499
ISLEntry start_;
498500
static ISLEntry end_;
499501
};

‎src/core/intrusive_string_set.h‎

Lines changed: 26 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,9 @@ class IntrusiveStringSet {
3535
: buckets_it_(it), end_(end), entry_(entry) {
3636
}
3737

38-
iterator(Buckets::iterator it, Buckets::iterator end) : buckets_it_(it), end_(end) {
38+
iterator(Buckets::iterator it, Buckets::iterator end, std::uint32_t time_now,
39+
std::uint32_t* set_size)
40+
: buckets_it_(it), end_(end), time_now_(time_now), set_size_(set_size) {
3941
SetEntryIt();
4042
}
4143

@@ -54,9 +56,11 @@ class IntrusiveStringSet {
5456
// void Advance();
5557

5658
iterator& operator++() {
57-
// TODO add expiration logic
58-
if (entry_)
59+
if (entry_) {
60+
if (entry_.ExpireIfNeeded(time_now_, &buckets_it_->obj_malloc_used_))
61+
(*set_size_)--;
5962
++entry_;
63+
}
6064
if (!entry_) {
6165
++buckets_it_;
6266
SetEntryIt();
@@ -95,14 +99,16 @@ class IntrusiveStringSet {
9599
Buckets::iterator buckets_it_;
96100
Buckets::iterator end_;
97101
IntrusiveStringList::iterator entry_;
102+
std::uint32_t time_now_;
103+
std::uint32_t* set_size_;
98104
};
99105

100106
iterator begin() {
101-
return iterator(entries_.begin(), entries_.end());
107+
return iterator(entries_.begin(), entries_.end(), time_now_, &size_);
102108
}
103109

104110
iterator end() {
105-
return iterator(entries_.end(), entries_.end());
111+
return iterator(entries_.end(), entries_.end(), time_now_, &size_);
106112
}
107113

108114
explicit IntrusiveStringSet(PMR_NS::memory_resource* mr = PMR_NS::get_default_resource())
@@ -111,14 +117,18 @@ class IntrusiveStringSet {
111117

112118
static constexpr uint32_t kMaxBatchLen = 32;
113119

114-
ISLEntry Add(std::string_view str, uint32_t ttl_sec = UINT32_MAX) {
120+
ISLEntry Add(std::string_view str, bool keepttl = true, uint32_t ttl_sec = UINT32_MAX) {
115121
if (entries_.empty() || size_ >= entries_.size()) {
116122
Reserve(Capacity() * 2);
117123
}
118124
uint64_t hash = Hash(str);
119125
const auto bucket_id = BucketId(hash);
120126

121127
if (auto item = FindInternal(bucket_id, str, hash); item.first != IntrusiveStringList::end()) {
128+
// Update ttl if found
129+
if (!keepttl) {
130+
item.first.SetExpiryTime(EntryTTL(ttl_sec), &entries_[item.second].obj_malloc_used_);
131+
}
122132
return {};
123133
}
124134

@@ -160,8 +170,9 @@ class IntrusiveStringSet {
160170
Reserve(span.size());
161171
unsigned res = 0;
162172
for (auto& s : span) {
163-
res++;
164-
Add(s, ttl_sec);
173+
if (Add(s, keepttl, ttl_sec)) {
174+
res++;
175+
}
165176
}
166177
return res;
167178
}
@@ -173,7 +184,7 @@ class IntrusiveStringSet {
173184
other->set_time(time_now());
174185
for (uint32_t bucket_id = 0; bucket_id < entries_.size(); ++bucket_id) {
175186
for (auto it = entries_[bucket_id].begin(); it != entries_[bucket_id].end(); ++it) {
176-
other->Add(it->Key(), it.HasExpiry() ? it->ExpiryTime() : UINT32_MAX);
187+
other->Add(it->Key(), true, it.HasExpiry() ? it->ExpiryTime() : UINT32_MAX);
177188
}
178189
}
179190
}
@@ -229,7 +240,7 @@ class IntrusiveStringSet {
229240
uint64_t hash = Hash(str);
230241
auto bucket_id = BucketId(hash);
231242
auto item = FindInternal(bucket_id, str, hash);
232-
return entries_[item.second].Erase(str);
243+
return entries_[item.second].Erase(str, &size_);
233244
}
234245

235246
iterator Find(std::string_view member) {
@@ -239,7 +250,11 @@ class IntrusiveStringSet {
239250
uint64_t hash = Hash(member);
240251
auto bucket_id = BucketId(hash);
241252
auto res = FindInternal(bucket_id, member, hash);
242-
return {res.first ? entries_.begin() + res.second : entries_.end(), entries_.end(), res.first};
253+
return {
254+
res.first ? entries_.begin() + res.second : entries_.end(),
255+
entries_.end(),
256+
res.first,
257+
};
243258
}
244259

245260
bool Contains(std::string_view member) {
@@ -369,7 +384,6 @@ class IntrusiveStringSet {
369384
std::uint32_t capacity_log_ = 0;
370385
std::uint32_t size_ = 0; // number of elements in the set.
371386
std::uint32_t time_now_ = 0;
372-
373387
Buckets entries_;
374388
};
375389

‎src/core/intrusive_string_set_test.cc‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -108,9 +108,9 @@ TEST_F(IntrusiveStringSetTest, IntrusiveStringListTest) {
108108
EXPECT_EQ(isl.Find("123456789", &num_expired_fields)->Key(), "123456789"sv);
109109
EXPECT_FALSE(isl.Find("test", &num_expired_fields));
110110

111-
EXPECT_TRUE(isl.Erase("23456789"));
111+
EXPECT_TRUE(isl.Erase("23456789", &num_expired_fields));
112112
EXPECT_FALSE(isl.Find("23456789", &num_expired_fields));
113-
EXPECT_FALSE(isl.Erase("test"));
113+
EXPECT_FALSE(isl.Erase("test", &num_expired_fields));
114114
EXPECT_FALSE(isl.Find("test", &num_expired_fields));
115115
}
116116

@@ -273,7 +273,7 @@ TEST_F(IntrusiveStringSetTest, Resizing) {
273273
unsigned size = 0;
274274
for (auto it = strs.begin(); it != strs.end(); ++it) {
275275
const auto& str = *it;
276-
EXPECT_TRUE(ss_->Add(str, 1));
276+
EXPECT_TRUE(ss_->Add(str, true, 1));
277277
EXPECT_EQ(ss_->UpperBoundSize(), size + 1);
278278

279279
// make sure we haven't lost any items after a grow
@@ -505,7 +505,7 @@ TEST_F(IntrusiveStringSetTest, Iteration) {
505505
}
506506

507507
TEST_F(IntrusiveStringSetTest, SetFieldExpireHasExpiry) {
508-
EXPECT_TRUE(ss_->Add("k1", 100));
508+
EXPECT_TRUE(ss_->Add("k1", true, 100));
509509
auto k = ss_->Find("k1");
510510
EXPECT_TRUE(k->HasExpiry());
511511
EXPECT_EQ(k->ExpiryTime(), 100);
@@ -526,17 +526,17 @@ TEST_F(IntrusiveStringSetTest, SetFieldExpireNoHasExpiry) {
526526
}
527527

528528
TEST_F(IntrusiveStringSetTest, Ttl) {
529-
EXPECT_TRUE(ss_->Add("bla"sv, 1));
530-
EXPECT_FALSE(ss_->Add("bla"sv, 1));
529+
EXPECT_TRUE(ss_->Add("bla"sv, true, 1));
530+
EXPECT_FALSE(ss_->Add("bla"sv, true, 1));
531531
auto it = ss_->Find("bla"sv);
532532
EXPECT_EQ(1u, it->ExpiryTime());
533533

534534
ss_->set_time(1);
535-
EXPECT_TRUE(ss_->Add("bla"sv, 1));
535+
EXPECT_TRUE(ss_->Add("bla"sv, true, 1));
536536
EXPECT_EQ(1u, ss_->UpperBoundSize());
537537

538538
for (unsigned i = 0; i < 100; ++i) {
539-
EXPECT_TRUE(ss_->Add(absl::StrCat("foo", i), 1));
539+
EXPECT_TRUE(ss_->Add(absl::StrCat("foo", i), true, 1));
540540
}
541541
EXPECT_EQ(101u, ss_->UpperBoundSize());
542542
it = ss_->Find("foo50");

0 commit comments

Comments
 (0)