Skip to content

Commit 6edff0d

Browse files
committed
formatting and fixes
1 parent a0c39d8 commit 6edff0d

File tree

5 files changed

+70
-73
lines changed

5 files changed

+70
-73
lines changed

libraries/multi-index-lru/include/userver/multi-index-lru/container.hpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -101,12 +101,12 @@ class Container {
101101
}
102102

103103
template <typename Tag>
104-
auto& get() {
104+
auto& get_index() {
105105
return container_.template get<Tag>();
106106
}
107107

108108
template <typename Tag>
109-
const auto& get() const {
109+
const auto& get_index() const {
110110
return container_.template get<Tag>();
111111
}
112112

libraries/multi-index-lru/include/userver/multi-index-lru/expirable_container.hpp

Lines changed: 31 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
#include <cassert>
88
#include <shared_mutex>
99
#include <mutex>
10+
#include <optional>
1011

1112
#include "impl/mpl_helpers.hpp"
1213
#include "container.hpp"
@@ -57,30 +58,24 @@ class ExpirableContainer {
5758
return result;
5859
}
5960

61+
bool insert(const Value& value) { return emplace(value).second; }
62+
63+
bool insert(Value&& value) { return emplace(std::move(value)).second; }
64+
6065
template <typename Tag, typename Key>
61-
auto find(const Key& key) {
66+
std::optional<Value> get(const Key& key) {
6267
std::lock_guard<userver::engine::SharedMutex> lock(mutex_);
63-
auto it = container_.template find<Tag, Key>(key);
64-
65-
if (it != container_.template end<Tag>()) {
66-
if (std::chrono::steady_clock::now() > it->last_accessed + ttl_) {
67-
container_.template get<Tag>().erase(it);
68-
return impl::TimestampedIteratorWrapper{container_.template end<Tag>()};
69-
}
70-
71-
it->last_accessed = std::chrono::steady_clock::now();
68+
auto it = find<Tag, Key>(lock, key);
69+
if (it == end<Tag>()) {
70+
return std::nullopt;
7271
}
73-
74-
return impl::TimestampedIteratorWrapper{it};
72+
return *it;
7573
}
7674

77-
bool insert(const Value& value) { return emplace(value).second; }
78-
79-
bool insert(Value&& value) { return emplace(std::move(value)).second; }
80-
8175
template <typename Tag, typename Key>
8276
bool contains(const Key& key) {
83-
return this->template find<Tag, Key>(key) != container_.template end<Tag>();
77+
std::lock_guard<userver::engine::SharedMutex> lock(mutex_);
78+
return this->template find<Tag, Key>(lock, key) != container_.template end<Tag>();
8479
}
8580

8681
template <typename Tag, typename Key>
@@ -119,11 +114,27 @@ class ExpirableContainer {
119114
using ExtendedIndexSpecifierList = impl::add_index_t<
120115
boost::multi_index::sequenced<>,
121116
IndexSpecifierList>;
122-
using BoostContainer = Container<CacheItem, IndexSpecifierList, Allocator>;
117+
using CacheContainer = Container<CacheItem, IndexSpecifierList, Allocator>;
118+
119+
template <typename Tag, typename Key>
120+
auto find(std::lock_guard<userver::engine::SharedMutex>&, const Key& key) {
121+
auto it = container_.template find<Tag, Key>(key);
122+
123+
if (it != container_.template end<Tag>()) {
124+
if (std::chrono::steady_clock::now() > it->last_accessed + ttl_) {
125+
container_.template get_index<Tag>().erase(it);
126+
return impl::TimestampedIteratorWrapper{container_.template end<Tag>()};
127+
}
128+
129+
it->last_accessed = std::chrono::steady_clock::now();
130+
}
131+
132+
return impl::TimestampedIteratorWrapper{it};
133+
}
123134

124135
void cleanup() {
125-
std::lock_guard<userver::engine::SharedMutex> lock(mutex_);
126136
auto now = std::chrono::steady_clock::now();
137+
std::lock_guard<userver::engine::SharedMutex> lock(mutex_);
127138

128139
auto& seq_index = container_.get_sequensed();
129140
while(!seq_index.empty()) {
@@ -156,7 +167,7 @@ class ExpirableContainer {
156167
}
157168
}
158169

159-
BoostContainer container_;
170+
CacheContainer container_;
160171
std::chrono::milliseconds ttl_;
161172
std::chrono::milliseconds cleanup_interval_;
162173
mutable userver::engine::SharedMutex mutex_;

libraries/multi-index-lru/src/container_test.cpp.cpp renamed to libraries/multi-index-lru/src/container_test.cpp

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,8 @@
11
#include <userver/multi-index-lru/container.hpp>
2-
#include <userver/multi-index-lru/expirable_container.hpp>
2+
#include <userver/utest/utest.hpp>
33

44
#include <string>
55

6-
#include <gtest/gtest.h>
76
#include <boost/multi_index/hashed_index.hpp>
87
#include <boost/multi_index/member.hpp>
98
#include <boost/multi_index/ordered_index.hpp>
@@ -45,7 +44,7 @@ class LRUUsersTest : public ::testing::Test {
4544
boost::multi_index::member<User, std::string, &User::name>>>>;
4645
};
4746

48-
TEST_F(LRUUsersTest, BasicOperations) {
47+
UTEST_F(LRUUsersTest, BasicOperations) {
4948
UserCache cache(3); // capacity == 3
5049

5150
// Test insertion
@@ -75,7 +74,7 @@ TEST_F(LRUUsersTest, BasicOperations) {
7574
EXPECT_NE(it, cache.end<EmailTag>());
7675
}
7776

78-
TEST_F(LRUUsersTest, LRUEviction) {
77+
UTEST_F(LRUUsersTest, LRUEviction) {
7978
UserCache cache(3);
8079

8180
cache.emplace(User{1, "alice@test.com", "Alice"});
@@ -121,7 +120,7 @@ class ProductsTest : public ::testing::Test {
121120
boost::multi_index::member<Product, std::string, &Product::name>>>>;
122121
};
123122

124-
TEST_F(ProductsTest, BasicProductOperations) {
123+
UTEST_F(ProductsTest, BasicProductOperations) {
125124
ProductCache cache(2);
126125

127126
cache.emplace(Product{"A1", "Laptop", 999.99});
@@ -132,7 +131,7 @@ TEST_F(ProductsTest, BasicProductOperations) {
132131
EXPECT_EQ(laptop->name, "Laptop");
133132
}
134133

135-
TEST_F(ProductsTest, ProductEviction) {
134+
UTEST_F(ProductsTest, ProductEviction) {
136135
ProductCache cache(2);
137136

138137
cache.emplace(Product{"A1", "Laptop", 999.99});

libraries/multi-index-lru/src/expirable_container_benchmark.cpp

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -104,9 +104,9 @@ static void ExpirableFindEmplaceMix(benchmark::State& state) {
104104

105105
for (auto _ : state) {
106106
for (std::size_t i = 0; i < read_ops; ++i) {
107-
cache.find<NameTag, std::string>(names[i]);
108-
cache.find<EmailTag, std::string>(emails[i]);
109-
cache.find<IdTag, int>(ids[i]);
107+
cache.get<NameTag, std::string>(names[i]);
108+
cache.get<EmailTag, std::string>(emails[i]);
109+
cache.get<IdTag, int>(ids[i]);
110110
}
111111

112112
for (std::size_t i = 0; i < write_ops; ++i) {
@@ -155,9 +155,9 @@ static void ExpirableGetOperations(benchmark::State& state) {
155155
state.ResumeTiming();
156156

157157
for (std::size_t i = 0; i < kOperationsNumber; ++i) {
158-
benchmark::DoNotOptimize(cache.find<NameTag, std::string>(names[i]));
159-
benchmark::DoNotOptimize(cache.find<EmailTag, std::string>(emails[i]));
160-
benchmark::DoNotOptimize(cache.find<IdTag, int>(ids[i]));
158+
benchmark::DoNotOptimize(cache.get<NameTag, std::string>(names[i]));
159+
benchmark::DoNotOptimize(cache.get<EmailTag, std::string>(emails[i]));
160+
benchmark::DoNotOptimize(cache.get<IdTag, int>(ids[i]));
161161
}
162162
}
163163

libraries/multi-index-lru/src/expirable_container_test.cpp

Lines changed: 26 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,10 @@
1-
#include <userver/multi-index-lru/container.hpp>
21
#include <userver/multi-index-lru/expirable_container.hpp>
32
#include <userver/utils/async.hpp>
4-
#include <userver/engine/run_standalone.hpp>
53
#include <userver/engine/task/task_with_result.hpp>
4+
#include <userver/utest/utest.hpp>
65

76
#include <string>
87

9-
#include <gtest/gtest.h>
108
#include <boost/multi_index/hashed_index.hpp>
119
#include <boost/multi_index/member.hpp>
1210
#include <boost/multi_index/ordered_index.hpp>
@@ -47,8 +45,7 @@ class ExpirableUsersTest : public ::testing::Test {
4745
boost::multi_index::member<User, std::string, &User::name>>>>;
4846
};
4947

50-
TEST_F(ExpirableUsersTest, BasicOperations) {
51-
userver::engine::RunStandalone([&] {
48+
UTEST_F(ExpirableUsersTest, BasicOperations) {
5249
UserCacheExpirable cache(3, std::chrono::seconds(10)); // capacity=3, TTL=10s
5350

5451
// Test insertion
@@ -60,34 +57,32 @@ TEST_F(ExpirableUsersTest, BasicOperations) {
6057
EXPECT_EQ(cache.capacity(), 3);
6158
EXPECT_FALSE(cache.empty());
6259

63-
// Test find by id
64-
auto by_id = cache.find<IdTag>(1);
65-
ASSERT_NE(by_id, cache.end<IdTag>());
60+
// Test get by id
61+
auto by_id = cache.get<IdTag>(1);
62+
EXPECT_TRUE(by_id.has_value());
6663
EXPECT_EQ(by_id->name, "Alice");
6764

68-
// Test find by email
69-
auto by_email = cache.find<EmailTag>("bob@test.com");
70-
ASSERT_NE(by_email, cache.end<EmailTag>());
65+
// Test get by email
66+
auto by_email = cache.get<EmailTag>("bob@test.com");
67+
EXPECT_TRUE(by_email.has_value());
7168
EXPECT_EQ(by_email->id, 2);
7269

73-
// Test find by name
74-
auto by_name = cache.find<NameTag>("Charlie");
75-
ASSERT_NE(by_name, cache.end<NameTag>());
70+
// Test get by name
71+
auto by_name = cache.get<NameTag>("Charlie");
72+
EXPECT_TRUE(by_name.has_value());
7673
EXPECT_EQ(by_name->email, "charlie@test.com");
77-
});
7874
}
7975

80-
TEST_F(ExpirableUsersTest, LRUEviction) {
81-
userver::engine::RunStandalone([&] {
76+
UTEST_F(ExpirableUsersTest, LRUEviction) {
8277
UserCacheExpirable cache(3, std::chrono::seconds(10));
8378

8479
cache.insert(User{1, "alice@test.com", "Alice"});
8580
cache.insert(User{2, "bob@test.com", "Bob"});
8681
cache.insert(User{3, "charlie@test.com", "Charlie"});
8782

8883
// Access Alice and Charlie to make them recently used
89-
cache.find<IdTag>(1);
90-
cache.find<IdTag>(3);
84+
cache.get<IdTag>(1);
85+
cache.get<IdTag>(3);
9186

9287
// Add fourth element - Bob should be evicted (LRU)
9388
cache.insert(User{4, "david@test.com", "David"});
@@ -97,11 +92,9 @@ TEST_F(ExpirableUsersTest, LRUEviction) {
9792
EXPECT_TRUE(cache.contains<IdTag>(3)); // Charlie remains
9893
EXPECT_TRUE(cache.contains<IdTag>(4)); // David added
9994
EXPECT_EQ(cache.size(), 3);
100-
});
10195
}
10296

103-
TEST_F(ExpirableUsersTest, TTLExpiration) {
104-
userver::engine::RunStandalone([&] {
97+
UTEST_F(ExpirableUsersTest, TTLExpiration) {
10598
using namespace std::chrono_literals;
10699

107100
UserCacheExpirable cache(100, 100ms); // Very short TTL for testing
@@ -120,11 +113,10 @@ TEST_F(ExpirableUsersTest, TTLExpiration) {
120113
EXPECT_FALSE(cache.contains<IdTag>(1));
121114
EXPECT_FALSE(cache.contains<IdTag>(2));
122115
EXPECT_EQ(cache.size(), 0);
123-
});
124116
}
125117

126-
TEST_F(ExpirableUsersTest, TTLRefreshOnAccess) {
127-
userver::engine::RunStandalone([&] {
118+
UTEST_F(ExpirableUsersTest, TTLRefreshOnAccess) {
119+
128120
using namespace std::chrono_literals;
129121

130122
UserCacheExpirable cache(100, 190ms);
@@ -144,11 +136,10 @@ TEST_F(ExpirableUsersTest, TTLRefreshOnAccess) {
144136
// Wait for full TTL from last access
145137
std::this_thread::sleep_for(200ms);
146138
EXPECT_FALSE(cache.contains<IdTag>(1));
147-
});
148139
}
149140

150-
TEST_F(ExpirableUsersTest, EraseOperations) {
151-
userver::engine::RunStandalone([&] {
141+
UTEST_F(ExpirableUsersTest, EraseOperations) {
142+
152143
UserCacheExpirable cache(3, std::chrono::seconds(10));
153144

154145
cache.insert(User{1, "alice@test.com", "Alice"});
@@ -161,11 +152,10 @@ TEST_F(ExpirableUsersTest, EraseOperations) {
161152

162153
EXPECT_FALSE(cache.erase<IdTag>(999)); // Non-existent
163154
EXPECT_EQ(cache.size(), 1);
164-
});
165155
}
166156

167-
TEST_F(ExpirableUsersTest, SetCapacity) {
168-
userver::engine::RunStandalone([&] {
157+
UTEST_F(ExpirableUsersTest, SetCapacity) {
158+
169159
UserCacheExpirable cache(5, std::chrono::seconds(10));
170160

171161
// Fill cache
@@ -181,11 +171,10 @@ TEST_F(ExpirableUsersTest, SetCapacity) {
181171

182172
// Size should be <= new capacity
183173
EXPECT_LE(cache.size(), 3);
184-
});
185174
}
186175

187-
TEST_F(ExpirableUsersTest, Clear) {
188-
userver::engine::RunStandalone([&] {
176+
UTEST_F(ExpirableUsersTest, Clear) {
177+
189178
UserCacheExpirable cache(5, std::chrono::seconds(10));
190179

191180
cache.insert(User{1, "alice@test.com", "Alice"});
@@ -200,11 +189,10 @@ TEST_F(ExpirableUsersTest, Clear) {
200189
EXPECT_TRUE(cache.empty());
201190
EXPECT_FALSE(cache.contains<IdTag>(1));
202191
EXPECT_FALSE(cache.contains<IdTag>(2));
203-
});
204192
}
205193

206-
TEST_F(ExpirableUsersTest, ThreadSafetyBasic) {
207-
userver::engine::RunStandalone([&] {
194+
UTEST_F(ExpirableUsersTest, ThreadSafetyBasic) {
195+
208196
UserCacheExpirable cache(100, std::chrono::seconds(10));
209197

210198
constexpr int kCoroutines = 4;
@@ -220,7 +208,7 @@ TEST_F(ExpirableUsersTest, ThreadSafetyBasic) {
220208
cache.insert(User{id, std::to_string(id) + "@test.com", "User" + std::to_string(id)});
221209

222210
if (id % 3 == 0) {
223-
cache.find<IdTag>(id);
211+
cache.get<IdTag>(id);
224212
cache.contains<IdTag>(id);
225213
}
226214

@@ -236,7 +224,6 @@ TEST_F(ExpirableUsersTest, ThreadSafetyBasic) {
236224
}
237225

238226
EXPECT_LE(cache.size(), 100);
239-
});
240227
}
241228
} // namespace
242229

0 commit comments

Comments
 (0)