Skip to content

Commit 4714445

Browse files
committed
kv/BinnedLRUCache: do not use autovector to keep entries to be deleted.
Signed-off-by: Garry Drankovich <[email protected]>
1 parent c118715 commit 4714445

File tree

2 files changed

+30
-21
lines changed

2 files changed

+30
-21
lines changed

src/kv/rocksdb_cache/BinnedLRUCache.cc

Lines changed: 21 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -131,7 +131,7 @@ bool BinnedLRUCacheShard::Unref(BinnedLRUHandle* e) {
131131
// Call deleter and free
132132

133133
void BinnedLRUCacheShard::EraseUnRefEntries() {
134-
ceph::autovector<BinnedLRUHandle*> last_reference_list;
134+
BinnedLRUHandle* deleted = nullptr;
135135
{
136136
std::lock_guard<std::mutex> l(mutex_);
137137
while (lru_.next != &lru_) {
@@ -144,13 +144,13 @@ void BinnedLRUCacheShard::EraseUnRefEntries() {
144144
old->SetInCache(false);
145145
Unref(old);
146146
usage_ -= old->charge;
147-
last_reference_list.push_back(old);
147+
ceph_assert(!old->next);
148+
old->next = deleted;
149+
deleted = old;
148150
}
149151
}
150152

151-
for (auto entry : last_reference_list) {
152-
entry->Free();
153-
}
153+
FreeDeleted(deleted);
154154
}
155155

156156
void BinnedLRUCacheShard::ApplyToAllCacheEntries(
@@ -270,7 +270,7 @@ void BinnedLRUCacheShard::MaintainPoolSize() {
270270
}
271271

272272
void BinnedLRUCacheShard::EvictFromLRU(size_t charge,
273-
ceph::autovector<BinnedLRUHandle*>* deleted) {
273+
BinnedLRUHandle*& deleted) {
274274
while (usage_ + charge > capacity_ && lru_.next != &lru_) {
275275
BinnedLRUHandle* old = lru_.next;
276276
ceph_assert(old->InCache());
@@ -280,23 +280,23 @@ void BinnedLRUCacheShard::EvictFromLRU(size_t charge,
280280
old->SetInCache(false);
281281
Unref(old);
282282
usage_ -= old->charge;
283-
deleted->push_back(old);
283+
ceph_assert(!old->next);
284+
old->next = deleted;
285+
deleted = old;
284286
}
285287
}
286288

287289
void BinnedLRUCacheShard::SetCapacity(size_t capacity) {
288-
ceph::autovector<BinnedLRUHandle*> last_reference_list;
290+
BinnedLRUHandle* deleted = nullptr;
289291
{
290292
std::lock_guard<std::mutex> l(mutex_);
291293
capacity_ = capacity;
292294
high_pri_pool_capacity_ = capacity_ * high_pri_pool_ratio_;
293-
EvictFromLRU(0, &last_reference_list);
295+
EvictFromLRU(0, deleted);
294296
}
295297
// we free the entries here outside of mutex for
296298
// performance reasons
297-
for (auto entry : last_reference_list) {
298-
entry->Free();
299-
}
299+
FreeDeleted(deleted);
300300
}
301301

302302
void BinnedLRUCacheShard::SetStrictCapacityLimit(bool strict_capacity_limit) {
@@ -379,7 +379,7 @@ rocksdb::Status BinnedLRUCacheShard::Insert(const rocksdb::Slice& key, uint32_t
379379
rocksdb::Cache::Handle** handle, rocksdb::Cache::Priority priority) {
380380
auto e = new BinnedLRUHandle();
381381
rocksdb::Status s;
382-
ceph::autovector<BinnedLRUHandle*> last_reference_list;
382+
BinnedLRUHandle* deleted = nullptr;
383383

384384
e->value = value;
385385
e->deleter = deleter;
@@ -400,14 +400,16 @@ rocksdb::Status BinnedLRUCacheShard::Insert(const rocksdb::Slice& key, uint32_t
400400
std::lock_guard<std::mutex> l(mutex_);
401401
// Free the space following strict LRU policy until enough space
402402
// is freed or the lru list is empty
403-
EvictFromLRU(charge, &last_reference_list);
403+
EvictFromLRU(charge, deleted);
404404

405405
if (usage_ - lru_usage_ + charge > capacity_ &&
406406
(strict_capacity_limit_ || handle == nullptr)) {
407407
if (handle == nullptr) {
408408
// Don't insert the entry but still return ok, as if the entry inserted
409409
// into cache and get evicted immediately.
410-
last_reference_list.push_back(e);
410+
ceph_assert(!e->next);
411+
e->next = deleted;
412+
deleted = e;
411413
} else {
412414
delete e;
413415
*handle = nullptr;
@@ -426,7 +428,9 @@ rocksdb::Status BinnedLRUCacheShard::Insert(const rocksdb::Slice& key, uint32_t
426428
// old is on LRU because it's in cache and its reference count
427429
// was just 1 (Unref returned 0)
428430
LRU_Remove(old);
429-
last_reference_list.push_back(old);
431+
ceph_assert(!old->next);
432+
old->next = deleted;
433+
deleted = old;
430434
}
431435
}
432436
if (handle == nullptr) {
@@ -440,9 +444,7 @@ rocksdb::Status BinnedLRUCacheShard::Insert(const rocksdb::Slice& key, uint32_t
440444

441445
// we free the entries here outside of mutex for
442446
// performance reasons
443-
for (auto entry : last_reference_list) {
444-
entry->Free();
445-
}
447+
FreeDeleted(deleted);
446448

447449
return s;
448450
}

src/kv/rocksdb_cache/BinnedLRUCache.h

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,6 @@
1515
#include <boost/circular_buffer.hpp>
1616

1717
#include "ShardedCache.h"
18-
#include "common/autovector.h"
1918
#include "common/dout.h"
2019
#include "include/ceph_assert.h"
2120
#include "common/ceph_context.h"
@@ -261,7 +260,15 @@ class alignas(CACHE_LINE_SIZE) BinnedLRUCacheShard : public CacheShard {
261260
// to hold (usage_ + charge) is freed or the lru list is empty
262261
// This function is not thread safe - it needs to be executed while
263262
// holding the mutex_
264-
void EvictFromLRU(size_t charge, ceph::autovector<BinnedLRUHandle*>* deleted);
263+
void EvictFromLRU(size_t charge, BinnedLRUHandle*& deleted);
264+
265+
void FreeDeleted(BinnedLRUHandle* deleted) {
266+
while (deleted) {
267+
auto* entry = deleted;
268+
deleted = deleted->next;
269+
entry->Free();
270+
}
271+
}
265272

266273
// Initialized before use.
267274
size_t capacity_;

0 commit comments

Comments
 (0)