Skip to content

Commit c44e7f9

Browse files
igchorigchor
committed
Accept reference to the item instead of handle in NVMCache::put()
1 parent 3a8d8b4 commit c44e7f9

File tree

5 files changed

+27
-30
lines changed

5 files changed

+27
-30
lines changed

‎cachelib/allocator/CacheAllocator-inl.h

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1461,7 +1461,7 @@ bool CacheAllocator<CacheTrait>::pushToNvmCacheFromRamForTesting(
14611461

14621462
if (handle && nvmCache_ && shouldWriteToNvmCache(*handle) &&
14631463
shouldWriteToNvmCacheExclusive(*handle)) {
1464-
nvmCache_->put(handle, nvmCache_->createPutToken(handle->getKey()));
1464+
nvmCache_->put(*handle, nvmCache_->createPutToken(handle->getKey()));
14651465
return true;
14661466
}
14671467
return false;
@@ -2712,7 +2712,7 @@ CacheAllocator<CacheTrait>::advanceIteratorAndTryEvictRegularItem(
27122712

27132713
if (evictToNvmCache && shouldWriteToNvmCacheExclusive(item)) {
27142714
XDCHECK(token.isValid());
2715-
nvmCache_->put(evictHandle, std::move(token));
2715+
nvmCache_->put(*evictHandle, std::move(token));
27162716
}
27172717
return evictHandle;
27182718
}
@@ -2774,7 +2774,7 @@ CacheAllocator<CacheTrait>::advanceIteratorAndTryEvictChainedItem(
27742774
if (evictToNvmCache && shouldWriteToNvmCacheExclusive(*parentHandle)) {
27752775
XDCHECK(token.isValid());
27762776
XDCHECK(parentHandle->hasChainedItem());
2777-
nvmCache_->put(parentHandle, std::move(token));
2777+
nvmCache_->put(*parentHandle, std::move(token));
27782778
}
27792779

27802780
return parentHandle;
@@ -2812,7 +2812,7 @@ CacheAllocator<CacheTrait>::evictNormalItemForSlabRelease(Item& item) {
28122812
// now that we are the only handle and we actually removed something from
28132813
// the RAM cache, we enqueue it to nvmcache.
28142814
if (evictToNvmCache && shouldWriteToNvmCacheExclusive(item)) {
2815-
nvmCache_->put(handle, std::move(token));
2815+
nvmCache_->put(*handle, std::move(token));
28162816
}
28172817

28182818
return handle;
@@ -2913,7 +2913,7 @@ CacheAllocator<CacheTrait>::evictChainedItemForSlabRelease(ChainedItem& child) {
29132913
// the RAM cache, we enqueue it to nvmcache.
29142914
if (evictToNvmCache && shouldWriteToNvmCacheExclusive(*parentHandle)) {
29152915
DCHECK(parentHandle->hasChainedItem());
2916-
nvmCache_->put(parentHandle, std::move(token));
2916+
nvmCache_->put(*parentHandle, std::move(token));
29172917
}
29182918

29192919
return parentHandle;

‎cachelib/allocator/nvmcache/NvmCache-inl.h

Lines changed: 7 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -460,19 +460,18 @@ uint32_t NvmCache<C>::getStorageSizeInNvm(const Item& it) {
460460
}
461461

462462
template <typename C>
463-
std::unique_ptr<NvmItem> NvmCache<C>::makeNvmItem(const WriteHandle& hdl) {
464-
const auto& item = *hdl;
463+
std::unique_ptr<NvmItem> NvmCache<C>::makeNvmItem(const Item& item) {
465464
auto poolId = cache_.getAllocInfo((void*)(&item)).poolId;
466465

467466
if (item.isChainedItem()) {
468467
throw std::invalid_argument(folly::sformat(
469-
"Chained item can not be flushed separately {}", hdl->toString()));
468+
"Chained item can not be flushed separately {}", item.toString()));
470469
}
471470

472471
auto chainedItemRange =
473-
CacheAPIWrapperForNvm<C>::viewAsChainedAllocsRange(cache_, *hdl);
472+
CacheAPIWrapperForNvm<C>::viewAsChainedAllocsRange(cache_, item);
474473
if (config_.encodeCb && !config_.encodeCb(EncodeDecodeContext{
475-
*(hdl.getInternal()), chainedItemRange})) {
474+
const_cast<Item&>(item), chainedItemRange})) {
476475
return nullptr;
477476
}
478477

@@ -496,12 +495,10 @@ std::unique_ptr<NvmItem> NvmCache<C>::makeNvmItem(const WriteHandle& hdl) {
496495
}
497496

498497
template <typename C>
499-
void NvmCache<C>::put(WriteHandle& hdl, PutToken token) {
498+
void NvmCache<C>::put(Item& item, PutToken token) {
500499
util::LatencyTracker tracker(stats().nvmInsertLatency_);
501-
HashedKey hk{hdl->getKey()};
500+
HashedKey hk{item.getKey()};
502501

503-
XDCHECK(hdl);
504-
auto& item = *hdl;
505502
// for regular items that can only write to nvmcache upon eviction, we
506503
// should not be recording a write for an nvmclean item unless it is marked
507504
// as evicted from nvmcache.
@@ -526,7 +523,7 @@ void NvmCache<C>::put(WriteHandle& hdl, PutToken token) {
526523
return;
527524
}
528525

529-
auto nvmItem = makeNvmItem(hdl);
526+
auto nvmItem = makeNvmItem(item);
530527
if (!nvmItem) {
531528
stats().numNvmPutEncodeFailure.inc();
532529
return;

‎cachelib/allocator/nvmcache/NvmCache.h

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -158,11 +158,11 @@ class NvmCache {
158158
PutToken createPutToken(folly::StringPiece key);
159159

160160
// store the given item in navy
161-
// @param hdl handle to cache item. should not be null
161+
// @param item reference to cache item
162162
// @param token the put token for the item. this must have been
163163
// obtained before enqueueing the put to maintain
164164
// consistency
165-
void put(WriteHandle& hdl, PutToken token);
165+
void put(Item& item, PutToken token);
166166

167167
// returns the current state of whether nvmcache is enabled or not. nvmcache
168168
// can be disabled if the backend implementation ends up in a corrupt state
@@ -286,7 +286,7 @@ class NvmCache {
286286
// returns true if there is tombstone entry for the key.
287287
bool hasTombStone(HashedKey hk);
288288

289-
std::unique_ptr<NvmItem> makeNvmItem(const WriteHandle& handle);
289+
std::unique_ptr<NvmItem> makeNvmItem(const Item& item);
290290

291291
// wrap an item into a blob for writing into navy.
292292
Blob makeBlob(const Item& it);

‎cachelib/allocator/nvmcache/tests/NvmCacheTests.cpp

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -2072,7 +2072,7 @@ TEST_F(NvmCacheTest, testEvictCB) {
20722072
auto handle = cache.allocate(pid, key, 100);
20732073
ASSERT_NE(nullptr, handle.get());
20742074
std::memcpy(handle->getMemory(), val.data(), val.size());
2075-
auto buf = toIOBuf(makeNvmItem(handle));
2075+
auto buf = toIOBuf(makeNvmItem(*handle));
20762076
evictCB(HashedKey{key.data()},
20772077
navy::BufferView(buf.length(), buf.data()),
20782078
navy::DestructorEvent::Recycled);
@@ -2093,7 +2093,7 @@ TEST_F(NvmCacheTest, testEvictCB) {
20932093
ASSERT_NE(nullptr, handle.get());
20942094
std::memcpy(handle->getMemory(), val.data(), val.size());
20952095
cache.insertOrReplace(handle);
2096-
auto buf = toIOBuf(makeNvmItem(handle));
2096+
auto buf = toIOBuf(makeNvmItem(*handle));
20972097
evictCB(HashedKey{key.data()},
20982098
navy::BufferView(buf.length(), buf.data()),
20992099
navy::DestructorEvent::Recycled);
@@ -2112,7 +2112,7 @@ TEST_F(NvmCacheTest, testEvictCB) {
21122112
std::memcpy(handle->getMemory(), val.data(), val.size());
21132113
cache.insertOrReplace(handle);
21142114
handle->markNvmClean();
2115-
auto buf = toIOBuf(makeNvmItem(handle));
2115+
auto buf = toIOBuf(makeNvmItem(*handle));
21162116
evictCB(HashedKey{key.data()},
21172117
navy::BufferView(buf.length(), buf.data()),
21182118
navy::DestructorEvent::Recycled);
@@ -2128,7 +2128,7 @@ TEST_F(NvmCacheTest, testEvictCB) {
21282128
auto handle = cache.allocate(pid, key, 100);
21292129
ASSERT_NE(nullptr, handle.get());
21302130
std::memcpy(handle->getMemory(), val.data(), val.size());
2131-
auto buf = toIOBuf(makeNvmItem(handle));
2131+
auto buf = toIOBuf(makeNvmItem(*handle));
21322132
evictCB(HashedKey{key.data()},
21332133
navy::BufferView(buf.length(), buf.data()),
21342134
navy::DestructorEvent::Removed);
@@ -2147,7 +2147,7 @@ TEST_F(NvmCacheTest, testEvictCB) {
21472147
ASSERT_NE(nullptr, handle.get());
21482148
std::memcpy(handle->getMemory(), val.data(), val.size());
21492149
cache.insertOrReplace(handle);
2150-
auto buf = toIOBuf(makeNvmItem(handle));
2150+
auto buf = toIOBuf(makeNvmItem(*handle));
21512151
evictCB(HashedKey{key.data()},
21522152
navy::BufferView(buf.length(), buf.data()),
21532153
navy::DestructorEvent::Removed);
@@ -2166,7 +2166,7 @@ TEST_F(NvmCacheTest, testEvictCB) {
21662166
std::memcpy(handle->getMemory(), val.data(), val.size());
21672167
cache.insertOrReplace(handle);
21682168
handle->markNvmClean();
2169-
auto buf = toIOBuf(makeNvmItem(handle));
2169+
auto buf = toIOBuf(makeNvmItem(*handle));
21702170
evictCB(HashedKey{key.data()},
21712171
navy::BufferView(buf.length(), buf.data()),
21722172
navy::DestructorEvent::Removed);
@@ -2228,7 +2228,7 @@ TEST_F(NvmCacheTest, testCreateItemAsIOBuf) {
22282228
ASSERT_NE(nullptr, handle.get());
22292229
std::memcpy(handle->getMemory(), val.data(), val.size());
22302230

2231-
auto dipper = makeNvmItem(handle);
2231+
auto dipper = makeNvmItem(*handle);
22322232
auto iobuf = createItemAsIOBuf(key, *dipper);
22332233

22342234
verifyItemInIOBuf(key, handle, iobuf.get());
@@ -2240,7 +2240,7 @@ TEST_F(NvmCacheTest, testCreateItemAsIOBuf) {
22402240
ASSERT_NE(nullptr, handle.get());
22412241
std::memcpy(handle->getMemory(), val.data(), val.size());
22422242

2243-
auto dipper = makeNvmItem(handle);
2243+
auto dipper = makeNvmItem(*handle);
22442244
auto iobuf = createItemAsIOBuf(key, *dipper);
22452245

22462246
verifyItemInIOBuf(key, handle, iobuf.get());
@@ -2269,7 +2269,7 @@ TEST_F(NvmCacheTest, testCreateItemAsIOBufChained) {
22692269
cache.addChainedItem(handle, std::move(chainedIt));
22702270
}
22712271

2272-
auto dipper = makeNvmItem(handle);
2272+
auto dipper = makeNvmItem(*handle);
22732273
auto iobuf = createItemAsIOBuf(key, *dipper);
22742274

22752275
verifyItemInIOBuf(key, handle, iobuf.get());

‎cachelib/allocator/nvmcache/tests/NvmTestBase.h

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -108,7 +108,7 @@ class NvmCacheTest : public testing::Test {
108108
void pushToNvmCacheFromRamForTesting(WriteHandle& handle) {
109109
auto nvmCache = getNvmCache();
110110
if (nvmCache) {
111-
nvmCache->put(handle, nvmCache->createPutToken(handle->getKey()));
111+
nvmCache->put(*handle, nvmCache->createPutToken(handle->getKey()));
112112
}
113113
}
114114

@@ -126,8 +126,8 @@ class NvmCacheTest : public testing::Test {
126126
return cache_ ? cache_->nvmCache_.get() : nullptr;
127127
}
128128

129-
std::unique_ptr<NvmItem> makeNvmItem(const WriteHandle& handle) {
130-
return getNvmCache()->makeNvmItem(handle);
129+
std::unique_ptr<NvmItem> makeNvmItem(const Item& item) {
130+
return getNvmCache()->makeNvmItem(item);
131131
}
132132

133133
std::unique_ptr<folly::IOBuf> createItemAsIOBuf(folly::StringPiece key,

0 commit comments

Comments
 (0)