Skip to content

Commit 1716061

Browse files
committed
Use existing (upstream) code for moving items between tiers
instead of relying on transparent synchronization mechanims (wait context).
1 parent 78ea8d3 commit 1716061

File tree

3 files changed

+30
-334
lines changed

3 files changed

+30
-334
lines changed

cachelib/allocator/CacheAllocator-inl.h

Lines changed: 24 additions & 223 deletions
Original file line numberDiff line numberDiff line change
@@ -44,8 +44,6 @@ CacheAllocator<CacheTrait>::CacheAllocator(Config config)
4444
[this](Item* it) -> ItemHandle { return acquire(it); })),
4545
chainedItemLocks_(config_.chainedItemsLockPower,
4646
std::make_shared<MurmurHash2>()),
47-
movesMap_(kShards),
48-
moveLock_(kShards),
4947
cacheCreationTime_{util::getCurrentTimeSec()} {
5048

5149
if (numTiers_ > 1 || std::holds_alternative<FileShmSegmentOpts>(
@@ -132,8 +130,6 @@ CacheAllocator<CacheTrait>::CacheAllocator(SharedMemNewT, Config config)
132130
[this](Item* it) -> ItemHandle { return acquire(it); })),
133131
chainedItemLocks_(config_.chainedItemsLockPower,
134132
std::make_shared<MurmurHash2>()),
135-
movesMap_(kShards),
136-
moveLock_(kShards),
137133
cacheCreationTime_{util::getCurrentTimeSec()} {
138134
initCommon(false);
139135
shmManager_->removeShm(detail::kShmInfoName,
@@ -170,8 +166,6 @@ CacheAllocator<CacheTrait>::CacheAllocator(SharedMemAttachT, Config config)
170166
[this](Item* it) -> ItemHandle { return acquire(it); })),
171167
chainedItemLocks_(config_.chainedItemsLockPower,
172168
std::make_shared<MurmurHash2>()),
173-
movesMap_(kShards),
174-
moveLock_(kShards),
175169
cacheCreationTime_{*metadata_.cacheCreationTime_ref()} {
176170
/* TODO - per tier? */
177171
for (auto pid : *metadata_.compactCachePools_ref()) {
@@ -272,6 +266,14 @@ void CacheAllocator<CacheTrait>::initCommon(bool dramCacheAttached) {
272266
nvmAdmissionPolicy_->initMinTTL(config_.nvmAdmissionMinTTL);
273267
}
274268
}
269+
if (numTiers_ > 1 && !config_.moveCb) {
270+
XLOG(WARN, "No moveCb set, using memcpy for moving items between tiers.");
271+
config_.moveCb = [](Item& oldItem, Item& newItem, Item* parentItem){
272+
if (parentItem != nullptr)
273+
throw std::runtime_error("TODO: chained items not supported");
274+
std::memcpy(newItem.getMemory(), oldItem.getMemory(), oldItem.getSize());
275+
};
276+
}
275277
initStats();
276278
initNvmCache(dramCacheAttached);
277279
initWorkers();
@@ -1289,159 +1291,11 @@ CacheAllocator<CacheTrait>::insertOrReplace(const ItemHandle& handle) {
12891291
return replaced;
12901292
}
12911293

1292-
/* Next two methods are used to asynchronously move Item between memory tiers.
1293-
*
1294-
* The thread, which moves Item, allocates new Item in the tier we are moving to
1295-
* and calls moveRegularItemOnEviction() method. This method does the following:
1296-
* 1. Create MoveCtx and put it to the movesMap.
1297-
* 2. Update the access container with the new item from the tier we are
1298-
* moving to. This Item has kIncomplete flag set.
1299-
* 3. Copy data from the old Item to the new one.
1300-
* 4. Unset the kIncomplete flag and Notify MoveCtx
1301-
*
1302-
* Concurrent threads which are getting handle to the same key:
1303-
* 1. When a handle is created it checks if the kIncomplete flag is set
1304-
* 2. If so, Handle implementation creates waitContext and adds it to the
1305-
* MoveCtx by calling addWaitContextForMovingItem() method.
1306-
* 3. Wait until the moving thread will complete its job.
1307-
*/
1308-
template <typename CacheTrait>
1309-
bool CacheAllocator<CacheTrait>::addWaitContextForMovingItem(
1310-
folly::StringPiece key, std::shared_ptr<WaitContext<ReadHandle>> waiter) {
1311-
auto shard = getShardForKey(key);
1312-
auto& movesMap = getMoveMapForShard(shard);
1313-
auto lock = getMoveLockForShard(shard);
1314-
auto it = movesMap.find(key);
1315-
if (it == movesMap.end()) {
1316-
return false;
1317-
}
1318-
auto ctx = it->second.get();
1319-
ctx->addWaiter(std::move(waiter));
1320-
return true;
1321-
}
1322-
1323-
template <typename CacheTrait>
1324-
typename CacheAllocator<CacheTrait>::ItemHandle
1325-
CacheAllocator<CacheTrait>::moveRegularItemOnEviction(
1326-
Item& oldItem, ItemHandle& newItemHdl) {
1327-
XDCHECK(oldItem.isMoving());
1328-
// TODO: should we introduce new latency tracker. E.g. evictRegularLatency_
1329-
// ??? util::LatencyTracker tracker{stats_.evictRegularLatency_};
1330-
1331-
if (!oldItem.isAccessible() || oldItem.isExpired()) {
1332-
return {};
1333-
}
1334-
1335-
XDCHECK_EQ(newItemHdl->getSize(), oldItem.getSize());
1336-
XDCHECK_NE(getTierId(oldItem), getTierId(*newItemHdl));
1337-
1338-
// take care of the flags before we expose the item to be accessed. this
1339-
// will ensure that when another thread removes the item from RAM, we issue
1340-
// a delete accordingly. See D7859775 for an example
1341-
if (oldItem.isNvmClean()) {
1342-
newItemHdl->markNvmClean();
1343-
}
1344-
1345-
folly::StringPiece key(oldItem.getKey());
1346-
auto shard = getShardForKey(key);
1347-
auto& movesMap = getMoveMapForShard(shard);
1348-
MoveCtx* ctx(nullptr);
1349-
{
1350-
auto lock = getMoveLockForShard(shard);
1351-
auto res = movesMap.try_emplace(key, std::make_unique<MoveCtx>());
1352-
if (!res.second) {
1353-
return {};
1354-
}
1355-
ctx = res.first->second.get();
1356-
}
1357-
1358-
auto resHdl = ItemHandle{};
1359-
auto guard = folly::makeGuard([key, this, ctx, shard, &resHdl]() {
1360-
auto& movesMap = getMoveMapForShard(shard);
1361-
if (resHdl)
1362-
resHdl->unmarkIncomplete();
1363-
auto lock = getMoveLockForShard(shard);
1364-
ctx->setItemHandle(std::move(resHdl));
1365-
movesMap.erase(key);
1366-
});
1367-
1368-
// TODO: Possibly we can use markMoving() instead. But today
1369-
// moveOnSlabRelease logic assume that we mark as moving old Item
1370-
// and than do copy and replace old Item with the new one in access
1371-
// container. Furthermore, Item can be marked as Moving only
1372-
// if it is linked to MM container. In our case we mark the new Item
1373-
// and update access container before the new Item is ready (content is
1374-
// copied).
1375-
newItemHdl->markIncomplete();
1376-
1377-
// Inside the access container's lock, this checks if the old item is
1378-
// accessible and its refcount is zero. If the item is not accessible,
1379-
// there is no point to replace it since it had already been removed
1380-
// or in the process of being removed. If the item is in cache but the
1381-
// refcount is non-zero, it means user could be attempting to remove
1382-
// this item through an API such as remove(ItemHandle). In this case,
1383-
// it is unsafe to replace the old item with a new one, so we should
1384-
// also abort.
1385-
if (!accessContainer_->replaceIf(oldItem, *newItemHdl,
1386-
itemMovingPredicate)) {
1387-
return {};
1388-
}
1389-
1390-
if (config_.moveCb) {
1391-
// Execute the move callback. We cannot make any guarantees about the
1392-
// consistency of the old item beyond this point, because the callback can
1393-
// do more than a simple memcpy() e.g. update external references. If there
1394-
// are any remaining handles to the old item, it is the caller's
1395-
// responsibility to invalidate them. The move can only fail after this
1396-
// statement if the old item has been removed or replaced, in which case it
1397-
// should be fine for it to be left in an inconsistent state.
1398-
config_.moveCb(oldItem, *newItemHdl, nullptr);
1399-
} else {
1400-
std::memcpy(newItemHdl->getWritableMemory(), oldItem.getMemory(),
1401-
oldItem.getSize());
1402-
}
1403-
1404-
// Inside the MM container's lock, this checks if the old item exists to
1405-
// make sure that no other thread removed it, and only then replaces it.
1406-
if (!replaceInMMContainer(oldItem, *newItemHdl)) {
1407-
accessContainer_->remove(*newItemHdl);
1408-
return {};
1409-
}
1410-
1411-
// Replacing into the MM container was successful, but someone could have
1412-
// called insertOrReplace() or remove() before or after the
1413-
// replaceInMMContainer() operation, which would invalidate newItemHdl.
1414-
if (!newItemHdl->isAccessible()) {
1415-
removeFromMMContainer(*newItemHdl);
1416-
return {};
1417-
}
1418-
1419-
// no one can add or remove chained items at this point
1420-
if (oldItem.hasChainedItem()) {
1421-
// safe to acquire handle for a moving Item
1422-
auto oldHandle = acquire(&oldItem);
1423-
XDCHECK_EQ(1u, oldHandle->getRefCount()) << oldHandle->toString();
1424-
XDCHECK(!newItemHdl->hasChainedItem()) << newItemHdl->toString();
1425-
try {
1426-
auto l = chainedItemLocks_.lockExclusive(oldItem.getKey());
1427-
transferChainLocked(oldHandle, newItemHdl);
1428-
} catch (const std::exception& e) {
1429-
// this should never happen because we drained all the handles.
1430-
XLOGF(DFATAL, "{}", e.what());
1431-
throw;
1432-
}
1433-
1434-
XDCHECK(!oldItem.hasChainedItem());
1435-
XDCHECK(newItemHdl->hasChainedItem());
1436-
}
1437-
newItemHdl.unmarkNascent();
1438-
resHdl = std::move(newItemHdl); // guard will assign it to ctx under lock
1439-
return acquire(&oldItem);
1440-
}
1441-
14421294
template <typename CacheTrait>
1295+
template <typename Predicate>
14431296
bool CacheAllocator<CacheTrait>::moveRegularItem(Item& oldItem,
1444-
ItemHandle& newItemHdl) {
1297+
ItemHandle& newItemHdl,
1298+
Predicate &&predicate) {
14451299
XDCHECK(config_.moveCb);
14461300
util::LatencyTracker tracker{stats_.moveRegularLatency_};
14471301

@@ -1450,8 +1304,6 @@ bool CacheAllocator<CacheTrait>::moveRegularItem(Item& oldItem,
14501304
}
14511305

14521306
XDCHECK_EQ(newItemHdl->getSize(), oldItem.getSize());
1453-
XDCHECK_EQ(reinterpret_cast<uintptr_t>(&getMMContainer(oldItem)),
1454-
reinterpret_cast<uintptr_t>(&getMMContainer(*newItemHdl)));
14551307

14561308
// take care of the flags before we expose the item to be accessed. this
14571309
// will ensure that when another thread removes the item from RAM, we issue
@@ -1477,7 +1329,7 @@ bool CacheAllocator<CacheTrait>::moveRegularItem(Item& oldItem,
14771329
// this item through an API such as remove(ItemHandle). In this case,
14781330
// it is unsafe to replace the old item with a new one, so we should
14791331
// also abort.
1480-
if (!accessContainer_->replaceIf(oldItem, *newItemHdl, itemMovingPredicate)) {
1332+
if (!accessContainer_->replaceIf(oldItem, *newItemHdl, predicate)) {
14811333
return false;
14821334
}
14831335

@@ -1518,63 +1370,6 @@ bool CacheAllocator<CacheTrait>::moveRegularItem(Item& oldItem,
15181370
return true;
15191371
}
15201372

1521-
template <typename CacheTrait>
1522-
bool CacheAllocator<CacheTrait>::moveRegularItemForPromotion(Item& oldItem,
1523-
ItemHandle& newItemHdl) {
1524-
XDCHECK(config_.moveCb);
1525-
util::LatencyTracker tracker{stats_.moveRegularLatency_};
1526-
1527-
if (!oldItem.isAccessible() || oldItem.isExpired()) {
1528-
return false;
1529-
}
1530-
1531-
XDCHECK_EQ(newItemHdl->getSize(), oldItem.getSize());
1532-
1533-
if (config_.moveCb) {
1534-
// Execute the move callback. We cannot make any guarantees about the
1535-
// consistency of the old item beyond this point, because the callback can
1536-
// do more than a simple memcpy() e.g. update external references. If there
1537-
// are any remaining handles to the old item, it is the caller's
1538-
// responsibility to invalidate them. The move can only fail after this
1539-
// statement if the old item has been removed or replaced, in which case it
1540-
// should be fine for it to be left in an inconsistent state.
1541-
config_.moveCb(oldItem, *newItemHdl, nullptr);
1542-
} else {
1543-
std::memcpy(newItemHdl->getWritableMemory(), oldItem.getMemory(),
1544-
oldItem.getSize());
1545-
}
1546-
1547-
auto predicate = [this](const Item& item) {
1548-
// if inclusive cache is allowed, replace even if there are active users.
1549-
return config_.numDuplicateElements > 0 || item.getRefCount() == 0;
1550-
};
1551-
if (!accessContainer_->replaceIf(oldItem, *newItemHdl, predicate)) {
1552-
return false;
1553-
}
1554-
1555-
// Inside the MM container's lock, this checks if the old item exists to
1556-
// make sure that no other thread removed it, and only then replaces it.
1557-
if (!replaceInMMContainer(oldItem, *newItemHdl)) {
1558-
accessContainer_->remove(*newItemHdl);
1559-
return false;
1560-
}
1561-
1562-
// Replacing into the MM container was successful, but someone could have
1563-
// called insertOrReplace() or remove() before or after the
1564-
// replaceInMMContainer() operation, which would invalidate newItemHdl.
1565-
if (!newItemHdl->isAccessible()) {
1566-
removeFromMMContainer(*newItemHdl);
1567-
return false;
1568-
}
1569-
1570-
// no one can add or remove chained items at this point
1571-
if (oldItem.hasChainedItem()) {
1572-
throw std::runtime_error("Not supported");
1573-
}
1574-
newItemHdl.unmarkNascent();
1575-
return true;
1576-
}
1577-
15781373
template <typename CacheTrait>
15791374
bool CacheAllocator<CacheTrait>::moveChainedItem(ChainedItem& oldItem,
15801375
ItemHandle& newItemHdl) {
@@ -1814,8 +1609,9 @@ CacheAllocator<CacheTrait>::tryEvictToNextMemoryTier(
18141609

18151610
if (newItemHdl) {
18161611
XDCHECK_EQ(newItemHdl->getSize(), item.getSize());
1817-
1818-
return moveRegularItemOnEviction(item, newItemHdl);
1612+
if (tryMovingForSlabRelease(item, newItemHdl, itemMovingPredicate)) {
1613+
return acquire(&item);
1614+
}
18191615
}
18201616
}
18211617

@@ -1841,7 +1637,11 @@ CacheAllocator<CacheTrait>::tryPromoteToNextMemoryTier(
18411637

18421638
if (newItemHdl) {
18431639
XDCHECK_EQ(newItemHdl->getSize(), item.getSize());
1844-
if (moveRegularItemForPromotion(item, newItemHdl)) {
1640+
auto predicate = [this](const Item& item) {
1641+
// if inclusive cache is allowed, replace even if there are active users.
1642+
return config_.numDuplicateElements > 0 || item.getRefCount() == 0;
1643+
};
1644+
if (tryMovingForSlabRelease(item, newItemHdl, std::move(predicate))) {
18451645
return true;
18461646
}
18471647
}
@@ -2918,7 +2718,7 @@ bool CacheAllocator<CacheTrait>::moveForSlabRelease(
29182718

29192719
// if we have a valid handle, try to move, if not, we retry.
29202720
if (newItemHdl) {
2921-
isMoved = tryMovingForSlabRelease(oldItem, newItemHdl);
2721+
isMoved = tryMovingForSlabRelease(oldItem, newItemHdl, itemMovingPredicate);
29222722
if (isMoved) {
29232723
break;
29242724
}
@@ -3041,8 +2841,9 @@ CacheAllocator<CacheTrait>::allocateNewItemForOldItem(const Item& oldItem) {
30412841
}
30422842

30432843
template <typename CacheTrait>
2844+
template <typename Predicate>
30442845
bool CacheAllocator<CacheTrait>::tryMovingForSlabRelease(
3045-
Item& oldItem, ItemHandle& newItemHdl) {
2846+
Item& oldItem, ItemHandle& newItemHdl, Predicate&& predicate) {
30462847
// By holding onto a user-level synchronization object, we ensure moving
30472848
// a regular item or chained item is synchronized with any potential
30482849
// user-side mutation.
@@ -3074,7 +2875,7 @@ bool CacheAllocator<CacheTrait>::tryMovingForSlabRelease(
30742875

30752876
return oldItem.isChainedItem()
30762877
? moveChainedItem(oldItem.asChainedItem(), newItemHdl)
3077-
: moveRegularItem(oldItem, newItemHdl);
2878+
: moveRegularItem(oldItem, newItemHdl, std::move(predicate));
30782879
}
30792880

30802881
template <typename CacheTrait>

0 commit comments

Comments
 (0)