@@ -1104,6 +1104,22 @@ CacheAllocator<CacheTrait>::insertOrReplace(const ItemHandle& handle) {
11041104 return replaced;
11051105}
11061106
1107+ template <typename CacheTrait>
1108+ bool CacheAllocator<CacheTrait>::addWaitContextForMovingItem(
1109+ folly::StringPiece key,
1110+ std::shared_ptr<WaitContext<ItemHandle>> waiter) {
1111+ auto shard = getShardForKey (key);
1112+ auto & movesMap = getMoveMapForShard (shard);
1113+ auto lock = getMoveLockForShard (shard);
1114+ auto it = movesMap.find (key);
1115+ if (it == movesMap.end ()) {
1116+ return false ;
1117+ }
1118+ auto ctx = it->second .get ();
1119+ ctx->addWaiter (std::move (waiter));
1120+ return true ;
1121+ }
1122+
11071123template <typename CacheTrait>
11081124bool CacheAllocator<CacheTrait>::moveRegularItemOnEviction(
11091125 Item& oldItem,
@@ -1125,22 +1141,35 @@ bool CacheAllocator<CacheTrait>::moveRegularItemOnEviction(
11251141 newItemHdl->markNvmClean ();
11261142 }
11271143
1128- if (config_. moveCb ) {
1129- // Execute the move callback. We cannot make any guarantees about the
1130- // consistency of the old item beyond this point, because the callback can
1131- // do more than a simple memcpy() e.g. update external references. If there
1132- // are any remaining handles to the old item, it is the caller's
1133- // responsibility to invalidate them. The move can only fail after this
1134- // statement if the old item has been removed or replaced, in which case it
1135- // should be fine for it to be left in an inconsistent state.
1136- config_. moveCb (oldItem, *newItemHdl, nullptr ) ;
1137- } else {
1138- std::memcpy (newItemHdl-> getWritableMemory (), oldItem. getMemory (), oldItem. getSize () );
1144+ folly::StringPiece key (oldItem. getKey ());
1145+ auto shard = getShardForKey (key);
1146+ auto & movesMap = getMoveMapForShard (shard);
1147+ MoveCtx* ctx ( nullptr );
1148+ {
1149+ auto lock = getMoveLockForShard (shard);
1150+ auto res = movesMap. try_emplace (key, std::make_unique<MoveCtx>());
1151+ if (!res. second ) {
1152+ return false ;
1153+ }
1154+ ctx = res. first -> second . get ( );
11391155 }
11401156
1141- // TODO: Possible data race. We copied Item's memory to the newItemHdl
1142- // but have not updated accessContainer yet. Concurrent threads might get handle
1143- // to the old Item.
1157+ auto resHdl = ItemHandle{};
1158+ auto guard = folly::makeGuard ([key, this , ctx, shard, &resHdl]() {
1159+ auto & movesMap = getMoveMapForShard (shard);
1160+ resHdl->unmarkNotReady ();
1161+ auto lock = getMoveLockForShard (shard);
1162+ ctx->setItemHandle (std::move (resHdl));
1163+ movesMap.erase (key);
1164+ });
1165+
1166+ // TODO: Possibly we can use markMoving() instead. But today
1167+ // moveOnSlabRelease logic assume that we mark as moving old Item
1168+ // and than do copy and replace old Item with the new one in access
1169+ // container. Furthermore, Item can be marked as Moving only
1170+ // if it is linked to MM container. In our case we mark the new Item
1171+ // and update access container before the new Item is ready (content is copied).
1172+ newItemHdl->markNotReady ();
11441173
11451174 // Inside the access container's lock, this checks if the old item is
11461175 // accessible and its refcount is zero. If the item is not accessible,
@@ -1154,6 +1183,19 @@ bool CacheAllocator<CacheTrait>::moveRegularItemOnEviction(
11541183 return false ;
11551184 }
11561185
1186+ if (config_.moveCb ) {
1187+ // Execute the move callback. We cannot make any guarantees about the
1188+ // consistency of the old item beyond this point, because the callback can
1189+ // do more than a simple memcpy() e.g. update external references. If there
1190+ // are any remaining handles to the old item, it is the caller's
1191+ // responsibility to invalidate them. The move can only fail after this
1192+ // statement if the old item has been removed or replaced, in which case it
1193+ // should be fine for it to be left in an inconsistent state.
1194+ config_.moveCb (oldItem, *newItemHdl, nullptr );
1195+ } else {
1196+ std::memcpy (newItemHdl->getWritableMemory (), oldItem.getMemory (), oldItem.getSize ());
1197+ }
1198+
11571199 // Inside the MM container's lock, this checks if the old item exists to
11581200 // make sure that no other thread removed it, and only then replaces it.
11591201 if (!replaceInMMContainer (oldItem, *newItemHdl)) {
@@ -1188,6 +1230,7 @@ bool CacheAllocator<CacheTrait>::moveRegularItemOnEviction(
11881230 XDCHECK (newItemHdl->hasChainedItem ());
11891231 }
11901232 newItemHdl.unmarkNascent ();
1233+ resHdl = newItemHdl.clone (); // guard will assign it to ctx under lock
11911234 return true ;
11921235}
11931236
0 commit comments