Skip to content

Commit 849fbd4

Browse files
dudnykdudnyk
authored
Add cache optimizations (#1330)
Replace substr with memcmp to avoid additional allocations and put it to the IsPrefix function. Put checking memory cache from the first cache in the order for checking Contains to the last one as it usually small in size and and not sufficient to store the entire cache. Add checks expiry validity in case of LRU cache as there is no need for additional checks if the expiry is not valid Check protected_data size in IsProtected if there are elements in ProtectedKeyList. Relates-To: OLPSUP-18424 Signed-off-by: Yevhenii Dudnyk <[email protected]>
1 parent 2f3989e commit 849fbd4

File tree

5 files changed

+73
-44
lines changed

5 files changed

+73
-44
lines changed

‎olp-cpp-sdk-core/src/cache/DefaultCacheImpl.cpp‎

Lines changed: 11 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -434,34 +434,36 @@ bool DefaultCacheImpl::Contains(const std::string& key) const {
434434
return false;
435435
}
436436

437-
if (memory_cache_ && memory_cache_->Contains(key)) {
438-
return true;
437+
if (protected_cache_ && protected_cache_->Contains(key)) {
438+
return GetRemainingExpiryTime(key, *protected_cache_) > 0;
439439
}
440440

441441
// if lru exist check if key is there
442442
if (mutable_cache_lru_) {
443443
auto it = mutable_cache_lru_->FindNoPromote(key);
444444
if (it != mutable_cache_lru_->end()) {
445445
ValueProperties props = it->value();
446+
if (!IsExpiryValid(props.expiry)) {
447+
return true;
448+
}
449+
446450
props.expiry -= olp::cache::InMemoryCache::DefaultTimeProvider()();
447-
return (props.expiry > 0);
451+
return props.expiry > 0;
448452
// if lru exist, but key not found, this case possible only for protected
449453
// keys
454+
450455
} else if (protected_keys_.IsProtected(key)) {
451456
return mutable_cache_ && mutable_cache_->Contains(key);
452457
}
453458

454459
// check in mutable cache only if lru does not exist
455460
} else if (mutable_cache_ && mutable_cache_->Contains(key)) {
456-
return (GetRemainingExpiryTime(key, *mutable_cache_) > 0) ||
461+
return GetRemainingExpiryTime(key, *mutable_cache_) > 0 ||
457462
protected_keys_.IsProtected(key);
458463
}
459464

460-
if (protected_cache_ && protected_cache_->Contains(key)) {
461-
return (GetRemainingExpiryTime(key, *protected_cache_) > 0);
462-
}
463-
464-
return false;
465+
return !protected_cache_ && !mutable_cache_ && memory_cache_ &&
466+
memory_cache_->Contains(key);
465467
}
466468

467469
bool DefaultCacheImpl::AddKeyLru(std::string key, const leveldb::Slice& value) {

‎olp-cpp-sdk-core/src/cache/InMemoryCache.cpp‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,10 @@ namespace {
2525
inline bool HasExpiry(time_t expiry_seconds) {
2626
return (expiry_seconds != InMemoryCache::kExpiryMax);
2727
}
28+
bool IsPrefix(const std::string& key, const std::string& prefix) {
29+
return (key.size() >= prefix.size()) &&
30+
(memcmp(key.data(), prefix.data(), prefix.size()) == 0);
31+
}
2832
} // namespace
2933

3034
InMemoryCache::InMemoryCache(size_t max_size, ModelCacheCostFunc cache_cost,
@@ -98,7 +102,7 @@ void InMemoryCache::RemoveKeysWithPrefix(const std::string& key_prefix,
98102
std::lock_guard<std::mutex> lock{mutex_};
99103

100104
for (auto it = item_tuples_.begin(); it != item_tuples_.end();) {
101-
if (it->key().substr(0, key_prefix.length()) == key_prefix) {
105+
if (IsPrefix(it.key(), key_prefix)) {
102106
// Check if this key is not protected, and if it is do not remove
103107
if (filter && filter(it->key())) {
104108
++it;

‎olp-cpp-sdk-core/src/cache/ProtectedKeyList.cpp‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -147,6 +147,10 @@ bool ProtectedKeyList::Release(const KeyValueCache::KeyListType& keys) {
147147
}
148148

149149
bool ProtectedKeyList::IsProtected(const std::string& key) const {
150+
if (protected_data_.empty()) {
151+
return false;
152+
}
153+
150154
auto it = protected_data_.lower_bound(key);
151155
if (it == protected_data_.end()) {
152156
return false;

‎olp-cpp-sdk-core/tests/cache/DefaultCacheImplTest.cpp‎

Lines changed: 51 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -232,8 +232,9 @@ TEST_F(DefaultCacheImplTest, LruCachePut) {
232232

233233
cache.Open();
234234
cache.Clear();
235-
cache.Put(key, data_string, [=]() { return data_string; },
236-
(std::numeric_limits<time_t>::max)());
235+
cache.Put(
236+
key, data_string, [=]() { return data_string; },
237+
(std::numeric_limits<time_t>::max)());
237238

238239
EXPECT_TRUE(cache.ContainsLru(key));
239240
}
@@ -266,8 +267,9 @@ TEST_F(DefaultCacheImplTest, LruCacheGetPromote) {
266267

267268
cache.Open();
268269
cache.Clear();
269-
cache.Put(key1, data_string, [=]() { return data_string; },
270-
(std::numeric_limits<time_t>::max)());
270+
cache.Put(
271+
key1, data_string, [=]() { return data_string; },
272+
(std::numeric_limits<time_t>::max)());
271273
cache.Put(key2, std::make_shared<std::vector<unsigned char>>(binary_data),
272274
(std::numeric_limits<time_t>::max)());
273275

@@ -334,8 +336,9 @@ TEST_F(DefaultCacheImplTest, LruCacheRemove) {
334336
(std::numeric_limits<time_t>::max)());
335337
cache.Put(key2, std::make_shared<std::vector<unsigned char>>(binary_data),
336338
(std::numeric_limits<time_t>::max)());
337-
cache.Put(key3, data_string, [=]() { return data_string; },
338-
(std::numeric_limits<time_t>::max)());
339+
cache.Put(
340+
key3, data_string, [=]() { return data_string; },
341+
(std::numeric_limits<time_t>::max)());
339342

340343
EXPECT_TRUE(cache.ContainsLru(key1));
341344
EXPECT_TRUE(cache.ContainsLru(key2));
@@ -379,7 +382,8 @@ TEST_F(DefaultCacheImplTest, MutableCacheExpired) {
379382
cache1.Clear();
380383

381384
cache1.Put(key1, data_ptr, expiry);
382-
cache1.Put(key2, data_string, [=]() { return data_string; }, expiry);
385+
cache1.Put(
386+
key2, data_string, [=]() { return data_string; }, expiry);
383387
cache1.Close();
384388

385389
cache::CacheSettings settings2;
@@ -421,7 +425,8 @@ TEST_F(DefaultCacheImplTest, ProtectedCacheExpired) {
421425
cache1.Clear();
422426

423427
cache1.Put(key1, data_ptr, expiry);
424-
cache1.Put(key2, data_string, [=]() { return data_string; }, expiry);
428+
cache1.Put(
429+
key2, data_string, [=]() { return data_string; }, expiry);
425430
cache1.Close();
426431

427432
cache::CacheSettings settings2;
@@ -468,13 +473,15 @@ TEST_F(DefaultCacheImplTest, MutableCacheSize) {
468473
cache.Open();
469474
cache.Clear();
470475

471-
cache.Put(key1, data_string, [=]() { return data_string; },
472-
(std::numeric_limits<time_t>::max)());
476+
cache.Put(
477+
key1, data_string, [=]() { return data_string; },
478+
(std::numeric_limits<time_t>::max)());
473479
auto data_size = key1.size() + data_string.size();
474480

475481
EXPECT_EQ(data_size, cache.Size(CacheType::kMutable));
476482

477-
cache.Put(key2, data_string, [=]() { return data_string; }, expiry);
483+
cache.Put(
484+
key2, data_string, [=]() { return data_string; }, expiry);
478485
data_size +=
479486
key2.size() + data_string.size() + cache.CalculateExpirySize(key2);
480487

@@ -509,8 +516,9 @@ TEST_F(DefaultCacheImplTest, MutableCacheSize) {
509516
cache.Clear();
510517

511518
cache.Put(key1, data_ptr, (std::numeric_limits<time_t>::max)());
512-
cache.Put(key2, data_string, [=]() { return data_string; },
513-
(std::numeric_limits<time_t>::max)());
519+
cache.Put(
520+
key2, data_string, [=]() { return data_string; },
521+
(std::numeric_limits<time_t>::max)());
514522

515523
cache.Remove(key1);
516524
cache.Remove(key2);
@@ -527,7 +535,8 @@ TEST_F(DefaultCacheImplTest, MutableCacheSize) {
527535
cache.Clear();
528536

529537
cache.Put(key1, data_ptr, expiry);
530-
cache.Put(key2, data_string, [=]() { return data_string; }, expiry);
538+
cache.Put(
539+
key2, data_string, [=]() { return data_string; }, expiry);
531540

532541
cache.Remove(key1);
533542
cache.Remove(key2);
@@ -545,7 +554,8 @@ TEST_F(DefaultCacheImplTest, MutableCacheSize) {
545554

546555
cache.Put(key1, data_ptr, (std::numeric_limits<time_t>::max)());
547556
cache.Put(key2, data_ptr, expiry);
548-
cache.Put(key3, data_string, [=]() { return data_string; }, expiry);
557+
cache.Put(
558+
key3, data_string, [=]() { return data_string; }, expiry);
549559
const auto data_size =
550560
key3.size() + data_string.size() + cache.CalculateExpirySize(key3);
551561

@@ -563,7 +573,8 @@ TEST_F(DefaultCacheImplTest, MutableCacheSize) {
563573
cache.Clear();
564574

565575
cache.Put(key1, data_ptr, -1);
566-
cache.Put(key2, data_string, [=]() { return data_string; }, -1);
576+
cache.Put(
577+
key2, data_string, [=]() { return data_string; }, -1);
567578

568579
cache.Get(key1);
569580
cache.Get(key2, [](const std::string& value) { return value; });
@@ -582,8 +593,9 @@ TEST_F(DefaultCacheImplTest, MutableCacheSize) {
582593
cache.Open();
583594
cache.Clear();
584595

585-
cache.Put(key, data_string, [=]() { return data_string; },
586-
(std::numeric_limits<time_t>::max)());
596+
cache.Put(
597+
key, data_string, [=]() { return data_string; },
598+
(std::numeric_limits<time_t>::max)());
587599
cache.Close();
588600
EXPECT_EQ(0u, cache.Size(CacheType::kMutable));
589601

@@ -825,21 +837,24 @@ TEST_F(DefaultCacheImplTest, ProtectTest) {
825837
DefaultCacheImplHelper cache(settings);
826838
ASSERT_EQ(olp::cache::DefaultCache::Success, cache.Open());
827839
ASSERT_TRUE(cache.Clear());
828-
cache.Put(key1, key1_data_string, [=]() { return key1_data_string; }, 2);
829-
cache.Put(other_key1, key1_data_string, [=]() { return key1_data_string; },
830-
2);
840+
cache.Put(
841+
key1, key1_data_string, [=]() { return key1_data_string; }, 2);
842+
cache.Put(
843+
other_key1, key1_data_string, [=]() { return key1_data_string; }, 2);
831844
ASSERT_TRUE(cache.Contains(key1));
832845
// protect single keys, and prefix
833846
ASSERT_TRUE(cache.Protect({key1, key2, "other"}));
834847
// Try to protect key already protected by prefix
835848
ASSERT_FALSE(cache.Protect({other_key1}));
836849
ASSERT_FALSE(cache.Contains(key2));
837-
cache.Put(key2, key2_data_string, [=]() { return key2_data_string; }, 2);
838-
cache.Put(key3, key3_data_string, [=]() { return key3_data_string; }, 2);
839-
cache.Put(other_key2, key2_data_string, [=]() { return key2_data_string; },
840-
2);
841-
cache.Put(other_key3, key3_data_string, [=]() { return key3_data_string; },
842-
2);
850+
cache.Put(
851+
key2, key2_data_string, [=]() { return key2_data_string; }, 2);
852+
cache.Put(
853+
key3, key3_data_string, [=]() { return key3_data_string; }, 2);
854+
cache.Put(
855+
other_key2, key2_data_string, [=]() { return key2_data_string; }, 2);
856+
cache.Put(
857+
other_key3, key3_data_string, [=]() { return key3_data_string; }, 2);
843858
ASSERT_TRUE(cache.Protect({key3}));
844859
ASSERT_TRUE(cache.Release({key1}));
845860
std::this_thread::sleep_for(std::chrono::seconds(3));
@@ -969,8 +984,9 @@ TEST_F(DefaultCacheImplTest, ReadOnlyPartitionForProtectedCache) {
969984
cache1.Open());
970985
cache1.Clear();
971986

972-
cache1.Put(key, data_string, [=]() { return data_string; },
973-
std::numeric_limits<time_t>::max());
987+
cache1.Put(
988+
key, data_string, [=]() { return data_string; },
989+
std::numeric_limits<time_t>::max());
974990
cache1.Close();
975991

976992
// Make readonly
@@ -997,8 +1013,9 @@ TEST_F(DefaultCacheImplTest, ProtectTestWithoutMutableCache) {
9971013
DefaultCacheImplHelper cache({});
9981014
ASSERT_EQ(olp::cache::DefaultCache::Success, cache.Open());
9991015
ASSERT_TRUE(cache.Clear());
1000-
cache.Put(key1, key1_data_string, [=]() { return key1_data_string; },
1001-
std::numeric_limits<time_t>::max());
1016+
cache.Put(
1017+
key1, key1_data_string, [=]() { return key1_data_string; },
1018+
std::numeric_limits<time_t>::max());
10021019
ASSERT_TRUE(cache.Contains(key1));
10031020
ASSERT_FALSE(cache.Protect({key1}));
10041021

@@ -1012,9 +1029,9 @@ TEST_F(DefaultCacheImplTest, ProtectTestWithoutMutableCache) {
10121029
DefaultCacheImplHelper cache(settings);
10131030
ASSERT_EQ(olp::cache::DefaultCache::Success, cache.Open());
10141031
ASSERT_TRUE(cache.Clear());
1015-
ASSERT_TRUE(cache.Put(key1, key1_data_string,
1016-
[=]() { return key1_data_string; },
1017-
std::numeric_limits<time_t>::max()));
1032+
ASSERT_TRUE(cache.Put(
1033+
key1, key1_data_string, [=]() { return key1_data_string; },
1034+
std::numeric_limits<time_t>::max()));
10181035
ASSERT_TRUE(cache.Protect({key1}));
10191036
cache.Close();
10201037
}

‎olp-cpp-sdk-core/tests/cache/ProtectedKeyListTest.cpp‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,9 @@ TEST(ProtectedKeyList, Protect) {
5353
EXPECT_FALSE(protected_keys.IsDirty());
5454

5555
// protect key
56+
EXPECT_FALSE(protected_keys.IsProtected({"key:1"}));
5657
EXPECT_TRUE(protected_keys.Protect({"key:1"}, cb));
58+
EXPECT_TRUE(protected_keys.IsProtected({"key:1"}));
5759
EXPECT_TRUE(protected_keys.IsDirty());
5860
auto raw_data = protected_keys.Serialize();
5961
EXPECT_TRUE(raw_data.get());

0 commit comments

Comments
 (0)