From 160bb77d58163a522efdccd75de5b22088cb70ba Mon Sep 17 00:00:00 2001 From: Open Source Bot Date: Wed, 15 Feb 2023 19:44:16 -0800 Subject: [PATCH 01/44] Updating submodules Summary: GitHub commits: https://github.com/facebook/wangle/commit/43e56205514ee4d77e483f4f52ff27dac3216514 https://github.com/facebookexperimental/edencommon/commit/3934bece13d925ae5dd99bd993caa0258f480fcc https://github.com/facebookincubator/katran/commit/a4c94aa4b0899c5480f803b557fb894c4e0a5ac3 Reviewed By: jurajh-fb fbshipit-source-id: 720310662068e4aa245198d817666700c9dde1d5 --- cachelib/external/fbthrift | 2 +- cachelib/external/fizz | 2 +- cachelib/external/wangle | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/cachelib/external/fbthrift b/cachelib/external/fbthrift index 33a9fbc25..1dea97776 160000 --- a/cachelib/external/fbthrift +++ b/cachelib/external/fbthrift @@ -1 +1 @@ -Subproject commit 33a9fbc258f21818f20ea03a55c979014882e84d +Subproject commit 1dea977766b157ff2f5df9922534361b44f9698e diff --git a/cachelib/external/fizz b/cachelib/external/fizz index 9198ca6e7..2716439e6 160000 --- a/cachelib/external/fizz +++ b/cachelib/external/fizz @@ -1 +1 @@ -Subproject commit 9198ca6e7daa50fae6b8413d745b4faaf97dfd10 +Subproject commit 2716439e6d0efdeec3c27d75793249bd42accdf0 diff --git a/cachelib/external/wangle b/cachelib/external/wangle index 6bc77c8d4..43e562055 160000 --- a/cachelib/external/wangle +++ b/cachelib/external/wangle @@ -1 +1 @@ -Subproject commit 6bc77c8d46b5ef68d77e921bb1e3d1e576adb8fe +Subproject commit 43e56205514ee4d77e483f4f52ff27dac3216514 From ba8d6b003a4f5856d88af02b09fac74d7f12ae07 Mon Sep 17 00:00:00 2001 From: Open Source Bot Date: Thu, 16 Feb 2023 13:23:03 -0800 Subject: [PATCH 02/44] Updating submodules Summary: GitHub commits: https://github.com/facebook/fb303/commit/c51a2906c5be9f12223b2314501b7cc3e9f801d9 https://github.com/facebook/fbthrift/commit/5e97023405d6bddc686e836e9a6d1117947cd9dd https://github.com/facebook/proxygen/commit/3710a3267346e43095e1f140e72706538ac22f23 https://github.com/facebook/wangle/commit/fd298004026d8f72fcfc37e6b4ff1749a88ea4d1 https://github.com/facebook/watchman/commit/fca53d2d93d8fc2f256aa99b4ebecc4e9bb5ac1a https://github.com/facebookexperimental/edencommon/commit/3d8a6d905c416ff7b8ae6c70765d34231d747a9d https://github.com/facebookexperimental/rust-shed/commit/75c5d75f7ec67dacf44db3dcf444b7511f038c58 https://github.com/facebookincubator/fizz/commit/bae2d7ebe3b348679e832102b65d47aceb4ac2f8 https://github.com/facebookincubator/katran/commit/139c75dfb61626dea75951ec1097a1373b5a182e https://github.com/facebookincubator/mvfst/commit/001332d5e9c5a8ee47e9c932e640474037832abf https://github.com/facebookincubator/velox/commit/ec0ea1fd5ffcf6dec60d2e53831478cbaf2ca4a0 Reviewed By: jurajh-fb fbshipit-source-id: a8a8ff6a7fddeaa7bbccb3faa6a737c50d22cfe2 --- cachelib/external/fbthrift | 2 +- cachelib/external/fizz | 2 +- cachelib/external/folly | 2 +- cachelib/external/wangle | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/cachelib/external/fbthrift b/cachelib/external/fbthrift index 1dea97776..5e9702340 160000 --- a/cachelib/external/fbthrift +++ b/cachelib/external/fbthrift @@ -1 +1 @@ -Subproject commit 1dea977766b157ff2f5df9922534361b44f9698e +Subproject commit 5e97023405d6bddc686e836e9a6d1117947cd9dd diff --git a/cachelib/external/fizz b/cachelib/external/fizz index 2716439e6..bae2d7ebe 160000 --- a/cachelib/external/fizz +++ b/cachelib/external/fizz @@ -1 +1 @@ -Subproject commit 2716439e6d0efdeec3c27d75793249bd42accdf0 +Subproject commit bae2d7ebe3b348679e832102b65d47aceb4ac2f8 diff --git a/cachelib/external/folly b/cachelib/external/folly index 128cfac6a..9aeeaf493 160000 --- a/cachelib/external/folly +++ b/cachelib/external/folly @@ -1 +1 @@ -Subproject commit 128cfac6ac3d69825bad2af852fced3f63d87411 +Subproject commit 9aeeaf4933c271559c995df862b52af5bd9645c2 diff --git a/cachelib/external/wangle b/cachelib/external/wangle index 43e562055..fd2980040 160000 --- a/cachelib/external/wangle +++ b/cachelib/external/wangle @@ -1 +1 @@ -Subproject commit 43e56205514ee4d77e483f4f52ff27dac3216514 +Subproject commit fd298004026d8f72fcfc37e6b4ff1749a88ea4d1 From ba9dd68a1bbae05676d332e9aa4c1a6d6b79348e Mon Sep 17 00:00:00 2001 From: Hao Wu Date: Fri, 17 Feb 2023 10:09:28 -0800 Subject: [PATCH 03/44] Change some of the stats to rate Summary: As mentioned in the tasks, these stats make more sense in rates. Reviewed By: jiayuebao Differential Revision: D43243498 fbshipit-source-id: 556b0b4a062235bb3b34dd076e5f7a5a88316f8c --- cachelib/navy/block_cache/BlockCache.cpp | 9 ++++++--- cachelib/navy/driver/Driver.cpp | 6 ++++-- 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/cachelib/navy/block_cache/BlockCache.cpp b/cachelib/navy/block_cache/BlockCache.cpp index f34605b68..84dadd13e 100644 --- a/cachelib/navy/block_cache/BlockCache.cpp +++ b/cachelib/navy/block_cache/BlockCache.cpp @@ -723,9 +723,11 @@ void BlockCache::getCounters(const CounterVisitor& visitor) const { reclaimValueChecksumErrorCount_.get(), CounterVisitor::CounterType::RATE); visitor("navy_bc_cleanup_entry_header_checksum_errors", - cleanupEntryHeaderChecksumErrorCount_.get()); + cleanupEntryHeaderChecksumErrorCount_.get(), + CounterVisitor::CounterType::RATE); visitor("navy_bc_cleanup_value_checksum_errors", - cleanupValueChecksumErrorCount_.get()); + cleanupValueChecksumErrorCount_.get(), + CounterVisitor::CounterType::RATE); visitor("navy_bc_succ_lookups", succLookupCount_.get(), CounterVisitor::CounterType::RATE); visitor("navy_bc_removes", removeCount_.get(), @@ -750,7 +752,8 @@ void BlockCache::getCounters(const CounterVisitor& visitor) const { visitor("navy_bc_reinsertion_errors", reinsertionErrorCount_.get(), CounterVisitor::CounterType::RATE); visitor("navy_bc_lookup_for_item_destructor_errors", - lookupForItemDestructorErrorCount_.get()); + lookupForItemDestructorErrorCount_.get(), + CounterVisitor::CounterType::RATE); visitor("navy_bc_remove_attempt_collisions", removeAttemptCollisions_.get(), CounterVisitor::CounterType::RATE); // Allocator visits region manager diff --git a/cachelib/navy/driver/Driver.cpp b/cachelib/navy/driver/Driver.cpp index 1615d1cc4..29215cc16 100644 --- a/cachelib/navy/driver/Driver.cpp +++ b/cachelib/navy/driver/Driver.cpp @@ -273,8 +273,10 @@ void Driver::getCounters(const CounterVisitor& visitor) const { CounterVisitor::CounterType::RATE); visitor("navy_rejected_bytes", rejectedBytes_.get(), CounterVisitor::CounterType::RATE); - visitor("navy_accepted_bytes", acceptedBytes_.get()); - visitor("navy_accepted", acceptedCount_.get()); + visitor("navy_accepted_bytes", acceptedBytes_.get(), + CounterVisitor::CounterType::RATE); + visitor("navy_accepted", acceptedCount_.get(), + CounterVisitor::CounterType::RATE); visitor("navy_parcel_memory", parcelMemory_.get()); visitor("navy_concurrent_inserts", concurrentInserts_.get()); From ad8bfd693f824f904cdcab4f2319aa68a6bf7e4b Mon Sep 17 00:00:00 2001 From: Open Source Bot Date: Fri, 17 Feb 2023 11:40:50 -0800 Subject: [PATCH 04/44] Updating submodules Summary: GitHub commits: https://github.com/facebook/cachelib/commit/ba9dd68a1bbae05676d332e9aa4c1a6d6b79348e https://github.com/facebook/fbthrift/commit/23208ff909ee751de695e6806f14820137045803 https://github.com/facebookincubator/velox/commit/68b8a45ff019bb20ed204451c29ac34352760840 Reviewed By: jurajh-fb fbshipit-source-id: f8581e13cd1c0a2510d87f42c813ca2c65eaef1d --- cachelib/external/fbthrift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cachelib/external/fbthrift b/cachelib/external/fbthrift index 5e9702340..23208ff90 160000 --- a/cachelib/external/fbthrift +++ b/cachelib/external/fbthrift @@ -1 +1 @@ -Subproject commit 5e97023405d6bddc686e836e9a6d1117947cd9dd +Subproject commit 23208ff909ee751de695e6806f14820137045803 From bdad762aa2bc38e97c9afc45c4edb9186750b1d3 Mon Sep 17 00:00:00 2001 From: Open Source Bot Date: Sat, 18 Feb 2023 11:28:38 -0800 Subject: [PATCH 05/44] Updating submodules Summary: GitHub commits: https://github.com/facebook/fbthrift/commit/3b00d58af9a028db696e2034c54cf9500fda53a7 Reviewed By: jurajh-fb fbshipit-source-id: 23ecb07e7e9ebd4dfbf3980de141ea1ebe36bb12 --- cachelib/external/fbthrift | 2 +- cachelib/external/fizz | 2 +- cachelib/external/folly | 2 +- cachelib/external/wangle | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/cachelib/external/fbthrift b/cachelib/external/fbthrift index 23208ff90..3b00d58af 160000 --- a/cachelib/external/fbthrift +++ b/cachelib/external/fbthrift @@ -1 +1 @@ -Subproject commit 23208ff909ee751de695e6806f14820137045803 +Subproject commit 3b00d58af9a028db696e2034c54cf9500fda53a7 diff --git a/cachelib/external/fizz b/cachelib/external/fizz index bae2d7ebe..2da27e939 160000 --- a/cachelib/external/fizz +++ b/cachelib/external/fizz @@ -1 +1 @@ -Subproject commit bae2d7ebe3b348679e832102b65d47aceb4ac2f8 +Subproject commit 2da27e939de1aa4eeecc6bc8d3a32844a75bd42b diff --git a/cachelib/external/folly b/cachelib/external/folly index 9aeeaf493..3c5efbdff 160000 --- a/cachelib/external/folly +++ b/cachelib/external/folly @@ -1 +1 @@ -Subproject commit 9aeeaf4933c271559c995df862b52af5bd9645c2 +Subproject commit 3c5efbdff2d01b83fc76827518c14a786f9c28ce diff --git a/cachelib/external/wangle b/cachelib/external/wangle index fd2980040..8733674dc 160000 --- a/cachelib/external/wangle +++ b/cachelib/external/wangle @@ -1 +1 @@ -Subproject commit fd298004026d8f72fcfc37e6b4ff1749a88ea4d1 +Subproject commit 8733674dc8977b22324b2dfbd956aa6763a9834c From b42327aea0839c726e8579fc9b36165d2d74bd30 Mon Sep 17 00:00:00 2001 From: Open Source Bot Date: Sat, 18 Feb 2023 18:09:16 -0800 Subject: [PATCH 06/44] Updating submodules Summary: GitHub commits: https://github.com/facebook/fbthrift/commit/df0eb612d7cb64ab73ed2f3b42bfe787a6ed4653 https://github.com/facebook/proxygen/commit/7fc3feabba6dca34b03e012d9a981f05e4ea27a3 Reviewed By: jurajh-fb fbshipit-source-id: fa982baf35fd98edc52be391f03d15a9c2c333bf --- cachelib/external/fbthrift | 2 +- cachelib/external/wangle | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/cachelib/external/fbthrift b/cachelib/external/fbthrift index 3b00d58af..df0eb612d 160000 --- a/cachelib/external/fbthrift +++ b/cachelib/external/fbthrift @@ -1 +1 @@ -Subproject commit 3b00d58af9a028db696e2034c54cf9500fda53a7 +Subproject commit df0eb612d7cb64ab73ed2f3b42bfe787a6ed4653 diff --git a/cachelib/external/wangle b/cachelib/external/wangle index 8733674dc..cb61ed175 160000 --- a/cachelib/external/wangle +++ b/cachelib/external/wangle @@ -1 +1 @@ -Subproject commit 8733674dc8977b22324b2dfbd956aa6763a9834c +Subproject commit cb61ed1759c692a4c69b85df7cb983ee920e91cf From 0d49b53fc50fbe00b48ef854454b884e11768878 Mon Sep 17 00:00:00 2001 From: Open Source Bot Date: Sun, 19 Feb 2023 20:30:30 -0800 Subject: [PATCH 07/44] Updating submodules Summary: GitHub commits: https://github.com/facebook/fbthrift/commit/1d01d76ff25b7d5c5d36a08f111834082277f8e7 Reviewed By: jurajh-fb fbshipit-source-id: bdefba88e8d002c65d8b7c11e86c8aa772996d9b --- cachelib/external/fbthrift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cachelib/external/fbthrift b/cachelib/external/fbthrift index df0eb612d..1d01d76ff 160000 --- a/cachelib/external/fbthrift +++ b/cachelib/external/fbthrift @@ -1 +1 @@ -Subproject commit df0eb612d7cb64ab73ed2f3b42bfe787a6ed4653 +Subproject commit 1d01d76ff25b7d5c5d36a08f111834082277f8e7 From bb2d1c657667a15f5ff96fc8a2dd5208bd93aee3 Mon Sep 17 00:00:00 2001 From: Open Source Bot Date: Tue, 21 Feb 2023 10:54:23 -0800 Subject: [PATCH 08/44] Updating submodules Summary: GitHub commits: https://github.com/facebook/fbthrift/commit/1b848d85947b8d20e00521e95f78690a73543678 https://github.com/facebook/rocksdb/commit/cfe50f7e77326aac5b04050afcda05059a25667c https://github.com/pytorch/fbgemm/commit/c55d8006722eae1ec886502363d8386862a13719 Reviewed By: bigfootjon fbshipit-source-id: e840d1116311ac52336e598442cf66cdc50d7725 --- cachelib/external/fbthrift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cachelib/external/fbthrift b/cachelib/external/fbthrift index 1d01d76ff..1b848d859 160000 --- a/cachelib/external/fbthrift +++ b/cachelib/external/fbthrift @@ -1 +1 @@ -Subproject commit 1d01d76ff25b7d5c5d36a08f111834082277f8e7 +Subproject commit 1b848d85947b8d20e00521e95f78690a73543678 From c167a8327810110cac2ab739ab2346a05cde5a88 Mon Sep 17 00:00:00 2001 From: Open Source Bot Date: Tue, 21 Feb 2023 14:59:14 -0800 Subject: [PATCH 09/44] Updating submodules Summary: GitHub commits: https://github.com/facebook/fbthrift/commit/90fddab67b2a40ec0edda28fd014402258726eef Reviewed By: bigfootjon fbshipit-source-id: cf7ac48c00c548805fb74663fe3e9bd9f5906cc2 --- cachelib/external/fbthrift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cachelib/external/fbthrift b/cachelib/external/fbthrift index 1b848d859..90fddab67 160000 --- a/cachelib/external/fbthrift +++ b/cachelib/external/fbthrift @@ -1 +1 @@ -Subproject commit 1b848d85947b8d20e00521e95f78690a73543678 +Subproject commit 90fddab67b2a40ec0edda28fd014402258726eef From 32ed439b6b319896386cdd4aa818b2ccf285ae23 Mon Sep 17 00:00:00 2001 From: Jiayue Bao Date: Tue, 21 Feb 2023 15:17:04 -0800 Subject: [PATCH 10/44] Return replacedPtr from InsertOrReplace API Summary: Currently `replacedPtr` is passed as an argument and such usage isn't easy to understand (we kept receiving user feedback on that). Now we want to return it. Reviewed By: therealgymmy, antonf, jaesoo-fb Differential Revision: D43143989 fbshipit-source-id: eaae055107050cd67a7ca8b26b716fffc0c002f2 --- .../experimental/objcache2/ObjectCache-inl.h | 28 +++++----- cachelib/experimental/objcache2/ObjectCache.h | 18 +++---- .../objcache2/persistence/Serialization.h | 4 +- .../objcache2/tests/ObjectCacheTest.cpp | 51 +++++++++++-------- 4 files changed, 54 insertions(+), 47 deletions(-) diff --git a/cachelib/experimental/objcache2/ObjectCache-inl.h b/cachelib/experimental/objcache2/ObjectCache-inl.h index 345a27d52..70cfc445b 100644 --- a/cachelib/experimental/objcache2/ObjectCache-inl.h +++ b/cachelib/experimental/objcache2/ObjectCache-inl.h @@ -153,12 +153,13 @@ std::shared_ptr ObjectCache::findToWrite( template template -std::pair::AllocStatus, std::shared_ptr> +std::tuple::AllocStatus, + std::shared_ptr, + std::shared_ptr> ObjectCache::insertOrReplace(folly::StringPiece key, std::unique_ptr object, size_t objectSize, - uint32_t ttlSecs, - std::shared_ptr* replacedPtr) { + uint32_t ttlSecs) { if (config_.objectSizeTrackingEnabled && objectSize == 0) { throw std::invalid_argument( "Object size tracking is enabled but object size is set to be 0."); @@ -176,7 +177,8 @@ ObjectCache::insertOrReplace(folly::StringPiece key, allocateFromL1(key, ttlSecs, 0 /* use current time as creationTime */); if (!handle) { insertErrors_.inc(); - return {AllocStatus::kAllocError, std::shared_ptr(std::move(object))}; + return {AllocStatus::kAllocError, std::shared_ptr(std::move(object)), + nullptr}; } // We don't release the object here because insertOrReplace could throw when // the replaced item is out of refcount; in this case, the object isn't @@ -187,16 +189,15 @@ ObjectCache::insertOrReplace(folly::StringPiece key, auto replaced = this->l1Cache_->insertOrReplace(handle); + std::shared_ptr replacedPtr = nullptr; if (replaced) { replaces_.inc(); - if (replacedPtr) { - auto itemPtr = reinterpret_cast(replaced->getMemory()); - // Just release the handle. Cache destorys object when all handles - // released. - auto deleter = [h = std::move(replaced)](T*) {}; - *replacedPtr = std::shared_ptr( - reinterpret_cast(itemPtr->objectPtr), std::move(deleter)); - } + auto itemPtr = reinterpret_cast(replaced->getMemory()); + // Just release the handle. Cache destorys object when all handles + // released. + auto deleter = [h = std::move(replaced)](T*) {}; + replacedPtr = std::shared_ptr(reinterpret_cast(itemPtr->objectPtr), + std::move(deleter)); } // Just release the handle. Cache destorys object when all handles released. @@ -209,7 +210,8 @@ ObjectCache::insertOrReplace(folly::StringPiece key, // Release the object as it has been successfully inserted to the cache. object.release(); - return {AllocStatus::kSuccess, std::shared_ptr(ptr, std::move(deleter))}; + return {AllocStatus::kSuccess, std::shared_ptr(ptr, std::move(deleter)), + replacedPtr}; } template diff --git a/cachelib/experimental/objcache2/ObjectCache.h b/cachelib/experimental/objcache2/ObjectCache.h index 85abac068..f4cd2a9bb 100644 --- a/cachelib/experimental/objcache2/ObjectCache.h +++ b/cachelib/experimental/objcache2/ObjectCache.h @@ -146,22 +146,20 @@ class ObjectCache : public ObjectCacheBase { // if objectSizeTracking is enabled, a non-zero value must // be passed. // @param ttlSecs object expiring seconds. - // @param replacedPtr a pointer to a shared_ptr, if it is not nullptr it will - // be assigned to the replaced object. // // @throw cachelib::exception::RefcountOverflow if the item we are replacing // is already out of refcounts. // @throw std::invalid_argument if objectSizeTracking is enabled but // objectSize is 0. - // @return a pair of allocation status and shared_ptr of newly inserted - // object. + // @return a tuple of allocation status, shared_ptr of newly inserted + // object and shared_ptr of old object that has been replaced (nullptr + // if no replacement happened) template - std::pair> insertOrReplace( - folly::StringPiece key, - std::unique_ptr object, - size_t objectSize = 0, - uint32_t ttlSecs = 0, - std::shared_ptr* replacedPtr = nullptr); + std::tuple, std::shared_ptr> + insertOrReplace(folly::StringPiece key, + std::unique_ptr object, + size_t objectSize = 0, + uint32_t ttlSecs = 0); // Insert the object into the cache with given key. If the key exists in the // cache, the new object won't be inserted. diff --git a/cachelib/experimental/objcache2/persistence/Serialization.h b/cachelib/experimental/objcache2/persistence/Serialization.h index cccb414b4..4edad88e4 100644 --- a/cachelib/experimental/objcache2/persistence/Serialization.h +++ b/cachelib/experimental/objcache2/persistence/Serialization.h @@ -68,9 +68,9 @@ struct ObjectDeserializer { Deserializer deserializer{reinterpret_cast(payload.begin()), reinterpret_cast(payload.end())}; auto ptr = std::make_unique(deserializer.deserialize()); - auto [allocStatus, _] = + auto res = objCache_.insertOrReplace(key, std::move(ptr), objectSize, ttlSecs); - return allocStatus == ObjectCache::AllocStatus::kSuccess; + return std::get<0>(res) == ObjectCache::AllocStatus::kSuccess; } // cache key of the object to be deserialized diff --git a/cachelib/experimental/objcache2/tests/ObjectCacheTest.cpp b/cachelib/experimental/objcache2/tests/ObjectCacheTest.cpp index 0bcc120de..6bafa4058 100644 --- a/cachelib/experimental/objcache2/tests/ObjectCacheTest.cpp +++ b/cachelib/experimental/objcache2/tests/ObjectCacheTest.cpp @@ -206,12 +206,12 @@ class ObjectCacheTest : public ::testing::Test { foo->a = 1; foo->b = 2; foo->c = 3; - auto res = objcache->insertOrReplace("Foo", std::move(foo)); - EXPECT_EQ(ObjectCache::AllocStatus::kSuccess, res.first); - ASSERT_NE(nullptr, res.second); - EXPECT_EQ(1, res.second->a); - EXPECT_EQ(2, res.second->b); - EXPECT_EQ(3, res.second->c); + auto [allocRes, ptr, _] = objcache->insertOrReplace("Foo", std::move(foo)); + EXPECT_EQ(ObjectCache::AllocStatus::kSuccess, allocRes); + ASSERT_NE(nullptr, ptr); + EXPECT_EQ(1, ptr->a); + EXPECT_EQ(2, ptr->b); + EXPECT_EQ(3, ptr->c); auto found2 = objcache->template find("Foo"); ASSERT_NE(nullptr, found2); @@ -238,7 +238,7 @@ class ObjectCacheTest : public ::testing::Test { foo->b = 2; foo->c = 3; auto res1 = objcache->insertOrReplace("Foo", std::move(foo)); - EXPECT_EQ(ObjectCache::AllocStatus::kSuccess, res1.first); + EXPECT_EQ(ObjectCache::AllocStatus::kSuccess, std::get<0>(res1)); auto found1 = objcache->template find("Foo"); ASSERT_NE(nullptr, found1); @@ -251,7 +251,7 @@ class ObjectCacheTest : public ::testing::Test { foo2->e = 5; foo2->f = 6; auto res2 = objcache->insertOrReplace("Foo2", std::move(foo2)); - EXPECT_EQ(ObjectCache::AllocStatus::kSuccess, res2.first); + EXPECT_EQ(ObjectCache::AllocStatus::kSuccess, std::get<0>(res2)); auto found2 = objcache->template find("Foo2"); ASSERT_NE(nullptr, found2); @@ -272,7 +272,7 @@ class ObjectCacheTest : public ::testing::Test { foo4->b = 2; foo4->c = 3; auto res1 = objcache->insertOrReplace("Foo4", std::move(foo4)); - EXPECT_EQ(ObjectCache::AllocStatus::kSuccess, res1.first); + EXPECT_EQ(ObjectCache::AllocStatus::kSuccess, std::get<0>(res1)); auto found1 = objcache->template find("Foo4"); ASSERT_NE(nullptr, found1); @@ -285,7 +285,7 @@ class ObjectCacheTest : public ::testing::Test { foo5->e = 5; foo5->f = 6; auto res2 = objcache->insertOrReplace("Foo5", std::move(foo5)); - EXPECT_EQ(ObjectCache::AllocStatus::kSuccess, res2.first); + EXPECT_EQ(ObjectCache::AllocStatus::kSuccess, std::get<0>(res2)); auto found2 = objcache->template find("Foo5"); ASSERT_NE(nullptr, found2); @@ -385,11 +385,14 @@ class ObjectCacheTest : public ::testing::Test { foo1->a = 1; foo1->b = 2; foo1->c = 3; - std::shared_ptr replaced; - auto res = - objcache->insertOrReplace("Foo", std::move(foo1), 0, 0, &replaced); - EXPECT_EQ(ObjectCache::AllocStatus::kSuccess, res.first); - EXPECT_EQ(nullptr, replaced); + + auto [res1, ptr1, replaced1] = + objcache->insertOrReplace("Foo", std::move(foo1)); + EXPECT_EQ(ObjectCache::AllocStatus::kSuccess, res1); + EXPECT_EQ(1, ptr1->a); + EXPECT_EQ(2, ptr1->b); + EXPECT_EQ(3, ptr1->c); + EXPECT_EQ(nullptr, replaced1); auto found1 = objcache->template find("Foo"); ASSERT_NE(nullptr, found1); @@ -401,12 +404,16 @@ class ObjectCacheTest : public ::testing::Test { foo2->a = 10; foo2->b = 20; foo2->c = 30; - res = objcache->insertOrReplace("Foo", std::move(foo2), 0, 0, &replaced); - EXPECT_EQ(ObjectCache::AllocStatus::kSuccess, res.first); - ASSERT_NE(nullptr, replaced); - EXPECT_EQ(1, replaced->a); - EXPECT_EQ(2, replaced->b); - EXPECT_EQ(3, replaced->c); + auto [res2, ptr2, replaced2] = + objcache->insertOrReplace("Foo", std::move(foo2)); + EXPECT_EQ(ObjectCache::AllocStatus::kSuccess, res2); + EXPECT_EQ(10, ptr2->a); + EXPECT_EQ(20, ptr2->b); + EXPECT_EQ(30, ptr2->c); + ASSERT_NE(nullptr, replaced2); + EXPECT_EQ(1, replaced2->a); + EXPECT_EQ(2, replaced2->b); + EXPECT_EQ(3, replaced2->c); auto found2 = objcache->template find("Foo"); ASSERT_NE(nullptr, found2); @@ -497,7 +504,7 @@ class ObjectCacheTest : public ::testing::Test { // replace foo1 with foo2 { auto res = objcache->insertOrReplace("Foo", std::move(foo2), foo2Size); - ASSERT_EQ(ObjectCache::AllocStatus::kSuccess, res.first); + ASSERT_EQ(ObjectCache::AllocStatus::kSuccess, std::get<0>(res)); auto found = objcache->template find("Foo"); ASSERT_NE(nullptr, found); From dd0af61a5fcb621ac253f67715b5fad8994627c7 Mon Sep 17 00:00:00 2001 From: Open Source Bot Date: Tue, 21 Feb 2023 15:51:28 -0800 Subject: [PATCH 11/44] Updating submodules Summary: GitHub commits: https://github.com/facebook/folly/commit/fdb333b2bb6dd9424d19c4e12385726ff87e60d4 https://github.com/facebook/watchman/commit/0b238b069f54a4de27c35a3e2352c37bccc12d60 https://github.com/pytorch/fbgemm/commit/d142cdf42560496e5f473a0f85220ff75681acc8 Reviewed By: bigfootjon fbshipit-source-id: ac185dc9dc24ba7d2765dca010958ce6b38bd029 --- cachelib/external/folly | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cachelib/external/folly b/cachelib/external/folly index 3c5efbdff..fdb333b2b 160000 --- a/cachelib/external/folly +++ b/cachelib/external/folly @@ -1 +1 @@ -Subproject commit 3c5efbdff2d01b83fc76827518c14a786f9c28ce +Subproject commit fdb333b2bb6dd9424d19c4e12385726ff87e60d4 From cce19ea5feeba497e8e72e7bda9e899d146a37df Mon Sep 17 00:00:00 2001 From: Open Source Bot Date: Wed, 22 Feb 2023 13:29:47 -0800 Subject: [PATCH 12/44] Updating submodules Summary: GitHub commits: https://github.com/facebook/fbthrift/commit/cbc3de581fdf36ba474b0c135b9e785e504f1c1e https://github.com/facebook/rocksdb/commit/229297d1b83c1885e7db2573b9b44736a7be23a5 https://github.com/facebook/watchman/commit/d873e11529fae6d0bbd74cfe95d41732dbfbded8 https://github.com/facebookexperimental/rust-shed/commit/2f45b886ae2eb0bf59664036f0fd36e3b20923d9 Reviewed By: bigfootjon fbshipit-source-id: a65fa2098f6b6f1704154e0509e2f5423f9679eb --- cachelib/external/fbthrift | 2 +- cachelib/external/fizz | 2 +- cachelib/external/folly | 2 +- cachelib/external/wangle | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/cachelib/external/fbthrift b/cachelib/external/fbthrift index 90fddab67..cbc3de581 160000 --- a/cachelib/external/fbthrift +++ b/cachelib/external/fbthrift @@ -1 +1 @@ -Subproject commit 90fddab67b2a40ec0edda28fd014402258726eef +Subproject commit cbc3de581fdf36ba474b0c135b9e785e504f1c1e diff --git a/cachelib/external/fizz b/cachelib/external/fizz index 2da27e939..287625bd6 160000 --- a/cachelib/external/fizz +++ b/cachelib/external/fizz @@ -1 +1 @@ -Subproject commit 2da27e939de1aa4eeecc6bc8d3a32844a75bd42b +Subproject commit 287625bd6676b812e75ad0b088a61f72b4c9e681 diff --git a/cachelib/external/folly b/cachelib/external/folly index fdb333b2b..ce2b95715 160000 --- a/cachelib/external/folly +++ b/cachelib/external/folly @@ -1 +1 @@ -Subproject commit fdb333b2bb6dd9424d19c4e12385726ff87e60d4 +Subproject commit ce2b95715de229fcb51bd97410469a3ad4d2bfb2 diff --git a/cachelib/external/wangle b/cachelib/external/wangle index cb61ed175..44690e789 160000 --- a/cachelib/external/wangle +++ b/cachelib/external/wangle @@ -1 +1 @@ -Subproject commit cb61ed1759c692a4c69b85df7cb983ee920e91cf +Subproject commit 44690e7894842a7127245837b69627d4b964aabd From 6357906c331954f34eccc5b870a7984da79f27f4 Mon Sep 17 00:00:00 2001 From: Daniel Wong Date: Thu, 23 Feb 2023 20:29:55 -0800 Subject: [PATCH 13/44] Pin fmt version at 8.0.1 (#196) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Summary: Change build script to pin fmt version to same as folly's to minimize future breaks. Pull Request resolved: https://github.com/facebook/CacheLib/pull/196 Test Plan: Built successfully on a fresh clone of CacheLib. (Also had to change `external_git_branch=dev` for zstd to deal with the cmake/zstd issue in https://github.com/facebook/CacheLib/issues/194, but that should resolve when gets merged into release) **Context:** OSS build broke between 3-5 Jan 2023, likely due to changes in folly. While switching to v9.1.0 or 9.0.0 fixes the issue at hand, it seems sensible to match folly, which specifies fmt v8.0.1: https://github.com/facebook/folly/blob/main/build/fbcode_builder/manifests/fmt > https://github.com/facebook/CacheLib/issues/62 agordon: For the other packages, you'll notice we do use a specific git tag or branch… I notice `fmt` is an exception - not pinned to a specific git tag or revision - likely an omission that can be fixed. Related CacheLib issues: https://github.com/facebook/CacheLib/issues/186, https://github.com/facebook/CacheLib/issues/189, https://github.com/facebook/CacheLib/issues/107, https://github.com/facebook/CacheLib/issues/97, https://github.com/facebook/CacheLib/issues/62 Possibly related CacheLib commit: 67cc11ad6f5fb7b1e1948513292ef00edee34f5e Last working (Jan 3): https://github.com/facebook/CacheLib/actions/runs/3826992478 First failed (Jan 5): https://github.com/facebook/CacheLib/actions/runs/3844002307/jobs/6546742348 Error: `error: static assertion failed: Cannot format an argument. To make type T formattable provide a formatter specialization: https://fmt.dev/latest/api.html#udt` Reviewed By: therealgymmy Differential Revision: D43517927 Pulled By: jiayuebao fbshipit-source-id: 2d28791f7804d862b646263b96b10b835f843d8c --- contrib/build-package.sh | 1 + 1 file changed, 1 insertion(+) diff --git a/contrib/build-package.sh b/contrib/build-package.sh index ff487967c..6e7acac5c 100755 --- a/contrib/build-package.sh +++ b/contrib/build-package.sh @@ -160,6 +160,7 @@ case "$1" in REPODIR=cachelib/external/$NAME SRCDIR=$REPODIR external_git_clone=yes + external_git_tag="8.0.1" cmake_custom_params="-DBUILD_SHARED_LIBS=ON" if test "$build_tests" = "yes" ; then cmake_custom_params="$cmake_custom_params -DFMT_TEST=YES" From a9257379ab72ef2733f35e814c425d12785f0ea9 Mon Sep 17 00:00:00 2001 From: Jaesoo Lee Date: Fri, 24 Feb 2023 19:09:53 -0800 Subject: [PATCH 14/44] KVReplayGenerator: parse GET_LEASE and SET_LEASE operations Summary: Memcached's WSA logger will now emits GET_LEASE and SET_LEASE operations as well. This changes makes the cachebench treats those as GET and SET, respectively, for compatibility. Reviewed By: therealgymmy Differential Revision: D43336316 fbshipit-source-id: c9b842d567b9fb2128b822bd429f5dce30b378da --- cachelib/cachebench/workload/KVReplayGenerator.h | 6 +++--- .../workload/tests/KVReplayGeneratorTest.cpp | 15 +++++++++++++++ 2 files changed, 18 insertions(+), 3 deletions(-) diff --git a/cachelib/cachebench/workload/KVReplayGenerator.h b/cachelib/cachebench/workload/KVReplayGenerator.h index 4b1297008..a9124e2bd 100644 --- a/cachelib/cachebench/workload/KVReplayGenerator.h +++ b/cachelib/cachebench/workload/KVReplayGenerator.h @@ -230,10 +230,10 @@ inline bool KVReplayGenerator::parseRequest(const std::string& line, // Set op const auto& op = fields[SampleFields::OP]; - // TODO only memcache optypes are supported - if (!op.compare("GET")) { + // TODO implement GET_LEASE and SET_LEASE emulations + if (!op.compare("GET") || !op.compare("GET_LEASE")) { req->req_.setOp(OpType::kGet); - } else if (!op.compare("SET")) { + } else if (!op.compare("SET") || !op.compare("SET_LEASE")) { req->req_.setOp(OpType::kSet); } else if (!op.compare("DELETE")) { req->req_.setOp(OpType::kDel); diff --git a/cachelib/cachebench/workload/tests/KVReplayGeneratorTest.cpp b/cachelib/cachebench/workload/tests/KVReplayGeneratorTest.cpp index 72a55a402..16e4e5206 100644 --- a/cachelib/cachebench/workload/tests/KVReplayGeneratorTest.cpp +++ b/cachelib/cachebench/workload/tests/KVReplayGeneratorTest.cpp @@ -56,6 +56,18 @@ struct TraceEntry { size_t expKeySize = std::max(keySize_, reqKey.size()); expKeySize = std::min(expKeySize, 256); ASSERT_EQ(reqKey.size(), expKeySize); + ASSERT_EQ(req.req_.getOp(), getOpType()); + } + + OpType getOpType() { + if (!op_.compare("GET") || !op_.compare("GET_LEASE")) { + return OpType::kGet; + } else if (!op_.compare("SET") || !op_.compare("SET_LEASE")) { + return OpType::kSet; + } else if (!op_.compare("DELETE")) { + return OpType::kDel; + } + return OpType::kSize; } std::string key_; @@ -86,8 +98,11 @@ TEST(KVReplayGeneratorTest, BasicFormat) { // ,,,,, {7, "GET", 0, 2, std::nullopt, true}, {7, "GET", 0, 2, 50, true}, + {7, "GET_LEASE", 0, 2, 50, true}, {20, "SET", 100, 35, std::nullopt, true}, {20, "SET", 100, 35, 3600, true}, + {20, "SAT", 100, 35, 3600, false}, // invalid op name + {20, "SET_LEASE", 100, 35, 3600, true}, {7, "GET", 0, 0, std::nullopt, false}, // invalid op count {7, "GET", 0, 0, 600, false}, // invalid op count {1024, "SET", 100, 35, 300, true}, // key truncated From f7e13a4bf723a6e128e2707accc12c8c3594bfa2 Mon Sep 17 00:00:00 2001 From: Jiayue Bao Date: Mon, 27 Feb 2023 08:41:42 -0800 Subject: [PATCH 15/44] Update @braintree/sanitize-url version Summary: update to >6.0.1 version Reviewed By: antonk52 Differential Revision: D43575577 fbshipit-source-id: 7143da212ab6f124bffdfdaf3be29ff3ab986ffb --- website/package.json | 3 ++- website/yarn.lock | 8 ++++---- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/website/package.json b/website/package.json index 8c58fda9a..ac9801eee 100644 --- a/website/package.json +++ b/website/package.json @@ -43,7 +43,8 @@ "ansi-html": "0.0.8", "ua-parser-js": "^1.0.33", "eta": "^2.0.0", - "http-cache-semantics": "^4.1.1" + "http-cache-semantics": "^4.1.1", + "@braintree/sanitize-url": "^6.0.1" }, "browserslist": { "production": [ diff --git a/website/yarn.lock b/website/yarn.lock index 19f12eb0d..51e55da8c 100644 --- a/website/yarn.lock +++ b/website/yarn.lock @@ -1809,10 +1809,10 @@ "@babel/helper-validator-identifier" "^7.18.6" to-fast-properties "^2.0.0" -"@braintree/sanitize-url@^6.0.0": - version "6.0.0" - resolved "https://registry.yarnpkg.com/@braintree/sanitize-url/-/sanitize-url-6.0.0.tgz#fe364f025ba74f6de6c837a84ef44bdb1d61e68f" - integrity sha512-mgmE7XBYY/21erpzhexk4Cj1cyTQ9LzvnTxtzM17BJ7ERMNE6W72mQRo0I1Ud8eFJ+RVVIcBNhLFZ3GX4XFz5w== +"@braintree/sanitize-url@^6.0.0", "@braintree/sanitize-url@^6.0.1": + version "6.0.2" + resolved "https://registry.yarnpkg.com/@braintree/sanitize-url/-/sanitize-url-6.0.2.tgz#6110f918d273fe2af8ea1c4398a88774bb9fc12f" + integrity sha512-Tbsj02wXCbqGmzdnXNk0SOF19ChhRU70BsroIi4Pm6Ehp56in6vch94mfbdQ17DozxkL3BAVjbZ4Qc1a0HFRAg== "@colors/colors@1.5.0": version "1.5.0" From 70ff91f9558fb17e67003e272d2b75df5317d6bc Mon Sep 17 00:00:00 2001 From: Daniel Wong Date: Mon, 27 Feb 2023 08:47:26 -0800 Subject: [PATCH 16/44] Add missing numa deps for fedora, rocky, arch (#197) Summary: Fix OSS builds by adding numa deps to build files. Currently some fail on missing `numa.h`. Context: https://github.com/facebook/CacheLib/issues/161 added the dependencies to the centOS, debian, and ubuntu18 build files. The PR was opened in Sep 2022 but only landed in Dec 2022, and so probably missed out on the fedora, rocky and arch build files which were added in-between those dates. Having had those build actions run on PRs would have caught this (currently, they are only scheduled.) Pull Request resolved: https://github.com/facebook/CacheLib/pull/197 Test Plan: Github Actions builds (ideally, https://github.com/facebook/CacheLib/issues/198 would be landed first.) I've checked that those packages exist for the respective repositories but didn't run them myself. Reviewed By: jaesoo-fb Differential Revision: D43587970 Pulled By: jiayuebao fbshipit-source-id: 8c59e48528042350e576a45ffc3bf2520699f5a9 --- contrib/prerequisites-arch.sh | 1 + contrib/prerequisites-fedora32.sh | 1 + contrib/prerequisites-fedora34.sh | 3 ++- contrib/prerequisites-rocky9.sh | 3 ++- 4 files changed, 6 insertions(+), 2 deletions(-) diff --git a/contrib/prerequisites-arch.sh b/contrib/prerequisites-arch.sh index 85a8656f7..249f6c808 100755 --- a/contrib/prerequisites-arch.sh +++ b/contrib/prerequisites-arch.sh @@ -19,4 +19,5 @@ sudo pacman -S --needed --noconfirm cmake \ boost \ double-conversion \ libdwarf \ + numactl \ libsodium diff --git a/contrib/prerequisites-fedora32.sh b/contrib/prerequisites-fedora32.sh index 235d6c1a8..942cac047 100755 --- a/contrib/prerequisites-fedora32.sh +++ b/contrib/prerequisites-fedora32.sh @@ -21,6 +21,7 @@ sudo dnf -y install bison flex patch bzip2 cmake \ zlib-devel lz4-devel xz-devel bzip2-devel \ jemalloc-devel snappy-devel libsodium-devel libdwarf-devel libaio-devel \ gmock-devel gflags-devel gtest gtest-devel \ + numactl-devel \ fmt fmt-devel # DO NOT INSTALL glog-devel - need to build from source for the glog-*.cmake files diff --git a/contrib/prerequisites-fedora34.sh b/contrib/prerequisites-fedora34.sh index 7e45c8740..c7182cc51 100755 --- a/contrib/prerequisites-fedora34.sh +++ b/contrib/prerequisites-fedora34.sh @@ -19,4 +19,5 @@ sudo dnf -y install bison flex patch bzip2 cmake \ double-conversion double-conversion-devel make g++ \ boost-devel libevent-devel openssl-devel libunwind-devel \ zlib-devel lz4-devel xz-devel bzip2-devel \ - jemalloc-devel snappy-devel libsodium-devel libdwarf-devel libaio-devel + jemalloc-devel snappy-devel libsodium-devel libdwarf-devel libaio-devel \ + numactl-devel diff --git a/contrib/prerequisites-rocky9.sh b/contrib/prerequisites-rocky9.sh index bec5b8201..06720aba2 100755 --- a/contrib/prerequisites-rocky9.sh +++ b/contrib/prerequisites-rocky9.sh @@ -38,7 +38,8 @@ sudo dnf install -y \ jemalloc-devel \ libsodium-devel \ libaio-devel \ - binutils-devel + binutils-devel \ + numactl-devel sudo dnf install -y \ From 6ad7a318b43de25ba41067c3d6a851d5a60d1633 Mon Sep 17 00:00:00 2001 From: generatedunixname89002005287564 Date: Mon, 27 Feb 2023 10:31:09 -0800 Subject: [PATCH 17/44] fbcode/cachelib/allocator/datastruct/serialize Reviewed By: avalonalex Differential Revision: D43616310 fbshipit-source-id: 3367ad01ba31e5dc561d63a4f3b9746170e64912 --- .../allocator/datastruct/serialize/objects.thrift | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/cachelib/allocator/datastruct/serialize/objects.thrift b/cachelib/allocator/datastruct/serialize/objects.thrift index bd2c8b79b..223b804e5 100644 --- a/cachelib/allocator/datastruct/serialize/objects.thrift +++ b/cachelib/allocator/datastruct/serialize/objects.thrift @@ -22,17 +22,17 @@ namespace cpp2 facebook.cachelib.serialization // Saved state for an SList struct SListObject { - 2: required i64 size, - 3: required i64 compressedHead, // Pointer to the head element + 2: required i64 size; + 3: required i64 compressedHead; // Pointer to the head element // TODO(bwatling): remove the default value and clean up SList::SList() once // we can rely on 'compressedTail' always being valid. - 4: i64 compressedTail = -1, // Pointer to the tail element + 4: i64 compressedTail = -1; // Pointer to the tail element } struct DListObject { - 1: required i64 compressedHead, - 2: required i64 compressedTail, - 3: required i64 size, + 1: required i64 compressedHead; + 2: required i64 compressedTail; + 3: required i64 size; } struct MultiDListObject { From df5b9f6ef35c55e432b6713c52397a03dd19c34c Mon Sep 17 00:00:00 2001 From: Daniel Wong Date: Mon, 27 Feb 2023 16:21:05 -0800 Subject: [PATCH 18/44] Run Github Actions on pull requests (#198) Summary: Run GitHub action builds on every pull request, in addition to currently daily scheduled runs. Benefit: avoid accidentally breaking other OS builds. This would have caught https://github.com/facebook/CacheLib/issues/197. This triggers whenever PRs are opened or when commits are added to a PR. Frequency of PRs (total of 76 PRs over last 18 months since CacheLib was open-sourced) is much less than frequency of daily scheduled builds, so this shouldn't add too many builds overall. Pull Request resolved: https://github.com/facebook/CacheLib/pull/198 Reviewed By: therealgymmy Differential Revision: D43625609 Pulled By: jiayuebao fbshipit-source-id: 1572f6da32584ce6a1983d5e64afedf17ff17457 --- .github/workflows/build-cachelib-centos-8-1.yml | 1 + .github/workflows/build-cachelib-centos-8-5.yml | 1 + .github/workflows/build-cachelib-debian-10.yml | 1 + .github/workflows/build-cachelib-fedora-36.yml | 1 + .github/workflows/build-cachelib-rockylinux-8.yml | 1 + .github/workflows/build-cachelib-rockylinux-9.yml | 1 + .github/workflows/build-cachelib-ubuntu-18.yml | 1 + .github/workflows/build-cachelib-ubuntu-20.yml | 1 + .github/workflows/build-cachelib-ubuntu-22.yml | 1 + 9 files changed, 9 insertions(+) diff --git a/.github/workflows/build-cachelib-centos-8-1.yml b/.github/workflows/build-cachelib-centos-8-1.yml index 5eb1090b0..3983e0c78 100644 --- a/.github/workflows/build-cachelib-centos-8-1.yml +++ b/.github/workflows/build-cachelib-centos-8-1.yml @@ -14,6 +14,7 @@ name: build-cachelib-centos-8-1 on: # push: + pull_request: schedule: - cron: '0 11 * * 1,3,5' jobs: diff --git a/.github/workflows/build-cachelib-centos-8-5.yml b/.github/workflows/build-cachelib-centos-8-5.yml index 3ffee3776..4e6c2d12e 100644 --- a/.github/workflows/build-cachelib-centos-8-5.yml +++ b/.github/workflows/build-cachelib-centos-8-5.yml @@ -14,6 +14,7 @@ name: build-cachelib-centos-8.5 on: # push: + pull_request: schedule: - cron: '0 9 * * *' jobs: diff --git a/.github/workflows/build-cachelib-debian-10.yml b/.github/workflows/build-cachelib-debian-10.yml index c7c67e072..56fb57629 100644 --- a/.github/workflows/build-cachelib-debian-10.yml +++ b/.github/workflows/build-cachelib-debian-10.yml @@ -14,6 +14,7 @@ name: build-cachelib-debian-10 on: # push: + pull_request: schedule: - cron: '0 13 * * *' jobs: diff --git a/.github/workflows/build-cachelib-fedora-36.yml b/.github/workflows/build-cachelib-fedora-36.yml index 216dbf584..f8c042440 100644 --- a/.github/workflows/build-cachelib-fedora-36.yml +++ b/.github/workflows/build-cachelib-fedora-36.yml @@ -14,6 +14,7 @@ name: build-cachelib-fedora-36 on: # push: + pull_request: schedule: - cron: '0 19 * * *' jobs: diff --git a/.github/workflows/build-cachelib-rockylinux-8.yml b/.github/workflows/build-cachelib-rockylinux-8.yml index 879dc2756..c8af12327 100644 --- a/.github/workflows/build-cachelib-rockylinux-8.yml +++ b/.github/workflows/build-cachelib-rockylinux-8.yml @@ -14,6 +14,7 @@ name: build-cachelib-rockylinux-8.6 on: # push: + pull_request: schedule: - cron: '0 15 * * 2,4,6' jobs: diff --git a/.github/workflows/build-cachelib-rockylinux-9.yml b/.github/workflows/build-cachelib-rockylinux-9.yml index f6a86d75a..e26eac6ff 100644 --- a/.github/workflows/build-cachelib-rockylinux-9.yml +++ b/.github/workflows/build-cachelib-rockylinux-9.yml @@ -14,6 +14,7 @@ name: build-cachelib-rockylinux-9.0 on: # push: + pull_request: schedule: - cron: '0 17 * * *' jobs: diff --git a/.github/workflows/build-cachelib-ubuntu-18.yml b/.github/workflows/build-cachelib-ubuntu-18.yml index fad34c089..ad068278a 100644 --- a/.github/workflows/build-cachelib-ubuntu-18.yml +++ b/.github/workflows/build-cachelib-ubuntu-18.yml @@ -19,6 +19,7 @@ name: build-cachelib-ubuntu-18 on: # push: + pull_request: schedule: - cron: '0 5 * * 2,4,6' jobs: diff --git a/.github/workflows/build-cachelib-ubuntu-20.yml b/.github/workflows/build-cachelib-ubuntu-20.yml index 35a3f507e..a8380fdb9 100644 --- a/.github/workflows/build-cachelib-ubuntu-20.yml +++ b/.github/workflows/build-cachelib-ubuntu-20.yml @@ -15,6 +15,7 @@ name: build-cachelib-ubuntu-20 on: # push: + pull_request: schedule: - cron: '0 5 * * 1,3,5' jobs: diff --git a/.github/workflows/build-cachelib-ubuntu-22.yml b/.github/workflows/build-cachelib-ubuntu-22.yml index b4374a5b9..4db194431 100644 --- a/.github/workflows/build-cachelib-ubuntu-22.yml +++ b/.github/workflows/build-cachelib-ubuntu-22.yml @@ -15,6 +15,7 @@ name: build-cachelib-ubuntu-22 on: # push: + pull_request: schedule: - cron: '0 7 * * *' jobs: From e8151adb8bb1fa4f628232c35cab06cad2ffd052 Mon Sep 17 00:00:00 2001 From: Jiayue Bao Date: Tue, 28 Feb 2023 10:25:27 -0800 Subject: [PATCH 19/44] Add a custom deleter class to access the Item Handle Summary: Add a custom deleter class that stores a `handle`. This allows object-cache to access the Item Handle via a shared_ptr. Both size-awareness feature and getting/updating object's TTL require that. Deleter class is marked as private because we don't want to expose `Handle` to object-cache users. Reviewed By: therealgymmy, jaesoo-fb Differential Revision: D42503594 fbshipit-source-id: 16ac14e6a84a1cfa80a3c145d440790002734a34 --- .../experimental/objcache2/ObjectCache-inl.h | 18 ++++----- cachelib/experimental/objcache2/ObjectCache.h | 37 +++++++++++++++++++ 2 files changed, 46 insertions(+), 9 deletions(-) diff --git a/cachelib/experimental/objcache2/ObjectCache-inl.h b/cachelib/experimental/objcache2/ObjectCache-inl.h index 70cfc445b..9f1b91631 100644 --- a/cachelib/experimental/objcache2/ObjectCache-inl.h +++ b/cachelib/experimental/objcache2/ObjectCache-inl.h @@ -128,8 +128,8 @@ std::shared_ptr ObjectCache::find(folly::StringPiece key) { succL1Lookups_.inc(); auto ptr = found->template getMemoryAs()->objectPtr; - // Just release the handle. Cache destorys object when all handles released. - auto deleter = [h = std::move(found)](const T*) {}; + // Use custom deleter + auto deleter = Deleter(std::move(found)); return std::shared_ptr(reinterpret_cast(ptr), std::move(deleter)); } @@ -146,8 +146,8 @@ std::shared_ptr ObjectCache::findToWrite( succL1Lookups_.inc(); auto ptr = found->template getMemoryAs()->objectPtr; - // Just release the handle. Cache destorys object when all handles released. - auto deleter = [h = std::move(found)](T*) {}; + // Use custom deleter + auto deleter = Deleter(std::move(found)); return std::shared_ptr(reinterpret_cast(ptr), std::move(deleter)); } @@ -200,9 +200,6 @@ ObjectCache::insertOrReplace(folly::StringPiece key, std::move(deleter)); } - // Just release the handle. Cache destorys object when all handles released. - auto deleter = [h = std::move(handle)](T*) {}; - // update total object size if (config_.objectSizeTrackingEnabled) { totalObjectSizeBytes_.fetch_add(objectSize, std::memory_order_relaxed); @@ -210,6 +207,9 @@ ObjectCache::insertOrReplace(folly::StringPiece key, // Release the object as it has been successfully inserted to the cache. object.release(); + + // Use custom deleter + auto deleter = Deleter(std::move(handle)); return {AllocStatus::kSuccess, std::shared_ptr(ptr, std::move(deleter)), replacedPtr}; } @@ -256,8 +256,8 @@ ObjectCache::insert(folly::StringPiece key, object.release(); } - // Just release the handle. Cache destorys object when all handles released. - auto deleter = [h = std::move(handle)](T*) {}; + // Use custom deleter + auto deleter = Deleter(std::move(handle)); return {success ? AllocStatus::kSuccess : AllocStatus::kKeyAlreadyExists, std::shared_ptr(ptr, std::move(deleter))}; } diff --git a/cachelib/experimental/objcache2/ObjectCache.h b/cachelib/experimental/objcache2/ObjectCache.h index f4cd2a9bb..5f8aab85a 100644 --- a/cachelib/experimental/objcache2/ObjectCache.h +++ b/cachelib/experimental/objcache2/ObjectCache.h @@ -94,6 +94,43 @@ class ObjectCache : public ObjectCacheBase { // make constructor private, but constructable by std::make_unique struct InternalConstructor {}; + template + class Deleter { + public: + using ReadHandle = typename AllocatorT::ReadHandle; + using WriteHandle = typename AllocatorT::WriteHandle; + using Handle = std::variant; + + explicit Deleter(typename AllocatorT::ReadHandle&& hdl) + : hdl_(std::move(hdl)) {} + explicit Deleter(typename AllocatorT::WriteHandle&& hdl) + : hdl_(std::move(hdl)) {} + + void operator()(T*) { + // Just release the handle. + // Cache destorys object when all handles released. + std::holds_alternative(hdl_) + ? std::get(hdl_).reset() + : std::get(hdl_).reset(); + } + + WriteHandle& getWriteHandleRef() { + if (std::holds_alternative(hdl_)) { + hdl_ = std::move(std::get(hdl_)).toWriteHandle(); + } + return std::get(hdl_); + } + + ReadHandle& getReadHandleRef() { + return std::holds_alternative(hdl_) + ? std::get(hdl_) + : std::get(hdl_); + } + + private: + Handle hdl_; + }; + public: using ItemDestructor = std::function; using Key = KAllocation::Key; From 982781860c0ccc6c61cde83f33e72341d87e72d7 Mon Sep 17 00:00:00 2001 From: Jaesoo Lee Date: Tue, 28 Feb 2023 15:59:14 -0800 Subject: [PATCH 20/44] fix flaky tests in NvmCacheTests Summary: This change fixes following flaky tests in NvmCacheTests. * NvmCacheTest.Delete * NvmCacheTest.NvmEvicted * NvmCacheTest.EvictToNvmGetCheckCtime The root cause of the failures are essentially the same as D42443647 (https://github.com/facebook/CacheLib/commit/5e7ff9ab28cc00c74b19b176885d6af2e3d27d60) which fixed the problem for NvmCacheTest.EvictToNvmGet; we are inserting enough items that could be spilled to NVM cache, where the NvmCache::put() can be dropped and the item is evicted completely when the delete operations (and tombstones) issued as part of the insertion are still outstanding. In order to fix the problem, this change flushes the NVM cache periodically during the insertions. Also, since this could cause more regions are used, the size of NVM cache needs to be increased. This change bumps the default size of NVM cache to 200MB (previous 100MB). Also, the size of persist storage used in the test PersistenceCache has been bumped by 100MB accordingly, i.e., from 400MB to 500MB. This change addresses the github issue https://github.com/facebook/CacheLib/issues/169 Reviewed By: therealgymmy Differential Revision: D43592888 fbshipit-source-id: f0968884eb39fb5728b59129e98345df3240f01e --- .../allocator/nvmcache/tests/NvmCacheTests.cpp | 16 ++++++++++++++++ cachelib/allocator/tests/NvmTestUtils.h | 2 +- cachelib/persistence/tests/PersistenceCache.h | 2 +- 3 files changed, 18 insertions(+), 2 deletions(-) diff --git a/cachelib/allocator/nvmcache/tests/NvmCacheTests.cpp b/cachelib/allocator/nvmcache/tests/NvmCacheTests.cpp index 7355627fe..ec74c5198 100644 --- a/cachelib/allocator/nvmcache/tests/NvmCacheTests.cpp +++ b/cachelib/allocator/nvmcache/tests/NvmCacheTests.cpp @@ -245,7 +245,13 @@ TEST_F(NvmCacheTest, EvictToNvmGetCheckCtime) { ASSERT_NE(nullptr, it); cache_->insertOrReplace(it); keyToCtime.insert({key, it->getCreationTime()}); + // Avoid any nvm eviction being dropped due to the race with still + // outstanding remove operation for insertion + if (i % 100 == 0) { + nvm.flushNvmCache(); + } } + nvm.flushNvmCache(); const auto nEvictions = this->evictionCount() - evictBefore; ASSERT_LT(0, nEvictions); @@ -331,6 +337,11 @@ TEST_F(NvmCacheTest, Delete) { auto it = nvm.allocate(pid, key, 15 * 1024); ASSERT_NE(nullptr, it); nvm.insertOrReplace(it); + // Avoid any nvm eviction being dropped due to the race with still + // outstanding remove operation for insertion + if (i % 100 == 0) { + nvm.flushNvmCache(); + } } nvm.flushNvmCache(); @@ -533,6 +544,11 @@ TEST_F(NvmCacheTest, NvmEvicted) { auto it = nvm.allocate(pid, key, allocSize); ASSERT_NE(nullptr, it); nvm.insertOrReplace(it); + // Avoid any nvm eviction being dropped due to the race with still + // outstanding remove operation for insertion + if (i % 100 == 0) { + nvm.flushNvmCache(); + } } nvm.flushNvmCache(); diff --git a/cachelib/allocator/tests/NvmTestUtils.h b/cachelib/allocator/tests/NvmTestUtils.h index 6d6242aad..cad96c41d 100644 --- a/cachelib/allocator/tests/NvmTestUtils.h +++ b/cachelib/allocator/tests/NvmTestUtils.h @@ -27,7 +27,7 @@ namespace utils { using NavyConfig = navy::NavyConfig; inline NavyConfig getNvmTestConfig(const std::string& cacheDir) { NavyConfig config{}; - config.setSimpleFile(cacheDir + "/navy", 100 * 1024ULL * 1024ULL); + config.setSimpleFile(cacheDir + "/navy", 200 * 1024ULL * 1024ULL); config.setDeviceMetadataSize(4 * 1024 * 1024); config.setBlockSize(1024); config.setNavyReqOrderingShards(10); diff --git a/cachelib/persistence/tests/PersistenceCache.h b/cachelib/persistence/tests/PersistenceCache.h index 5400b4d4e..1db5b5fc8 100644 --- a/cachelib/persistence/tests/PersistenceCache.h +++ b/cachelib/persistence/tests/PersistenceCache.h @@ -213,7 +213,7 @@ class PersistenceCache { public: const uint32_t kNumKeys = 1024 * 1024; // 1 million const size_t kCacheSize = 100 * kNumKeys; // 100MB - const size_t kCapacity = 4 * kCacheSize; // 400MB + const size_t kCapacity = 5 * kCacheSize; // 500MB std::unique_ptr buffer_; std::string cacheDir_; From 9447a8acfb84c70c93333063bbee22d5e748a3e1 Mon Sep 17 00:00:00 2001 From: Darryl Gardner Date: Tue, 28 Feb 2023 17:07:02 -0800 Subject: [PATCH 21/44] Added PM9A3 support for Cachebench Write Bytes Calculations Summary: The Samsung PM9A3 does not report samsung in the model number so I added the specific model number to the vendorMap. Reviewed By: jaesoo-fb Differential Revision: D43676582 fbshipit-source-id: 6df19c40dd9da9563b75aa5847a1d1f9eb6a9aef --- cachelib/cachebench/util/NandWrites.cpp | 1 + .../cachebench/util/tests/NandWritesTest.cpp | 90 +++++++++++++++++++ 2 files changed, 91 insertions(+) diff --git a/cachelib/cachebench/util/NandWrites.cpp b/cachelib/cachebench/util/NandWrites.cpp index ae82aca65..370ddfa2b 100644 --- a/cachelib/cachebench/util/NandWrites.cpp +++ b/cachelib/cachebench/util/NandWrites.cpp @@ -400,6 +400,7 @@ uint64_t nandWriteBytes(const folly::StringPiece& deviceName, const folly::StringPiece&)>> vendorMap{{"samsung", samsungWriteBytes}, {"mz1lb960hbjr-", samsungWriteBytes}, + {"mzol23t8hcls-", samsungWriteBytes}, // The Samsung PM983a doesn't include Samsung in the model // number at this time, but it's a Samsung device. {"liteon", liteonWriteBytes}, diff --git a/cachelib/cachebench/util/tests/NandWritesTest.cpp b/cachelib/cachebench/util/tests/NandWritesTest.cpp index 0002e8a83..af09593f4 100644 --- a/cachelib/cachebench/util/tests/NandWritesTest.cpp +++ b/cachelib/cachebench/util/tests/NandWritesTest.cpp @@ -240,6 +240,96 @@ TEST_F(NandWritesTest, nandWriteBytes_handlesSamsungPM983aDevice) { EXPECT_EQ(nandWriteBytes("nvme1n1", kNvmePath, mockFactory_), 35061362294784); } +TEST_F(NandWritesTest, nandWriteBytes_handlesSamsungPM9A3Device) { + constexpr auto& kListOutput = R"EOF({ + "Devices" : [ + { + "DevicePath" : "/dev/nvme0n1", + "Firmware" : "P1FB007", + "Index" : 0, + "NameSpace" : 1, + "ModelNumber" : "MTFDHBA512TCK", + "ProductName" : "Non-Volatile memory controller: Micron Technology Inc Device 0x5410", + "SerialNumber" : " 21062E6B8061", + "UsedBytes" : 512110190592, + "MaximumLBA" : 1000215216, + "PhysicalSize" : 512110190592, + "SectorSize" : 512 + }, + { + "DevicePath" : "/dev/nvme1n1", + "Firmware" : "GDA82F2Q", + "Index" : 1, + "NameSpace" : 1, + "ModelNumber" : "MZOL23T8HCLS-00AFB", + "ProductName" : "Unknown device", + "SerialNumber" : "S5X9NG0T116005", + "UsedBytes" : 104910848, + "MaximumLBA" : 918149526, + "PhysicalSize" : 3760740458496, + "SectorSize" : 4096 + }, + { + "DevicePath" : "/dev/nvme2n1", + "Firmware" : "GDA82F2Q", + "Index" : 2, + "NameSpace" : 1, + "ModelNumber" : "MZOL23T8HCLS-00AFB", + "ProductName" : "Unknown device", + "SerialNumber" : "S5X9NG0T116027", + "UsedBytes" : 0, + "MaximumLBA" : 918149526, + "PhysicalSize" : 3760740458496, + "SectorSize" : 4096 + } + ] +})EOF"; + + constexpr auto& kSmartLogOutput = R"EOF( +[015:000] PhysicallyWrittenBytes : 241393664 +[031:016] Physically Read Bytes : 106217472 +[037:032] Bad NAND Block Count (Raw Value) : 0 +[039:038] Bad NAND Block Count (Normalized Value) : 100 +[047:040] Uncorrectable Read Error Count : 0 +[055:048] Soft ECC Error Count : 0 +[059:056] SSD End to end Correction Count (Detected Errors) : 0 +[063:060] SSD End to end Correction Count (Corrected Errors): 0 +[064:064] System Data Percentage Used : 0 +[068:065] User Data Erase Count (Min) : 0 +[072:069] User Data Erase Count (Max) : 1 +[080:073] Refresh Count : 0 +[086:081] Program Fail Count (Raw Value) : 0 +[088:087] Program Fail Count (Normalized Value) : 100 +[094:089] User Data Erase Fail Count (Raw Value) : 0 +[096:095] User Data Erase Fail Count (Normalized Value) : 100 +[102:097] System Area Erase Fail Count (Raw Value) : 0 +[104:103] System Area Erase Fail Count (Normalized value) : 100 +[105:105] Thermal Throttling Status : 0 +[106:106] Thermal Throttling Count : 0 +[108:107] PHY Error Count : 0 +[110:109] Bad DLLP Count : 0 +[112:111] Bad TLP Count : 0 +[114:113] Reserved : 0 +[118:115] Incomplete Shutdowns : 0 +[119:119] % Free Blocks : 96 +[121:120] PCIe Correctable Error Count (RTS) : 0 +[123:122] PCIe Correctable Error Count (RRS) : 0 +[131:124] XOR Recovery Count : 0 +[137:132] Bad System NAND block count (Raw Value) : 0 +[139:138] Bad System NAND block count (Normalized Value) : 100 +[141:140] Capacitor Health : 163 +[157:142] Endurance Estimate : 28862181 +[165:158] Security Version Number : 4294967296 +[167:166] Log Page Version : 1 +)EOF"; + + mockFactory_->expectedCommands( + {{{kNvmePath, "list", "-o", "json"}, kListOutput}, + {{kNvmePath, "samsung", "vs-smart-add-log", "/dev/nvme1n1"}, + kSmartLogOutput}}); + EXPECT_EQ(nandWriteBytes("nvme1n1", kNvmePath, mockFactory_), 241393664); +} + TEST_F(NandWritesTest, nandWriteBytes_handlesSeagateDevice) { constexpr auto& kListOutput = R"EOF({ "Devices" : [ From 293118bfed1d63726ab24e3bee39962da268ac44 Mon Sep 17 00:00:00 2001 From: Daniel Wong Date: Wed, 1 Mar 2023 09:47:43 -0800 Subject: [PATCH 22/44] Fix XDCHECK regression in lambda for gcc-8.x (#201) Summary: XDCHECK (or XCHECK) fails here for gcc-8.x in a lambda, so we move it outside. This occurs in CacheBench's AsyncCacheStressor.h. ``` /__w/CacheLib/CacheLib/cachelib/../cachelib/cachebench/runner/AsyncCacheStressor.h:306:7: internal compiler error: in cp_build_addr_expr_1, at cp/typeck.c:5965 ``` This line compiled fine with 8.5 before Jan 2023 so I suspect a regression in folly or other external library. It still compiles fine with gcc-7.5, 9.4, and 11.3.1. Possibly related commits: https://github.com/facebook/folly/commit/1aafad45f316896a7504396f421dacd6c10d7d5f and https://github.com/facebook/folly/commit/e6d09f66b9fc473bc108361d4c8dce8f29f7bcaf Line of gcc-8.5 that it fails on: https://github.com/gcc-mirror/gcc/blob/releases/gcc-8.5.0/gcc/cp/typeck.c#L5965 The version that isn't in a lambda compiles just fine: https://github.com/facebook/CacheLib/blob/df5b9f6ef35c55e432b6713c52397a03dd19c34c/cachelib/cachebench/runner/CacheStressor.h#L399 Pull Request resolved: https://github.com/facebook/CacheLib/pull/201 Test Plan: GitHub actions. Built fine on my fork with CentOS 8.5/gcc-8.5. This issue currently causes 3 builds that use gcc-8.5 to fail (2 CentOS and RockyLinux-8.6) and 1 build using gcc-8.3 (Debian). Reviewed By: therealgymmy Differential Revision: D43681854 Pulled By: jaesoo-fb fbshipit-source-id: f3a65aefedcd98a26a80bb6ad009ad0d64e2395b --- cachelib/cachebench/runner/AsyncCacheStressor.h | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/cachelib/cachebench/runner/AsyncCacheStressor.h b/cachelib/cachebench/runner/AsyncCacheStressor.h index 5b50db43b..830b50379 100644 --- a/cachelib/cachebench/runner/AsyncCacheStressor.h +++ b/cachelib/cachebench/runner/AsyncCacheStressor.h @@ -287,6 +287,10 @@ class AsyncCacheStressor : public Stressor { ++stats.get; auto lock = chainedItemAcquireUniqueLock(*key); + // This was moved outside the lambda, as otherwise gcc-8.x crashes with an + // internal compiler error here (suspected regression in folly). + XDCHECK(req->sizeBegin + 1 != req->sizeEnd); + auto onReadyFn = [&, req, key, l = std::move(lock), pid](auto hdl) { WriteHandle wHdl; if (hdl == nullptr) { @@ -303,7 +307,6 @@ class AsyncCacheStressor : public Stressor { } else { wHdl = std::move(hdl).toWriteHandle(); } - XDCHECK(req->sizeBegin + 1 != req->sizeEnd); bool chainSuccessful = false; for (auto j = req->sizeBegin + 1; j != req->sizeEnd; j++) { ++stats.addChained; From f853a42a804c7259d55272d70b010078c50ffe52 Mon Sep 17 00:00:00 2001 From: Jiayue Bao Date: Wed, 1 Mar 2023 15:39:43 -0800 Subject: [PATCH 23/44] Get or update object TTL via object shared_ptr Summary: Add the following TTL-related APIs: - getConfiguredTtlSec - getExpiryTimeSec - extendTtlSec - updateExpiryTimeSec Usage: ``` auto ptr = objcache->find("key"); auto configuredTtl = objcache->getConfiguredTtl(ptr); auto expiryTime = objcache->getExpiryTimeSec(ptr); objcache->extendTtl(ptr, std::chrono::seconds(3)); objcache->updateExpiryTimeSec(ptr, newExpiryTimeSecs); ``` Reviewed By: therealgymmy, jaesoo-fb Differential Revision: D43167879 fbshipit-source-id: 3b11fb0a2b9a3b5c38fcfd856ade100e6ae27470 --- cachelib/experimental/objcache2/ObjectCache.h | 79 +++++++++++++++ .../objcache2/tests/ObjectCacheTest.cpp | 95 +++++++++++++++++++ 2 files changed, 174 insertions(+) diff --git a/cachelib/experimental/objcache2/ObjectCache.h b/cachelib/experimental/objcache2/ObjectCache.h index 5f8aab85a..125321328 100644 --- a/cachelib/experimental/objcache2/ObjectCache.h +++ b/cachelib/experimental/objcache2/ObjectCache.h @@ -267,6 +267,63 @@ class ObjectCache : public ObjectCacheBase { : sizeController_->getCurrentEntriesLimit(); } + // Get the expiry timestamp of the object + // @param object object shared pointer returned from ObjectCache APIs + // + // @return the expiry timestamp in seconds of the object + // 0 if object is nullptr + template + uint32_t getExpiryTimeSec(const std::shared_ptr& object) const { + if (object == nullptr) { + return 0; + } + return getReadHandleRefInternal(object)->getExpiryTime(); + } + + // Get the configured TTL of the object + // @param object object shared pointer returned from ObjectCache APIs + // + // @return the configured TTL in seconds of the object + // 0 if object is nullptr + template + std::chrono::seconds getConfiguredTtl( + const std::shared_ptr& object) const { + if (object == nullptr) { + return std::chrono::seconds{0}; + } + return getReadHandleRefInternal(object)->getConfiguredTTL(); + } + + // Update the expiry timestamp of an object + // + // @param object object shared pointer returned from ObjectCache APIs + // @param expiryTimeSecs the expiryTime in seconds to update + // + // @return boolean indicating whether expiry time was successfully updated + template + bool updateExpiryTimeSec(std::shared_ptr& object, + uint32_t expiryTimeSecs) { + if (object == nullptr) { + return false; + } + return getWriteHandleRefInternal(object)->updateExpiryTime( + expiryTimeSecs); + } + + // Update expiry time to @ttl seconds from now. + // + // @param object object shared pointer returned from ObjectCache APIs + // @param ttl TTL in seconds (from now) + // + // @return boolean indicating whether TTL was successfully extended + template + bool extendTtl(std::shared_ptr& object, std::chrono::seconds ttl) { + if (object == nullptr) { + return false; + } + return getWriteHandleRefInternal(object)->extendTTL(ttl); + } + protected: // Serialize cache allocator config for exporting to Scuba std::map serializeConfigParams() const override; @@ -307,6 +364,28 @@ class ObjectCache : public ObjectCacheBase { bool stopSizeController(std::chrono::seconds timeout = std::chrono::seconds{ 0}); + // Get a ReadHandle reference from the object shared_ptr + template + typename AllocatorT::ReadHandle& getReadHandleRefInternal( + const std::shared_ptr& object) const { + auto* deleter = std::get_deleter>(object); + XDCHECK(deleter != nullptr); + auto& hdl = deleter->getReadHandleRef(); + XDCHECK(hdl != nullptr); + return hdl; + } + + // Get a WriteHandle reference from the object shared_ptr + template + typename AllocatorT::WriteHandle& getWriteHandleRefInternal( + std::shared_ptr& object) { + auto* deleter = std::get_deleter>(object); + XDCHECK(deleter != nullptr); + auto& hdl = deleter->getWriteHandleRef(); + XDCHECK(hdl != nullptr); + return hdl; + } + // Config passed to the cache. Config config_{}; diff --git a/cachelib/experimental/objcache2/tests/ObjectCacheTest.cpp b/cachelib/experimental/objcache2/tests/ObjectCacheTest.cpp index 6bafa4058..701654cd1 100644 --- a/cachelib/experimental/objcache2/tests/ObjectCacheTest.cpp +++ b/cachelib/experimental/objcache2/tests/ObjectCacheTest.cpp @@ -886,6 +886,69 @@ class ObjectCacheTest : public ::testing::Test { } } + void testGetTtl() { + const uint32_t ttlSecs = 600; + + ObjectCacheConfig config; + config.setCacheName("test").setCacheCapacity(10'000).setItemDestructor( + [&](ObjectCacheDestructorData data) { data.deleteObject(); }); + auto objcache = ObjectCache::create(config); + + auto before = util::getCurrentTimeSec(); + std::this_thread::sleep_for(std::chrono::seconds{3}); + objcache->insertOrReplace("Foo", std::make_unique(), 0 /*object size*/, + ttlSecs); + + // lookup via find API + auto found1 = objcache->template find("Foo"); + ASSERT_NE(nullptr, found1); + + // get TTL info + EXPECT_EQ(ttlSecs, objcache->getConfiguredTtl(found1).count()); + EXPECT_LE(before + ttlSecs, objcache->getExpiryTimeSec(found1)); + + // lookup via findToWrite API + auto found2 = objcache->template findToWrite("Foo"); + ASSERT_NE(nullptr, found2); + + // get TTL info + EXPECT_EQ(ttlSecs, objcache->getConfiguredTtl(found2).count()); + EXPECT_LE(before + ttlSecs, objcache->getExpiryTimeSec(found2)); + } + + void testUpdateTtl() { + const uint32_t ttlSecs = 600; + + ObjectCacheConfig config; + config.setCacheName("test").setCacheCapacity(10'000).setItemDestructor( + [&](ObjectCacheDestructorData data) { data.deleteObject(); }); + auto objcache = ObjectCache::create(config); + + auto insertionTime = util::getCurrentTimeSec(); + objcache->insertOrReplace("Foo", std::make_unique(), 0 /*object size*/, + ttlSecs); + + auto found = objcache->template find("Foo"); + ASSERT_NE(nullptr, found); + + // get TTL info + EXPECT_EQ(ttlSecs, objcache->getConfiguredTtl(found).count()); + EXPECT_LE(insertionTime + ttlSecs, objcache->getExpiryTimeSec(found)); + + // update expiry time + auto currExpTime = objcache->getExpiryTimeSec(found); + EXPECT_TRUE(objcache->updateExpiryTimeSec(found, currExpTime + ttlSecs)); + EXPECT_EQ(2 * ttlSecs, objcache->getConfiguredTtl(found).count()); + EXPECT_EQ(currExpTime + ttlSecs, objcache->getExpiryTimeSec(found)); + + // extend TTL + auto now = util::getCurrentTimeSec(); + std::this_thread::sleep_for(std::chrono::seconds{3}); + EXPECT_TRUE(objcache->extendTtl(found, std::chrono::seconds(3 * ttlSecs))); + EXPECT_LE(now + ttlSecs, objcache->getExpiryTimeSec(found)); + EXPECT_LE(3 * ttlSecs, objcache->getConfiguredTtl(found).count()); + } + void testMultithreadReplace() { // Sanity test to see if insertOrReplace across multiple // threads are safe. @@ -1079,6 +1142,32 @@ class ObjectCacheTest : public ::testing::Test { fs[i].join(); } } + + void testMultithreadUpdateTtl() { + // Sanity test to see if update TTL across multiple + // threads is safe. + ObjectCacheConfig config; + config.setCacheName("test").setCacheCapacity(10'000).setItemDestructor( + [&](ObjectCacheDestructorData data) { data.deleteObject(); }); + auto objcache = ObjectCache::create(config); + objcache->insertOrReplace("key", std::make_unique(), 0, 60); + + auto runUpdateTtlOps = [&] { + for (int i = 0; i < 2000; i++) { + auto found = objcache->template find("key"); + auto configuredTtlSecs = objcache->getConfiguredTtl(found).count(); + objcache->extendTtl(found, std::chrono::seconds{configuredTtlSecs}); + } + }; + + std::vector ts; + for (int i = 0; i < 10; i++) { + ts.push_back(std::thread{runUpdateTtlOps}); + } + for (int i = 0; i < 10; i++) { + ts[i].join(); + } + } }; using AllocatorTypes = ::testing::TypestestPersistenceHighLoad(); } +TYPED_TEST(ObjectCacheTest, GetTtl) { this->testGetTtl(); } +TYPED_TEST(ObjectCacheTest, UpdateTtl) { this->testUpdateTtl(); } + TYPED_TEST(ObjectCacheTest, MultithreadReplace) { this->testMultithreadReplace(); } @@ -1135,6 +1227,9 @@ TYPED_TEST(ObjectCacheTest, MultithreadFindAndEviction) { TYPED_TEST(ObjectCacheTest, MultithreadFindAndReplaceWith10Shards) { this->testMultithreadFindAndReplaceWith10Shards(); } +TYPED_TEST(ObjectCacheTest, MultithreadUpdateTtl) { + this->testMultithreadUpdateTtl(); +} using ObjectCache = ObjectCache; TEST(ObjectCacheTest, LruEviction) { From c120a5307c3062b21253c27f4a3ea598ae780b8b Mon Sep 17 00:00:00 2001 From: Jiayue Bao Date: Wed, 1 Mar 2023 16:13:29 -0800 Subject: [PATCH 24/44] Remove isWriteHandle() API Summary: Based on our discussion, this API would be confusing if a reference of ReadHandle is obtained from a WriteHandle. We also don't want to make it virtual because this will increase `sizeof(ReadHandle)` / `sizeof(WriteHandle)` by 8 bytes. Reviewed By: therealgymmy Differential Revision: D43667308 fbshipit-source-id: a77a17113f1a23f84332ebfa7ea6772d7647339c --- cachelib/allocator/Handle.h | 4 ---- cachelib/allocator/tests/BaseAllocatorTest.h | 8 +------- 2 files changed, 1 insertion(+), 11 deletions(-) diff --git a/cachelib/allocator/Handle.h b/cachelib/allocator/Handle.h index a125ace1b..11d2bed2b 100644 --- a/cachelib/allocator/Handle.h +++ b/cachelib/allocator/Handle.h @@ -242,8 +242,6 @@ struct ReadHandleImpl { return hdl; } - bool isWriteHandle() const { return false; } - protected: // accessor. Calling getInternal() on handle with isReady() == false blocks // the thread until the handle is ready. @@ -571,8 +569,6 @@ struct WriteHandleImpl : public ReadHandleImpl { // creating this item handle. WriteHandleImpl clone() const { return WriteHandleImpl{ReadHandle::clone()}; } - bool isWriteHandle() const { return true; } - // Friends friend ReadHandle; // Only CacheAllocator and NvmCache can create non-default constructed handles diff --git a/cachelib/allocator/tests/BaseAllocatorTest.h b/cachelib/allocator/tests/BaseAllocatorTest.h index d684545cb..aa9d38a85 100644 --- a/cachelib/allocator/tests/BaseAllocatorTest.h +++ b/cachelib/allocator/tests/BaseAllocatorTest.h @@ -713,35 +713,29 @@ class BaseAllocatorTest : public AllocatorTest { auto handle = alloc.find("key"); ASSERT_NE(handle, nullptr); ASSERT_TRUE(isConst(handle->getMemory())); - ASSERT_EQ(handle.isWriteHandle(), false); // read handle clone auto handle2 = handle.clone(); ASSERT_TRUE(isConst(handle2->getMemory())); - ASSERT_EQ(handle2.isWriteHandle(), false); // upgrade a read handle to a write handle auto handle3 = std::move(handle).toWriteHandle(); ASSERT_FALSE(isConst(handle3->getMemory())); - ASSERT_EQ(handle3.isWriteHandle(), true); } { auto handle = alloc.findToWrite("key"); ASSERT_NE(handle, nullptr); ASSERT_FALSE(isConst(handle->getMemory())); - ASSERT_EQ(handle.isWriteHandle(), true); // write handle clone auto handle2 = handle.clone(); ASSERT_FALSE(isConst(handle2->getMemory())); - ASSERT_EQ(handle2.isWriteHandle(), true); // downgrade a write handle to a read handle ReadHandle handle3 = handle.clone(); ASSERT_NE(handle3, nullptr); ASSERT_TRUE(isConst(handle3->getMemory())); - ASSERT_EQ(handle3.isWriteHandle(), false); } { @@ -752,7 +746,7 @@ class BaseAllocatorTest : public AllocatorTest { // This is like doing a "clone" and setting it into wait context waitContext->set(alloc.find("key")); auto handle2 = std::move(handle).toWriteHandle(); - ASSERT_EQ(handle2.isWriteHandle(), true); + ASSERT_FALSE(isConst(handle2->getMemory())); } } From 7785d24b3030a64a2e7d885e8ad552d5fa758f9b Mon Sep 17 00:00:00 2001 From: Jaesoo Lee Date: Wed, 1 Mar 2023 16:16:43 -0800 Subject: [PATCH 25/44] Edit Cachebench_FB_HW_eval.md using inpage editor Summary: This diff has been automatically generated by the inpage editor. NOTE: If you want to update this diff, go via the preview link inside the static docs section below. Ensure you are editing the same page that was used to create this diff. Reviewed By: therealgymmy Differential Revision: D43667238 fbshipit-source-id: 0be4c1ef376a5a1a2de92afc311af24f66d10afd --- .../Cache_Library_User_Guides/Cachebench_FB_HW_eval.md | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/website/docs/Cache_Library_User_Guides/Cachebench_FB_HW_eval.md b/website/docs/Cache_Library_User_Guides/Cachebench_FB_HW_eval.md index ce9964913..09abe4b05 100644 --- a/website/docs/Cache_Library_User_Guides/Cachebench_FB_HW_eval.md +++ b/website/docs/Cache_Library_User_Guides/Cachebench_FB_HW_eval.md @@ -17,10 +17,11 @@ sufficient free memory (50+GB) and SSD capacity (1TB). * SSD Capacity: 100GB or more available capacity * Internet connection capable of accessing github.com and installing packages -## Set up the SSD devices using mdraid +## Set up the SSD devices -To gather SSD performance metrics, the SSD must be setup first. An example -below sets up a raid device to handle two ssds being used by CacheBench. +To gather SSD performance metrics, the SSD must be setup first. Cachebench (and CacheLib) supports using various types of devices for NVM cache including a raw block device or a regular file. When one wants to use multiple SSDs as NVM cache, the CacheLib also provides a native support for RAID0 (i.e., striping). + +Optionally, as an example, an user can setup and use md devices as follows. In this example, the md device is created from two ssd devices to be used as a raw block device in CacheBench. ```sh mdadm --create /dev/md0 --force --raid-devices=2 --level=0 --chunk=256 /dev/nvme1n1 /dev/nvme2n1 From 185bbe6664167a680f4070713987548573cbbd2c Mon Sep 17 00:00:00 2001 From: Daniel Wong Date: Thu, 2 Mar 2023 11:23:44 -0800 Subject: [PATCH 26/44] Fix Debian GitHub build & zstd CMake error (#200) Summary: 1. Workaround for Debian Docker image bug that is breaking Debian build on GitHub (Explicitly mark Git repo as safe). 2. Pin zstd to a commit that resolves problems with older CMakes (note: affects all OSes, not just Debian) Context for 1: In latest Debian Docker image , there is a regression that affects the checkout action. From https://github.com/actions/checkout/issues/1169: > - Checkout runs, and runs /usr/bin/git config --global --add safe.directory > - The global .gitconfig does not exist > - Any calls to git remain unsafe/dubious The suggested workaround was to use --system instead of --global. Pull Request resolved: https://github.com/facebook/CacheLib/pull/200 Test Plan: See if GitHub Action Debian build is fixed. Reviewed By: therealgymmy Differential Revision: D43720363 Pulled By: jaesoo-fb fbshipit-source-id: 54f3586cc7f8e72045e60d8dd454c7a77725e6b2 --- .github/workflows/build-cachelib-debian-10.yml | 3 +++ contrib/build-package.sh | 8 ++++++-- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/.github/workflows/build-cachelib-debian-10.yml b/.github/workflows/build-cachelib-debian-10.yml index 56fb57629..7f0ab29a6 100644 --- a/.github/workflows/build-cachelib-debian-10.yml +++ b/.github/workflows/build-cachelib-debian-10.yml @@ -52,6 +52,9 @@ jobs: g++ - || true - name: "checkout sources" uses: actions/checkout@v2 + - name: "Add Git safe directory" + # Workaround for Docker image bug (GitHub issue #199). + run: git config --system --add safe.directory $GITHUB_WORKSPACE - name: "Install Prerequisites" run: ./contrib/build.sh -S -B - name: "Test: update-submodules" diff --git a/contrib/build-package.sh b/contrib/build-package.sh index 6e7acac5c..755933bd4 100755 --- a/contrib/build-package.sh +++ b/contrib/build-package.sh @@ -102,11 +102,12 @@ test "$#" -eq 0 \ && die "missing dependancy name to build. See -h for help" ###################################### -## Check which dependecy was requested +## Check which dependency was requested ###################################### external_git_clone= external_git_branch= +# external_git_tag can also be used for commit hashes external_git_tag= update_submodules= cmake_custom_params= @@ -175,7 +176,10 @@ case "$1" in REPODIR=cachelib/external/$NAME SRCDIR=$REPODIR/build/cmake external_git_clone=yes - external_git_branch=release + # Previously, we pinned to release branch. v1.5.4 needed + # CMake >= 3.18, later reverted. While waiting for v1.5.5, + # pin to the fix: https://github.com/facebook/zstd/pull/3510 + external_git_tag=8420502e if test "$build_tests" = "yes" ; then cmake_custom_params="-DZSTD_BUILD_TESTS=ON" else From 968533f58cfdc9fa70bdedc46918005150899a2d Mon Sep 17 00:00:00 2001 From: Jaesoo Lee Date: Fri, 3 Mar 2023 09:43:41 -0800 Subject: [PATCH 27/44] fix broken installation page link Summary: An user in github reported an issue that the installation link is broken. Somehow, docs/installation/installation.md cannot be referenced by `/docs/installation/installation`, but only by `/docs/installation`. The root cause could not figured out yet, but this change fixes it as such for now. Reviewed By: jiayuebao Differential Revision: D43757739 fbshipit-source-id: 4abd3208800c0b68e9162d381f6395897f047b24 --- README.md | 2 +- .../docs/Cache_Library_User_Guides/Cachebench_FB_HW_eval.md | 5 ++--- .../docs/Cache_Library_User_Guides/Cachebench_Overview.md | 2 +- website/docs/installation/testing.md | 4 ++-- 4 files changed, 6 insertions(+), 7 deletions(-) diff --git a/README.md b/README.md index e05c932d1..523b8b20a 100644 --- a/README.md +++ b/README.md @@ -50,7 +50,7 @@ cd CacheLib Re-running `./contrib/build.sh` will update CacheLib and its dependencies to their latest versions and rebuild them. -See [build](https://cachelib.org/docs/installation/installation) for more details about +See [build](https://cachelib.org/docs/installation/) for more details about the building and installation process. diff --git a/website/docs/Cache_Library_User_Guides/Cachebench_FB_HW_eval.md b/website/docs/Cache_Library_User_Guides/Cachebench_FB_HW_eval.md index 09abe4b05..d17b7ac52 100644 --- a/website/docs/Cache_Library_User_Guides/Cachebench_FB_HW_eval.md +++ b/website/docs/Cache_Library_User_Guides/Cachebench_FB_HW_eval.md @@ -19,7 +19,7 @@ sufficient free memory (50+GB) and SSD capacity (1TB). ## Set up the SSD devices -To gather SSD performance metrics, the SSD must be setup first. Cachebench (and CacheLib) supports using various types of devices for NVM cache including a raw block device or a regular file. When one wants to use multiple SSDs as NVM cache, the CacheLib also provides a native support for RAID0 (i.e., striping). +To gather SSD performance metrics, the SSD must be setup first. Cachebench (and CacheLib) supports using various types of devices for NVM cache including a raw block device or a regular file. When one wants to use multiple SSDs as NVM cache, the CacheLib also provides a native support for RAID0 (i.e., striping). Optionally, as an example, an user can setup and use md devices as follows. In this example, the md device is created from two ssd devices to be used as a raw block device in CacheBench. @@ -143,7 +143,7 @@ mdadm --create /dev/md0 --force --raid-devices=2 --level=0 --chunk=256 /dev/nvme make install ``` -See [build and installation](/docs/installation/installation) for further details. +See [build and installation](/docs/installation) for further details. ## Running the benchmark for SSD perf testing @@ -197,7 +197,6 @@ For a full list of options that can be configured, see [configuring cachebench]( using the `--progress` and specifying a duration in seconds. If `--progress-stats-file` is also specified, on every progress interval, `cachebench` would log the internal stats to the specified file. - ## Running cachebench with the trace workload Meta is sharing anonymized traces captured from large scale production cache services. These traces are licensed under the same license as CacheLib. They are meant to help academic and industry researchers to optimize for our caching workloads. One can freely download it from our AWS S3 bucket and run the CacheBench to replay the trace with varying configuration as follows. diff --git a/website/docs/Cache_Library_User_Guides/Cachebench_Overview.md b/website/docs/Cache_Library_User_Guides/Cachebench_Overview.md index eb7264654..8c878e1be 100644 --- a/website/docs/Cache_Library_User_Guides/Cachebench_Overview.md +++ b/website/docs/Cache_Library_User_Guides/Cachebench_Overview.md @@ -53,6 +53,6 @@ developer's need. The following are few examples. ## Building cachebench -Follow instructions in [Installation](/docs/installation/installation) to build +Follow instructions in [Installation](/docs/installation) to build cachebench. This should install cachebench in your local machine under ```opt/cachelib/bin/cachebench``` diff --git a/website/docs/installation/testing.md b/website/docs/installation/testing.md index 02b2cb747..d8730127b 100644 --- a/website/docs/installation/testing.md +++ b/website/docs/installation/testing.md @@ -11,7 +11,7 @@ of the cache infrastructure. ## Building CacheLib Unit Tests To build the cachelib unit tests, use one of the following commands -(see [installation](docs/installation/installation) instructions for more details): +(see [installation](/docs/installation) instructions for more details): 1. Use `./contrib/build.sh` script with the `-T` option. 2. Use `./contrib/build-package.sh -t cachelib` (with the `-t` option) @@ -42,7 +42,7 @@ Running a single unit test binary: ```sh $ cd opt/cachelib/tests -$ ./allocator-test-ItemTest +$ ./allocator-test-ItemTest [==========] Running 6 tests from 1 test suite. [----------] Global test environment set-up. [----------] 6 tests from ItemTest From 496ed376b7000bca496e03a2b1cb4e47d24e7f24 Mon Sep 17 00:00:00 2001 From: Jaesoo Lee Date: Fri, 3 Mar 2023 12:13:05 -0800 Subject: [PATCH 28/44] fix another broken installation page link in index.js Summary: There is another missing link in index.js for installation page. This change fixes it. Reviewed By: jiayuebao Differential Revision: D43774697 fbshipit-source-id: 1c2ff1c801ae41ea6b50e6e687714f422bcb0c92 --- website/src/pages/index.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/website/src/pages/index.js b/website/src/pages/index.js index 151ec3f3f..5886c079f 100644 --- a/website/src/pages/index.js +++ b/website/src/pages/index.js @@ -117,7 +117,7 @@ function Home() { 'button button--secondary button--lg', styles.getStarted, )} - to={ useBaseUrl('docs/installation/installation') }> + to={ useBaseUrl('docs/installation') }> Get Started From 67d2d8be64206b6c75e123483c3ebe202b548a1c Mon Sep 17 00:00:00 2001 From: Michel Salim Date: Sat, 4 Mar 2023 18:50:50 -0800 Subject: [PATCH 29/44] temporarily disable PackIt (#205) Summary: We need to fix builds for the entire Folly stack first before this will work again Pull Request resolved: https://github.com/facebook/CacheLib/pull/205 Reviewed By: michel-slm, jiayuebao Differential Revision: D43786012 Pulled By: jaesoo-fb fbshipit-source-id: dfd1947d7d2c294fc622496d6054c2395ef0d5a3 --- .packit.yaml | 25 ------------------------- 1 file changed, 25 deletions(-) delete mode 100644 .packit.yaml diff --git a/.packit.yaml b/.packit.yaml deleted file mode 100644 index bea307d9d..000000000 --- a/.packit.yaml +++ /dev/null @@ -1,25 +0,0 @@ -# See the documentation for more information: -# https://packit.dev/docs/configuration - -specfile_path: cachelib.spec - -upstream_package_name: CacheLib -downstream_package_name: cachelib - -actions: - fix-spec-file: - - bash -c "sed -i cachelib.spec -e \"s/%global commit.*/%global commit $(git rev-parse HEAD)/\"" - - bash -c "sed -i cachelib.spec -e \"s/%global date.*/%global date $(git show -s --date=format:'%Y%m%d' --format=%cd)/\"" - create-archive: - - bash -c "COMMIT=$(git rev-parse HEAD); curl -ORL https://github.com/facebook/CacheLib/archive/${COMMIT}/cachelib-${COMMIT}.tar.gz; echo cachelib-${COMMIT}.tar.gz" - post-upstream-clone: "bash -c \"rm -rf cachelib-dist-git; git clone -b packit https://pagure.io/meta/cachelib.git cachelib-dist-git && mv cachelib-dist-git/cachelib*.{spec,patch} .\"" - -jobs: -- job: copr_build - trigger: pull_request - metadata: - targets: - - fedora-rawhide-aarch64 - - fedora-rawhide-x86_64 - - fedora-35-aarch64 - - fedora-35-x86_64 From 115732a51ed3c5b9ce88842f4ccb5c876eab6401 Mon Sep 17 00:00:00 2001 From: Jaesoo Lee Date: Mon, 6 Mar 2023 09:55:32 -0800 Subject: [PATCH 30/44] Edit Overview_A_random_walk_down_the_Cache_Library.md using inpage editor Summary: This diff has been automatically generated by the inpage editor. NOTE: If you want to update this diff, go via the preview link inside the static docs section below. Ensure you are editing the same page that was used to create this diff. Reviewed By: therealgymmy Differential Revision: D43781316 fbshipit-source-id: 18091011f9e4592240fb41c5fb9cc8cb40e62205 --- .../Overview_A_random_walk_down_the_Cache_Library.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/website/docs/Cache_Library_Architecture_Guide/Overview_A_random_walk_down_the_Cache_Library.md b/website/docs/Cache_Library_Architecture_Guide/Overview_A_random_walk_down_the_Cache_Library.md index f13fc1322..cf4e750c4 100644 --- a/website/docs/Cache_Library_Architecture_Guide/Overview_A_random_walk_down_the_Cache_Library.md +++ b/website/docs/Cache_Library_Architecture_Guide/Overview_A_random_walk_down_the_Cache_Library.md @@ -107,7 +107,7 @@ There will be a section discussing each of the bullets below. * For regular cache: Find the item in the chained hash map. From the item, get the slab it lives on and form the slab, identify the allocation class and promote the item on that particular LRU queue. Increment the refcount and return the item handle. ## Flash overview -Flash is organized in a similar way: there is a cache for smaller items (BigHash) and for larger item (Block Cache). Unlike DRAM, the client does not get to choose where the item goes. It's done automatically thresholding the size. Together, this constitutes [Navy](/docs/Cache_Library_Architecture_Guide/Navy_Architecture_Overview ) -- the flash cache engine of CacheLib. +Flash is organized in a similar way: there is a cache for smaller items (BigHash) and for larger item (Block Cache). Unlike DRAM, the client does not get to choose where the item goes. It's done automatically thresholding the size. Together, this constitutes [Navy](/docs/Cache_Library_Architecture_Guide/Navy_Overview ) -- the flash cache engine of CacheLib. * "block device" refers to devices that's read/write happen with a fixed size block (if it helps, substitute the word "page" here). It means you can't write with precision of bytes but have to incur overhead if you don't write an entire block. From 17bdc1818911fc13bee1bfd10480a496d6336496 Mon Sep 17 00:00:00 2001 From: generatedunixname226714639793621 Date: Tue, 7 Mar 2023 10:30:13 -0800 Subject: [PATCH 31/44] fbcode/cachelib/experimental/objcache2/persistence Reviewed By: jiayuebao Differential Revision: D43869027 fbshipit-source-id: 6f805a86b072e2fc96d4357672175e8283e06d0d --- .../objcache2/persistence/persistent_data.thrift | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/cachelib/experimental/objcache2/persistence/persistent_data.thrift b/cachelib/experimental/objcache2/persistence/persistent_data.thrift index 486e130a1..1d310693f 100644 --- a/cachelib/experimental/objcache2/persistence/persistent_data.thrift +++ b/cachelib/experimental/objcache2/persistence/persistent_data.thrift @@ -17,12 +17,12 @@ namespace cpp2 facebook.cachelib.objcache2.persistence struct Item { - 1: string key, - 2: string payload, - 3: i32 objectSize, - 4: i32 expiryTime, + 1: string key; + 2: string payload; + 3: i32 objectSize; + 4: i32 expiryTime; } struct Metadata { - 1: i32 threadCount, + 1: i32 threadCount; } From e679b72d5b78c51f1cd3a250ad4918250af8ddc9 Mon Sep 17 00:00:00 2001 From: Jaesoo Lee Date: Tue, 7 Mar 2023 15:54:02 -0800 Subject: [PATCH 32/44] README.md: added build status badges to main README.md Summary: As suggested by wonglkd in https://github.com/facebook/CacheLib/issues/199, this change adds the build status for all targets in the main README.md (https://github.com/facebook/CacheLib) Reviewed By: therealgymmy Differential Revision: D43854616 fbshipit-source-id: add32de29c160ed06e1778fef41bb37ffc359fd7 --- README.md | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/README.md b/README.md index 523b8b20a..7fc943b20 100644 --- a/README.md +++ b/README.md @@ -77,3 +77,18 @@ https://www.facebook.com/whitehat Facebook's security team will triage your report and determine whether or not is it eligible for a bounty under our program. + + +## Build status + +Clicking on a badge will show you the recent builds for that OS. If your target OS's build is failing, you may wish to check recent issues and PRs for known workarounds. + +- [![CentOS 8.1](https://github.com/facebook/cachelib/actions/workflows/build-cachelib-centos-8-1.yml/badge.svg?event=schedule)](https://github.com/facebook/cachelib/actions/workflows/build-cachelib-centos-8-1.yml?query=event%3Aschedule) +- [![CentOS 8.5](https://github.com/facebook/cachelib/actions/workflows/build-cachelib-centos-8-5.yml/badge.svg?event=schedule)](https://github.com/facebook/cachelib/actions/workflows/build-cachelib-centos-8-5.yml?query=event%3Aschedule) +- [![Debian 10](https://github.com/facebook/cachelib/actions/workflows/build-cachelib-debian-10.yml/badge.svg?event=schedule)](https://github.com/facebook/cachelib/actions/workflows/build-cachelib-debian-10.yml?query=event%3Aschedule) +- [![Fedora 36](https://github.com/facebook/cachelib/actions/workflows/build-cachelib-fedora-36.yml/badge.svg?event=schedule)](https://github.com/facebook/cachelib/actions/workflows/build-cachelib-fedora-36.yml?query=event%3Aschedule) +- [![Rocky Linux 8](https://github.com/facebook/cachelib/actions/workflows/build-cachelib-rockylinux-8.yml/badge.svg?event=schedule)](https://github.com/facebook/cachelib/actions/workflows/build-cachelib-rockylinux-8.yml?query=event%3Aschedule) +- [![Rocky Linux 9](https://github.com/facebook/cachelib/actions/workflows/build-cachelib-rockylinux-9.yml/badge.svg?event=schedule)](https://github.com/facebook/cachelib/actions/workflows/build-cachelib-rockylinux-9.yml?query=event%3Aschedule) +- [![Ubuntu 18](https://github.com/facebook/cachelib/actions/workflows/build-cachelib-ubuntu-18.yml/badge.svg?event=schedule)](https://github.com/facebook/cachelib/actions/workflows/build-cachelib-ubuntu-18.yml?query=event%3Aschedule) +- [![Ubuntu 20](https://github.com/facebook/cachelib/actions/workflows/build-cachelib-ubuntu-20.yml/badge.svg?event=schedule)](https://github.com/facebook/cachelib/actions/workflows/build-cachelib-ubuntu-20.yml?query=event%3Aschedule) +- [![Ubuntu 22](https://github.com/facebook/cachelib/actions/workflows/build-cachelib-ubuntu-22.yml/badge.svg?event=schedule)](https://github.com/facebook/cachelib/actions/workflows/build-cachelib-ubuntu-22.yml?query=event%3Aschedule) From 43a7ce3ab7b58c26e15b2972203948e8f016e2d5 Mon Sep 17 00:00:00 2001 From: Aliaa Atwi Date: Wed, 8 Mar 2023 11:54:15 -0800 Subject: [PATCH 33/44] Fix Bug in ItemValue handling in CacheBench Summary: **Context:** To enable user/intern simulations, D41134915 added 'itemValue' support in cachebench simulations. Any string that appears in the 'itemValue' column in the trace, is used to populate the cachelib item value (previously empty). For user/intern, we set this string to "0" for user and "1" for intern. Then in FBDep.cpp, we tell the admission policy to distinguish user/intern with a lambda function that parses item value. Prod uses a different lambda that reads item *header*, which has a user/intern flag set by Proxygen as part of the request to BigCache. We cannot use the same function and item header flag in simulations because the simulator does not run Proxygen. Instead, it emulates its behavior with trace + PieceWiseReplayGenerator. **This diff:** Currently, the logic that populates item value in cachebench (setStringItem below) takes a std::string and sets the item value with that. When we want to read the item in FBDep, we don't know where the end of the string is because the item can be bigger than the data. Right now, we're avoiding this by adding a null character at the end of the itemValue data string. FBDep then uses strnlen to find the length of the data until the null string. **This is not working as intended**, and the string read in FBDep includes gibberish after the "0" or "1" because it is not properly terminated. This diff fixes the issue. Reviewed By: jaesoo-fb Differential Revision: D43861361 fbshipit-source-id: d4974281e28165ecf8c23c44dc4a6b50d5b1b806 --- cachelib/cachebench/cache/Cache-inl.h | 13 +++++++++++-- cachelib/cachebench/runner/CacheStressor.h | 4 +--- 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/cachelib/cachebench/cache/Cache-inl.h b/cachelib/cachebench/cache/Cache-inl.h index ed8bfd1b0..dc5e878fc 100644 --- a/cachelib/cachebench/cache/Cache-inl.h +++ b/cachelib/cachebench/cache/Cache-inl.h @@ -794,8 +794,17 @@ void Cache::setUint64ToItem(WriteHandle& handle, template void Cache::setStringItem(WriteHandle& handle, const std::string& str) { - auto ptr = reinterpret_cast(getMemory(handle)); - std::memcpy(ptr, str.data(), std::min(str.size(), getSize(handle))); + auto dataSize = getSize(handle); + if (dataSize < 1) + return; + + auto ptr = reinterpret_cast(getMemory(handle)); + std::strncpy(ptr, str.c_str(), dataSize); + + // Make sure the copied string ends with null char + if (str.size() + 1 > dataSize) { + ptr[dataSize - 1] = '\0'; + } } template diff --git a/cachelib/cachebench/runner/CacheStressor.h b/cachelib/cachebench/runner/CacheStressor.h index 4ea54b1f1..d2433a734 100644 --- a/cachelib/cachebench/runner/CacheStressor.h +++ b/cachelib/cachebench/runner/CacheStressor.h @@ -254,9 +254,7 @@ class CacheStressor : public Stressor { } if (!itemValue.empty()) { - // Add the null character to ensure this is a proper c string. - // TODO(T141356292): Clean this up to avoid allocating a new string - cache_->setStringItem(handle, itemValue + "\0"); + cache_->setStringItem(handle, itemValue); } else { cache_->setStringItem(handle, hardcodedString_); } From 872d61a6f09ce100c510069f526ce9fb9cb701c6 Mon Sep 17 00:00:00 2001 From: generatedunixname226714639793621 Date: Wed, 8 Mar 2023 16:08:53 -0800 Subject: [PATCH 34/44] fbcode/cachelib/benchmarks Differential Revision: D43869116 fbshipit-source-id: c62c2518691313ec82c15c9f65ac304378222257 --- cachelib/benchmarks/DataTypeBench.thrift | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/cachelib/benchmarks/DataTypeBench.thrift b/cachelib/benchmarks/DataTypeBench.thrift index a1f16f4b7..94d53ade8 100644 --- a/cachelib/benchmarks/DataTypeBench.thrift +++ b/cachelib/benchmarks/DataTypeBench.thrift @@ -17,11 +17,9 @@ namespace cpp2 facebook.cachelib.datatypebench struct StdMap { - 1: required map m, + 1: required map m; } struct StdUnorderedMap { - 1: required map - (cpp.template = "std::unordered_map") - m, + 1: required map (cpp.template = "std::unordered_map") m; } From ffa54709fb021fa2d15494fed39d60df53b0fb0f Mon Sep 17 00:00:00 2001 From: generatedunixname226714639793621 Date: Wed, 8 Mar 2023 16:09:11 -0800 Subject: [PATCH 35/44] fbcode/cachelib/allocator/datastruct/tests Differential Revision: D43869054 fbshipit-source-id: e90b16576404b8eb5819c65d61fe972f0f518c66 --- cachelib/allocator/datastruct/tests/test_objects.thrift | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cachelib/allocator/datastruct/tests/test_objects.thrift b/cachelib/allocator/datastruct/tests/test_objects.thrift index 51af09851..4cb58b188 100644 --- a/cachelib/allocator/datastruct/tests/test_objects.thrift +++ b/cachelib/allocator/datastruct/tests/test_objects.thrift @@ -20,6 +20,6 @@ namespace cpp2 facebook.cachelib.test_serialization // testing warm rolls from the old format to the new format. // TODO(bwatling): remove this when 'compressedTail' is always present. struct SListObjectNoCompressedTail { - 2: required i64 size, - 3: required i64 compressedHead, // Pointer to the head element + 2: required i64 size; + 3: required i64 compressedHead; // Pointer to the head element } From 1031b13fc914104ca9b606edc7c183896e0bb46a Mon Sep 17 00:00:00 2001 From: generatedunixname226714639793621 Date: Wed, 8 Mar 2023 16:13:55 -0800 Subject: [PATCH 36/44] fbcode/cachelib/experimental/objcache2/tests Differential Revision: D43869046 fbshipit-source-id: 288bc98b8146424f71ece5d0685d08c59e3ed523 --- .../experimental/objcache2/tests/test_object.thrift | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/cachelib/experimental/objcache2/tests/test_object.thrift b/cachelib/experimental/objcache2/tests/test_object.thrift index bf6ad0743..a448014cd 100644 --- a/cachelib/experimental/objcache2/tests/test_object.thrift +++ b/cachelib/experimental/objcache2/tests/test_object.thrift @@ -17,13 +17,13 @@ namespace cpp2 facebook.cachelib.objcache2.test struct ThriftFoo { - 1: i32 a; - 2: i32 b; - 3: i32 c; + 1: i32 a; + 2: i32 b; + 3: i32 c; } struct ThriftFoo2 { - 1: i32 d; - 2: i32 e; - 3: i32 f; + 1: i32 d; + 2: i32 e; + 3: i32 f; } From d92910bc096906f94c0b9ef888e1b2d7fcdac68d Mon Sep 17 00:00:00 2001 From: generatedunixname226714639793621 Date: Wed, 8 Mar 2023 16:14:25 -0800 Subject: [PATCH 37/44] fbcode/cachelib/experimental/objcache/tests Differential Revision: D43869073 fbshipit-source-id: 52fea96db9cefe735ba56b564bf6af5566cd6d76 --- .../tests/ThriftCustomAllocator.thrift | 77 +++++++++++-------- 1 file changed, 44 insertions(+), 33 deletions(-) diff --git a/cachelib/experimental/objcache/tests/ThriftCustomAllocator.thrift b/cachelib/experimental/objcache/tests/ThriftCustomAllocator.thrift index 97f82d466..b1610b3aa 100644 --- a/cachelib/experimental/objcache/tests/ThriftCustomAllocator.thrift +++ b/cachelib/experimental/objcache/tests/ThriftCustomAllocator.thrift @@ -24,22 +24,26 @@ struct UseSimpleCustomAllocator { // A template type like map needs to use "cpp.template" to specify a replacement template 1: map< + // A concrete type like string needs to use "cpp.type" to specify a replacement type + string ( + cpp.use_allocator, + cpp.type = "facebook::cachelib::objcache::test::TestString", + ), + string ( + cpp.use_allocator, + cpp.type = "facebook::cachelib::objcache::test::TestString", + ) + > ( + cpp.use_allocator, + cpp.template = "facebook::cachelib::objcache::test::TestMap", + ) m; - // A concrete type like string needs to use "cpp.type" to specify a replacement type - string - (cpp.use_allocator, - cpp.type = "facebook::cachelib::objcache::test::TestString"), - - string - (cpp.use_allocator, - cpp.type = "facebook::cachelib::objcache::test::TestString") - - > (cpp.use_allocator, - cpp.template = "facebook::cachelib::objcache::test::TestMap") m; - - // Native types or types that do not allocate memory do NOT need custom allocator - 2: i32 m2; -} (cpp.allocator="facebook::cachelib::objcache::test::ScopedTestAllocator", cpp.allocator_via="m") + // Native types or types that do not allocate memory do NOT need custom allocator + 2: i32 m2; +} ( + cpp.allocator = "facebook::cachelib::objcache::test::ScopedTestAllocator", + cpp.allocator_via = "m", +) // TODO: thrift allocator propagation behavior is broken. Right now, for the following // myObj1 = myObj2; // even if the allocator copy-assignment propagation is false, myObj2's @@ -50,17 +54,21 @@ struct UseSimpleCustomAllocator { union UnionWithCustomAllocator { 1: map< - i32, - string - (cpp.use_allocator, - cpp.type = "facebook::cachelib::objcache::test::TestString") - > (cpp.use_allocator, - cpp.template = "facebook::cachelib::objcache::test::TestMap")m1; - 2: string - (cpp.use_allocator, - cpp.type = "facebook::cachelib::objcache::test::TestString") m2; + i32, + string ( + cpp.use_allocator, + cpp.type = "facebook::cachelib::objcache::test::TestString", + ) + > ( + cpp.use_allocator, + cpp.template = "facebook::cachelib::objcache::test::TestMap", + ) m1; + 2: string ( + cpp.use_allocator, + cpp.type = "facebook::cachelib::objcache::test::TestString", + ) m2; 3: i32 m3; -} (cpp.allocator="facebook::cachelib::objcache::test::ScopedTestAllocator") +} (cpp.allocator = "facebook::cachelib::objcache::test::ScopedTestAllocator") // TODO: even though thrift union does not support allocator. We still need to // annotate it with allocator so it has a `get_allocator()` method so // that when deserializing it will be able to pass an allocator an inner @@ -89,11 +97,14 @@ union UnionWithCustomAllocator { // } struct UseTwoF14Maps { - 1: map - (cpp.use_allocator, - cpp.template = "facebook::cachelib::objcache::test::TestFollyF14FastMap") m1; - 2: map - (cpp.use_allocator, - cpp.template = "facebook::cachelib::objcache::test::TestFollyF14FastMap") m2; -} (cpp.allocator= - "facebook::cachelib::objcache::test::TestF14TemplateAllocator>") + 1: map ( + cpp.use_allocator, + cpp.template = "facebook::cachelib::objcache::test::TestFollyF14FastMap", + ) m1; + 2: map ( + cpp.use_allocator, + cpp.template = "facebook::cachelib::objcache::test::TestFollyF14FastMap", + ) m2; +} ( + cpp.allocator = "facebook::cachelib::objcache::test::TestF14TemplateAllocator>", +) From 1e05a161e84c2bb341903ff4e5cdc1496c87b9e0 Mon Sep 17 00:00:00 2001 From: generatedunixname226714639793621 Date: Wed, 8 Mar 2023 16:43:58 -0800 Subject: [PATCH 38/44] fbcode/cachelib/navy/serialization Differential Revision: D43869045 fbshipit-source-id: 120149fa4cac2590b93f0c9a9fa5f7827efca3fc --- cachelib/navy/serialization/objects.thrift | 90 +++++++++++----------- 1 file changed, 45 insertions(+), 45 deletions(-) diff --git a/cachelib/navy/serialization/objects.thrift b/cachelib/navy/serialization/objects.thrift index 32be4bd17..887c90c6b 100644 --- a/cachelib/navy/serialization/objects.thrift +++ b/cachelib/navy/serialization/objects.thrift @@ -17,78 +17,78 @@ namespace cpp2 facebook.cachelib.navy.serialization struct IndexEntry { - 1: required i32 key = 0, - 2: required i32 address = 0, - 3: i16 sizeHint = 0, - 4: byte totalHits = 0, - 5: byte currentHits = 0, + 1: required i32 key = 0; + 2: required i32 address = 0; + 3: i16 sizeHint = 0; + 4: byte totalHits = 0; + 5: byte currentHits = 0; } struct IndexBucket { - 1: required i32 bucketId = 0, - 2: required list entries, + 1: required i32 bucketId = 0; + 2: required list entries; } struct Region { - 1: required i32 regionId = 0, - 2: required i32 lastEntryEndOffset = 0, - 3: required i32 classId = 0, - 4: required i32 numItems = 0, - 5: required bool pinned = false, - 6: i32 priority = 0, + 1: required i32 regionId = 0; + 2: required i32 lastEntryEndOffset = 0; + 3: required i32 classId = 0; + 4: required i32 numItems = 0; + 5: required bool pinned = false; + 6: i32 priority = 0; } struct RegionData { - 1: required list regions, - 2: required i32 regionSize = 0, + 1: required list regions; + 2: required i32 regionSize = 0; } struct FifoPolicyNodeData { - 1: required i32 idx, - 2: required i64 trackTime, + 1: required i32 idx; + 2: required i64 trackTime; } -struct FifoPolicyData{ - 1: required list queue, +struct FifoPolicyData { + 1: required list queue; } struct AccessStats { - 1: byte totalHits = 0, - 2: byte currHits = 0, - 3: byte numReinsertions = 0, + 1: byte totalHits = 0; + 2: byte currHits = 0; + 3: byte numReinsertions = 0; } struct AccessStatsPair { - 1: i64 key, - 2: AccessStats stats, + 1: i64 key; + 2: AccessStats stats; } struct AccessTracker { - 1: map deprecated_data, - 2: list data, + 1: map deprecated_data; + 2: list data; } struct BlockCacheConfig { - 1: required i64 version = 0, - 2: required i64 cacheBaseOffset = 0, - 3: required i64 cacheSize = 0, - 4: required i32 allocAlignSize = 0, - 5: required set deprecated_sizeClasses, - 6: required bool checksum = false, - 7: map deprecated_sizeDist, - 8: i64 holeCount = 0, - 9: i64 holeSizeTotal = 0, - 10: bool reinsertionPolicyEnabled = false, - 11: i64 usedSizeBytes = 0, + 1: required i64 version = 0; + 2: required i64 cacheBaseOffset = 0; + 3: required i64 cacheSize = 0; + 4: required i32 allocAlignSize = 0; + 5: required set deprecated_sizeClasses; + 6: required bool checksum = false; + 7: map deprecated_sizeDist; + 8: i64 holeCount = 0; + 9: i64 holeSizeTotal = 0; + 10: bool reinsertionPolicyEnabled = false; + 11: i64 usedSizeBytes = 0; } struct BigHashPersistentData { - 1: required i32 version = 0, - 2: required i64 generationTime = 0, - 3: required i64 itemCount = 0, - 4: required i64 bucketSize = 0, - 5: required i64 cacheBaseOffset = 0, - 6: required i64 numBuckets = 0, - 7: map deprecated_sizeDist, - 8: i64 usedSizeBytes = 0, + 1: required i32 version = 0; + 2: required i64 generationTime = 0; + 3: required i64 itemCount = 0; + 4: required i64 bucketSize = 0; + 5: required i64 cacheBaseOffset = 0; + 6: required i64 numBuckets = 0; + 7: map deprecated_sizeDist; + 8: i64 usedSizeBytes = 0; } From 0bc9f66f72b017616970f8dac7b2d87aeadc8692 Mon Sep 17 00:00:00 2001 From: Igor Chorazewicz Date: Wed, 8 Mar 2023 23:00:16 -0800 Subject: [PATCH 39/44] Introduce 'markedForEviction' state for the Item. (#183) Summary: It is similar to 'moving' but requires ref count to be 0. An item which is marked for eviction causes all incRef() calls to that item to fail. This will be used to ensure that once item is selected for eviction, no one can interfere and prevent the eviction from suceeding. 'markedForEviction' relies on the same 'exlusive' bit as the 'moving' state. To distinguish between those two states, 'moving' add 1 to the refCount. This is hidden from the user, so getRefCount() will not return that extra ref. Pull Request resolved: https://github.com/facebook/CacheLib/pull/183 Test Plan: flatmemcache shadow result: It seems there is a slight latency regression in allocation (~1 micro second) https://fburl.com/ods/u3isn8b4 It's probably small enough to ignore. But we can come back if this starts showing up in large scale in production. Imported from GitHub, without a `Test Plan:` line. Reviewed By: therealgymmy Differential Revision: D42089974 Pulled By: haowu14 fbshipit-source-id: 90477faa9443080a7ddefa37a524965a04b6f084 --- cachelib/allocator/CacheAllocator-inl.h | 88 ++++--- cachelib/allocator/CacheAllocator.h | 10 +- cachelib/allocator/CacheItem-inl.h | 56 ++-- cachelib/allocator/CacheItem.h | 57 ++-- cachelib/allocator/Refcount.h | 305 +++++++++++++++------- cachelib/allocator/tests/ItemTest.cpp | 14 +- cachelib/allocator/tests/RefCountTest.cpp | 102 ++++++-- 7 files changed, 450 insertions(+), 182 deletions(-) diff --git a/cachelib/allocator/CacheAllocator-inl.h b/cachelib/allocator/CacheAllocator-inl.h index 1d8959326..b8d0256d5 100644 --- a/cachelib/allocator/CacheAllocator-inl.h +++ b/cachelib/allocator/CacheAllocator-inl.h @@ -832,20 +832,21 @@ CacheAllocator::releaseBackToAllocator(Item& it, removeFromMMContainer(*head); - // If this chained item is marked as exclusive, we will not free it. - // We must capture the exclusive state before we do the decRef when + // If this chained item is marked as moving, we will not free it. + // We must capture the moving state before we do the decRef when // we know the item must still be valid - const bool wasExclusive = head->isExclusive(); + const bool wasMoving = head->isMoving(); + XDCHECK(!head->isMarkedForEviction()); // Decref and check if we were the last reference. Now if the item - // was marked exclusive, after decRef, it will be free to be released + // was marked moving, after decRef, it will be free to be released // by slab release thread const auto childRef = head->decRef(); - // If the item is already exclusive and we already decremented the + // If the item is already moving and we already decremented the // refcount, we don't need to free this item. We'll let the slab // release thread take care of that - if (!wasExclusive) { + if (!wasMoving) { if (childRef != 0) { throw std::runtime_error(folly::sformat( "chained item refcount is not zero. We cannot proceed! " @@ -853,7 +854,7 @@ CacheAllocator::releaseBackToAllocator(Item& it, childRef, head->toString())); } - // Item is not exclusive and refcount is 0, we can proceed to + // Item is not moving and refcount is 0, we can proceed to // free it or recylce the memory if (head == toRecycle) { XDCHECK(ReleaseRes::kReleased != res); @@ -881,9 +882,12 @@ CacheAllocator::releaseBackToAllocator(Item& it, } template -void CacheAllocator::incRef(Item& it) { - it.incRef(); - ++handleCount_.tlStats(); +bool CacheAllocator::incRef(Item& it) { + if (it.incRef()) { + ++handleCount_.tlStats(); + return true; + } + return false; } template @@ -903,8 +907,12 @@ CacheAllocator::acquire(Item* it) { SCOPE_FAIL { stats_.numRefcountOverflow.inc(); }; - incRef(*it); - return WriteHandle{it, *this}; + if (LIKELY(incRef(*it))) { + return WriteHandle{it, *this}; + } else { + // item is being evicted + return WriteHandle{}; + } } template @@ -1179,7 +1187,7 @@ bool CacheAllocator::moveChainedItem(ChainedItem& oldItem, // This item has been unlinked from its parent and we're the only // owner of it, so we're done here - if (!oldItem.isInMMContainer() || oldItem.isOnlyExclusive()) { + if (!oldItem.isInMMContainer() || oldItem.isOnlyMoving()) { return false; } @@ -1210,7 +1218,7 @@ bool CacheAllocator::moveChainedItem(ChainedItem& oldItem, // In case someone else had removed this chained item from its parent by now // So we check again to see if the it has been unlinked from its parent - if (!oldItem.isInMMContainer() || oldItem.isOnlyExclusive()) { + if (!oldItem.isInMMContainer() || oldItem.isOnlyMoving()) { return false; } @@ -1226,7 +1234,7 @@ bool CacheAllocator::moveChainedItem(ChainedItem& oldItem, // parent's chain and the MMContainer. auto oldItemHandle = replaceChainedItemLocked(oldItem, std::move(newItemHdl), *parentHandle); - XDCHECK(oldItemHandle->isExclusive()); + XDCHECK(oldItemHandle->isMoving()); XDCHECK(!oldItemHandle->isInMMContainer()); return true; @@ -1255,7 +1263,7 @@ CacheAllocator::findEviction(PoolId pid, ClassId cid) { : toRecycle; // make sure no other thead is evicting the item - if (candidate->getRefCount() != 0 || !candidate->markExclusive()) { + if (candidate->getRefCount() != 0 || !candidate->markMoving()) { ++itr; continue; } @@ -1270,11 +1278,11 @@ CacheAllocator::findEviction(PoolId pid, ClassId cid) { ? advanceIteratorAndTryEvictChainedItem(itr) : advanceIteratorAndTryEvictRegularItem(mmContainer, itr); evictionSuccessful = toReleaseHandle != nullptr; - // destroy toReleseHandle. The item won't be released to allocator - // since we marked it as exclusive. + // destroy toReleaseHandle. The item won't be released to allocator + // since we marked for eviction. } - const auto ref = candidate->unmarkExclusive(); + const auto ref = candidate->unmarkMoving(); if (ref == 0u) { // Invalidate iterator since later on we may use this mmContainer // again, which cannot be done unless we drop this iterator @@ -2361,7 +2369,7 @@ void CacheAllocator::releaseSlabImpl( // Need to mark an item for release before proceeding // If we can't mark as moving, it means the item is already freed const bool isAlreadyFreed = - !markExclusiveForSlabRelease(releaseContext, alloc, throttler); + !markMovingForSlabRelease(releaseContext, alloc, throttler); if (isAlreadyFreed) { continue; } @@ -2406,8 +2414,8 @@ bool CacheAllocator::moveForSlabRelease( stats_.numMoveAttempts.inc(); // Nothing to move and the key is likely also bogus for chained items. - if (oldItem.isOnlyExclusive()) { - oldItem.unmarkExclusive(); + if (oldItem.isOnlyMoving()) { + oldItem.unmarkMoving(); const auto res = releaseBackToAllocator(oldItem, RemoveContext::kNormal, false); XDCHECK(res == ReleaseRes::kReleased); @@ -2446,7 +2454,7 @@ bool CacheAllocator::moveForSlabRelease( // that's identical to this one to replace it. Here we just need to wait // until all users have dropped the item handles before we can proceed. startTime = util::getCurrentTimeSec(); - while (!oldItem.isOnlyExclusive()) { + while (!oldItem.isOnlyMoving()) { throttleWith(throttler, [&] { XLOGF(WARN, "Spent {} seconds, slab release still waiting for refcount to " @@ -2500,8 +2508,8 @@ CacheAllocator::allocateNewItemForOldItem(const Item& oldItem) { return {}; } - // Set up the destination for the move. Since oldChainedItem would have - // the exclusive bit set, it won't be picked for eviction. + // Set up the destination for the move. Since oldChainedItem would be + // marked as moving, it won't be picked for eviction. auto newItemHdl = allocateChainedItemInternal(parentHandle, oldChainedItem.getSize()); if (!newItemHdl) { @@ -2553,7 +2561,7 @@ bool CacheAllocator::tryMovingForSlabRelease( // item is still valid. const std::string parentKey = oldItem.asChainedItem().getParentItem(compressor_).getKey().str(); - if (oldItem.isOnlyExclusive()) { + if (oldItem.isOnlyMoving()) { // If chained item no longer has a refcount, its parent is already // being released, so we abort this try to moving. return false; @@ -2583,10 +2591,10 @@ void CacheAllocator::evictForSlabRelease( stats_.numEvictionAttempts.inc(); // if the item is already in a state where only the exclusive bit is set, - // nothing needs to be done. We simply need to unmark exclusive bit and free + // nothing needs to be done. We simply need to call unmarkMoving and free // the item. - if (item.isOnlyExclusive()) { - item.unmarkExclusive(); + if (item.isOnlyMoving()) { + item.unmarkMoving(); const auto res = releaseBackToAllocator(item, RemoveContext::kNormal, false); XDCHECK(ReleaseRes::kReleased == res); @@ -2617,7 +2625,7 @@ void CacheAllocator::evictForSlabRelease( stats_.numEvictionSuccesses.inc(); // we have the last handle. no longer need to hold on to the exclusive bit - item.unmarkExclusive(); + item.unmarkMoving(); // manually decrement the refcount to call releaseBackToAllocator const auto ref = decRef(*owningHandle); @@ -2629,7 +2637,7 @@ void CacheAllocator::evictForSlabRelease( } if (shutDownInProgress_) { - item.unmarkExclusive(); + item.unmarkMoving(); allocator_->abortSlabRelease(ctx); throw exception::SlabReleaseAborted( folly::sformat("Slab Release aborted while trying to evict" @@ -2775,9 +2783,9 @@ CacheAllocator::advanceIteratorAndTryEvictChainedItem( template typename CacheAllocator::WriteHandle CacheAllocator::evictNormalItemForSlabRelease(Item& item) { - XDCHECK(item.isExclusive()); + XDCHECK(item.isMoving()); - if (item.isOnlyExclusive()) { + if (item.isOnlyMoving()) { return WriteHandle{}; } @@ -2789,7 +2797,7 @@ CacheAllocator::evictNormalItemForSlabRelease(Item& item) { // We remove the item from both access and mm containers. It doesn't matter // if someone else calls remove on the item at this moment, the item cannot - // be freed as long as we have the exclusive bit set. + // be freed as long as it's marked for eviction. auto handle = accessContainer_->removeIf(item, std::move(predicate)); if (!handle) { @@ -2813,7 +2821,7 @@ CacheAllocator::evictNormalItemForSlabRelease(Item& item) { template typename CacheAllocator::WriteHandle CacheAllocator::evictChainedItemForSlabRelease(ChainedItem& child) { - XDCHECK(child.isExclusive()); + XDCHECK(child.isMoving()); // We have the child marked as moving, but dont know anything about the // state of the parent. Unlike the case of regular eviction where we are @@ -2835,7 +2843,7 @@ CacheAllocator::evictChainedItemForSlabRelease(ChainedItem& child) { // check if the child is still in mmContainer and the expected parent is // valid under the chained item lock. if (expectedParent.getKey() != parentKey || !child.isInMMContainer() || - child.isOnlyExclusive() || + child.isOnlyMoving() || &expectedParent != &child.getParentItem(compressor_) || !expectedParent.isAccessible() || !expectedParent.hasChainedItem()) { return {}; @@ -2890,14 +2898,14 @@ CacheAllocator::evictChainedItemForSlabRelease(ChainedItem& child) { // In case someone else had removed this chained item from its parent by now // So we check again to see if it has been unlinked from its parent - if (!child.isInMMContainer() || child.isOnlyExclusive()) { + if (!child.isInMMContainer() || child.isOnlyMoving()) { return {}; } // check after removing from the MMContainer that the parent is still not // being marked as moving. If parent is moving, it will release the child // item and we will wait for that. - if (parentHandle->isExclusive()) { + if (parentHandle->isMoving()) { return {}; } @@ -2930,7 +2938,7 @@ bool CacheAllocator::removeIfExpired(const ReadHandle& handle) { } template -bool CacheAllocator::markExclusiveForSlabRelease( +bool CacheAllocator::markMovingForSlabRelease( const SlabReleaseContext& ctx, void* alloc, util::Throttler& throttler) { // MemoryAllocator::processAllocForRelease will execute the callback // if the item is not already free. So there are three outcomes here: @@ -2949,7 +2957,7 @@ bool CacheAllocator::markExclusiveForSlabRelease( // Since this callback is executed, the item is not yet freed itemFreed = false; Item* item = static_cast(memory); - if (item->markExclusive()) { + if (item->markMoving()) { markedMoving = true; } }; diff --git a/cachelib/allocator/CacheAllocator.h b/cachelib/allocator/CacheAllocator.h index ed0096390..32d44c138 100644 --- a/cachelib/allocator/CacheAllocator.h +++ b/cachelib/allocator/CacheAllocator.h @@ -1308,7 +1308,7 @@ class CacheAllocator : public CacheBase { private: // wrapper around Item's refcount and active handle tracking - FOLLY_ALWAYS_INLINE void incRef(Item& it); + FOLLY_ALWAYS_INLINE bool incRef(Item& it); FOLLY_ALWAYS_INLINE RefcountWithFlags::Value decRef(Item& it); // drops the refcount and if needed, frees the allocation back to the memory @@ -1756,9 +1756,9 @@ class CacheAllocator : public CacheBase { // @return true when successfully marked as moving, // fasle when this item has already been freed - bool markExclusiveForSlabRelease(const SlabReleaseContext& ctx, - void* alloc, - util::Throttler& throttler); + bool markMovingForSlabRelease(const SlabReleaseContext& ctx, + void* alloc, + util::Throttler& throttler); // "Move" (by copying) the content in this item to another memory // location by invoking the move callback. @@ -1937,7 +1937,7 @@ class CacheAllocator : public CacheBase { } static bool parentEvictForSlabReleasePredicate(const Item& item) { - return item.getRefCount() == 1 && !item.isExclusive(); + return item.getRefCount() == 1 && !item.isMoving(); } std::unique_ptr createDeserializer(); diff --git a/cachelib/allocator/CacheItem-inl.h b/cachelib/allocator/CacheItem-inl.h index f59fa9d59..bf77b43aa 100644 --- a/cachelib/allocator/CacheItem-inl.h +++ b/cachelib/allocator/CacheItem-inl.h @@ -148,15 +148,16 @@ std::string CacheItem::toString() const { return folly::sformat( "item: " "memory={}:raw-ref={}:size={}:key={}:hex-key={}:" - "isInMMContainer={}:isAccessible={}:isExclusive={}:references={}:ctime=" + "isInMMContainer={}:isAccessible={}:isMarkedForEviction={}:" + "isMoving={}:references={}:ctime=" "{}:" "expTime={}:updateTime={}:isNvmClean={}:isNvmEvicted={}:hasChainedItem=" "{}", this, getRefCountAndFlagsRaw(), getSize(), folly::humanify(getKey().str()), folly::hexlify(getKey()), - isInMMContainer(), isAccessible(), isExclusive(), getRefCount(), - getCreationTime(), getExpiryTime(), getLastAccessTime(), isNvmClean(), - isNvmEvicted(), hasChainedItem()); + isInMMContainer(), isAccessible(), isMarkedForEviction(), isMoving(), + getRefCount(), getCreationTime(), getExpiryTime(), getLastAccessTime(), + isNvmClean(), isNvmEvicted(), hasChainedItem()); } } @@ -217,23 +218,43 @@ bool CacheItem::isInMMContainer() const noexcept { } template -bool CacheItem::markExclusive() noexcept { - return ref_.markExclusive(); +bool CacheItem::markForEviction() noexcept { + return ref_.markForEviction(); } template -RefcountWithFlags::Value CacheItem::unmarkExclusive() noexcept { - return ref_.unmarkExclusive(); +RefcountWithFlags::Value CacheItem::unmarkForEviction() noexcept { + return ref_.unmarkForEviction(); } template -bool CacheItem::isExclusive() const noexcept { - return ref_.isExclusive(); +bool CacheItem::isMarkedForEviction() const noexcept { + return ref_.isMarkedForEviction(); } template -bool CacheItem::isOnlyExclusive() const noexcept { - return ref_.isOnlyExclusive(); +bool CacheItem::markForEvictionWhenMoving() { + return ref_.markForEvictionWhenMoving(); +} + +template +bool CacheItem::markMoving() { + return ref_.markMoving(); +} + +template +RefcountWithFlags::Value CacheItem::unmarkMoving() noexcept { + return ref_.unmarkMoving(); +} + +template +bool CacheItem::isMoving() const noexcept { + return ref_.isMoving(); +} + +template +bool CacheItem::isOnlyMoving() const noexcept { + return ref_.isOnlyMoving(); } template @@ -335,7 +356,8 @@ bool CacheItem::updateExpiryTime(uint32_t expiryTimeSecs) noexcept { // check for moving to make sure we are not updating the expiry time while at // the same time re-allocating the item with the old state of the expiry time // in moveRegularItem(). See D6852328 - if (isExclusive() || !isInMMContainer() || isChainedItem()) { + if (isMoving() || isMarkedForEviction() || !isInMMContainer() || + isChainedItem()) { return false; } // attempt to atomically update the value of expiryTime @@ -451,12 +473,14 @@ std::string CacheChainedItem::toString() const { return folly::sformat( "chained item: " "memory={}:raw-ref={}:size={}:parent-compressed-ptr={}:" - "isInMMContainer={}:isAccessible={}:isExclusive={}:references={}:ctime={}" + "isInMMContainer={}:isAccessible={}:isMarkedForEviction={}:" + "isMoving={}:references={}:ctime={}" ":" "expTime={}:updateTime={}", this, Item::getRefCountAndFlagsRaw(), Item::getSize(), cPtr.getRaw(), - Item::isInMMContainer(), Item::isAccessible(), Item::isExclusive(), - Item::getRefCount(), Item::getCreationTime(), Item::getExpiryTime(), + Item::isInMMContainer(), Item::isAccessible(), + Item::isMarkedForEviction(), Item::isMoving(), Item::getRefCount(), + Item::getCreationTime(), Item::getExpiryTime(), Item::getLastAccessTime()); } diff --git a/cachelib/allocator/CacheItem.h b/cachelib/allocator/CacheItem.h index 06136db03..afee315cb 100644 --- a/cachelib/allocator/CacheItem.h +++ b/cachelib/allocator/CacheItem.h @@ -305,12 +305,17 @@ class CACHELIB_PACKED_ATTR CacheItem { */ RefcountWithFlags::Value getRefCountAndFlagsRaw() const noexcept; - FOLLY_ALWAYS_INLINE void incRef() { - if (LIKELY(ref_.incRef())) { - return; + // Increments item's ref count + // + // @return true on success, failure if item is marked as exclusive + // @throw exception::RefcountOverflow on ref count overflow + FOLLY_ALWAYS_INLINE bool incRef() { + try { + return ref_.incRef(); + } catch (exception::RefcountOverflow& e) { + throw exception::RefcountOverflow( + folly::sformat("{} item: {}", e.what(), toString())); } - throw exception::RefcountOverflow( - folly::sformat("Refcount maxed out. item: {}", toString())); } FOLLY_ALWAYS_INLINE RefcountWithFlags::Value decRef() { @@ -344,23 +349,43 @@ class CACHELIB_PACKED_ATTR CacheItem { /** * The following two functions corresond to whether or not an item is - * currently in the process of being moved. This happens during a slab - * rebalance, eviction or resize operation. + * currently in the process of being evicted. * - * An item can only be marked exclusive when `isInMMContainer` returns true. + * An item can only be marked exclusive when `isInMMContainer` returns true + * and item is not already exclusive nor moving and the ref count is 0. * This operation is atomic. * - * User can also query if an item "isOnlyExclusive". This returns true only - * if the refcount is 0 and only the exclusive bit is set. - * - * Unmarking exclusive does not depend on `isInMMContainer`. + * Unmarking exclusive does not depend on `isInMMContainer` * Unmarking exclusive will also return the refcount at the moment of * unmarking. */ - bool markExclusive() noexcept; - RefcountWithFlags::Value unmarkExclusive() noexcept; - bool isExclusive() const noexcept; - bool isOnlyExclusive() const noexcept; + bool markForEviction() noexcept; + RefcountWithFlags::Value unmarkForEviction() noexcept; + bool isMarkedForEviction() const noexcept; + + /** + * The following functions correspond to whether or not an item is + * currently in the processed of being moved. When moving, ref count + * is always >= 1. + * + * An item can only be marked moving when `isInMMContainer` returns true + * and item is not already exclusive nor moving. + * + * User can also query if an item "isOnlyMoving". This returns true only + * if the refcount is one and only the exclusive bit is set. + * + * Unmarking moving does not depend on `isInMMContainer` + * Unmarking moving will also return the refcount at the moment of + * unmarking. + */ + bool markMoving(); + RefcountWithFlags::Value unmarkMoving() noexcept; + bool isMoving() const noexcept; + bool isOnlyMoving() const noexcept; + + /** This function attempts to mark item as exclusive. + * Can only be called on the item that is moving.*/ + bool markForEvictionWhenMoving(); /** * Item cannot be marked both chained allocation and diff --git a/cachelib/allocator/Refcount.h b/cachelib/allocator/Refcount.h index c60dea34f..107e10735 100644 --- a/cachelib/allocator/Refcount.h +++ b/cachelib/allocator/Refcount.h @@ -132,32 +132,28 @@ class FOLLY_PACK_ATTR RefcountWithFlags { RefcountWithFlags& operator=(RefcountWithFlags&&) = delete; // Bumps up the reference count only if the new count will be strictly less - // than or equal to the maxCount. - // @return true if refcount is bumped. false otherwise. - FOLLY_ALWAYS_INLINE bool incRef() noexcept { - Value* const refPtr = &refCount_; - unsigned int nCASFailures = 0; - constexpr bool isWeak = false; - Value oldVal = __atomic_load_n(refPtr, __ATOMIC_RELAXED); - - while (true) { - const Value newCount = oldVal + static_cast(1); - if (UNLIKELY((oldVal & kAccessRefMask) == (kAccessRefMask))) { - return false; + // than or equal to the maxCount and the item is not exclusive + // @return true if refcount is bumped. false otherwise (if item is exclusive) + // @throw exception::RefcountOverflow if new count would be greater than + // maxCount + FOLLY_ALWAYS_INLINE bool incRef() { + auto predicate = [](const Value curValue) { + Value bitMask = getAdminRef(); + + const bool exlusiveBitIsSet = curValue & bitMask; + if (UNLIKELY((curValue & kAccessRefMask) == (kAccessRefMask))) { + throw exception::RefcountOverflow("Refcount maxed out."); } - if (__atomic_compare_exchange_n(refPtr, &oldVal, newCount, isWeak, - __ATOMIC_ACQ_REL, __ATOMIC_RELAXED)) { - return true; - } + // Check if the item is not marked for eviction + return !exlusiveBitIsSet || ((curValue & kAccessRefMask) != 0); + }; - if ((++nCASFailures % 4) == 0) { - // this pause takes up to 40 clock cycles on intel and the lock cmpxchgl - // above should take about 100 clock cycles. we pause once every 400 - // cycles or so if we are extremely unlucky. - folly::asm_volatile_pause(); - } - } + auto newValue = [](const Value curValue) { + return (curValue + static_cast(1)); + }; + + return atomicUpdateValue(predicate, newValue); } // Bumps down the reference count @@ -167,33 +163,38 @@ class FOLLY_PACK_ATTR RefcountWithFlags { // @throw RefcountUnderflow when we are trying to decremenet from 0 // refcount and have a refcount leak. FOLLY_ALWAYS_INLINE Value decRef() { - Value* const refPtr = &refCount_; - unsigned int nCASFailures = 0; - constexpr bool isWeak = false; - - Value oldVal = __atomic_load_n(refPtr, __ATOMIC_RELAXED); - while (true) { - const Value newCount = oldVal - static_cast(1); - if ((oldVal & kAccessRefMask) == 0) { + auto predicate = [](const Value curValue) { + if ((curValue & kAccessRefMask) == 0) { throw exception::RefcountUnderflow( "Trying to decRef with no refcount. RefCount Leak!"); } + return true; + }; - if (__atomic_compare_exchange_n(refPtr, &oldVal, newCount, isWeak, - __ATOMIC_ACQ_REL, __ATOMIC_RELAXED)) { - return newCount & kRefMask; - } - if ((++nCASFailures % 4) == 0) { - // this pause takes up to 40 clock cycles on intel and the lock cmpxchgl - // above should take about 100 clock cycles. we pause once every 400 - // cycles or so if we are extremely unlucky - folly::asm_volatile_pause(); - } - } + Value retValue; + auto newValue = [&retValue](const Value curValue) { + retValue = (curValue - static_cast(1)); + return retValue; + }; + + auto updated = atomicUpdateValue(predicate, newValue); + XDCHECK(updated); + + return retValue & kRefMask; } - // Return refcount excluding control bits and flags - Value getAccessRef() const noexcept { return getRaw() & kAccessRefMask; } + // Return refcount excluding moving refcount, control bits and flags. + Value getAccessRef() const noexcept { + auto raw = getRaw(); + auto accessRef = raw & kAccessRefMask; + + if ((raw & getAdminRef()) && accessRef >= 1) { + // if item is moving, ignore the extra ref + return accessRef - static_cast(1); + } else { + return accessRef; + } + } // Return access ref and the admin ref bits Value getRefWithAccessAndAdmin() const noexcept { @@ -246,65 +247,160 @@ class FOLLY_PACK_ATTR RefcountWithFlags { } /** - * The following four functions are used to track whether or not - * an item is currently in the process of being moved. This happens during a - * slab rebalance or resize operation or during eviction. + * The following two functions correspond to whether or not an item is + * currently in the process of being evicted. * - * An item can only be marked exclusive when `isInMMContainer` returns true - * and the item is not yet marked as exclusive. This operation is atomic. + * An item that is marked for eviction prevents from obtaining a handle to + * the item (incRef() will return false). This guarantees that eviction of + * marked item will always suceed. * - * User can also query if an item "isOnlyExclusive". This returns true only - * if the refcount is 0 and only the exclusive bit is set. + * An item can only be marked for eviction when `isInMMContainer` returns true + * and item does not have `kExclusive` bit set and access ref count is 0. + * This operation is atomic. * - * Unmarking exclusive does not depend on `isInMMContainer`. - * Unmarking exclusive will also return the refcount at the moment of - * unmarking. + * When item is marked for eviction, `kExclusive` bit is set and ref count is + * zero. + * + * Unmarking for eviction clears the `kExclusive` bit. `unamrkForEviction` + * does not depend on `isInMMContainer` nor `isAccessible` */ - bool markExclusive() noexcept { - Value bitMask = getAdminRef(); - Value conditionBitMask = getAdminRef(); + bool markForEviction() noexcept { + Value linkedBitMask = getAdminRef(); + Value exclusiveBitMask = getAdminRef(); - Value* const refPtr = &refCount_; - unsigned int nCASFailures = 0; - constexpr bool isWeak = false; - Value curValue = __atomic_load_n(refPtr, __ATOMIC_RELAXED); - while (true) { - const bool flagSet = curValue & conditionBitMask; - const bool alreadyExclusive = curValue & bitMask; - if (!flagSet || alreadyExclusive) { + auto predicate = [linkedBitMask, exclusiveBitMask](const Value curValue) { + const bool unlinked = !(curValue & linkedBitMask); + const bool alreadyExclusive = curValue & exclusiveBitMask; + + if (unlinked || alreadyExclusive) { return false; } - - const Value newValue = curValue | bitMask; - if (__atomic_compare_exchange_n(refPtr, &curValue, newValue, isWeak, - __ATOMIC_ACQ_REL, __ATOMIC_RELAXED)) { - XDCHECK(newValue & conditionBitMask); - return true; + if ((curValue & kAccessRefMask) != 0) { + return false; } - if ((++nCASFailures % 4) == 0) { - // this pause takes up to 40 clock cycles on intel and the lock cmpxchgl - // above should take about 100 clock cycles. we pause once every 400 - // cycles or so if we are extremely unlucky. - folly::asm_volatile_pause(); - } - } + return true; + }; + + auto newValue = [exclusiveBitMask](const Value curValue) { + return curValue | exclusiveBitMask; + }; + + return atomicUpdateValue(predicate, newValue); } - Value unmarkExclusive() noexcept { + + Value unmarkForEviction() noexcept { + XDCHECK(isMarkedForEviction()); Value bitMask = ~getAdminRef(); return __atomic_and_fetch(&refCount_, bitMask, __ATOMIC_ACQ_REL) & kRefMask; } - bool isExclusive() const noexcept { - return getRaw() & getAdminRef(); + + bool isMarkedForEviction() const noexcept { + auto raw = getRaw(); + return (raw & getAdminRef()) && ((raw & kAccessRefMask) == 0); + } + + /** + * The following functions correspond to whether or not an item is + * currently in the processed of being moved. + * + * A `moving` item cannot be recycled nor freed to the allocator. It has + * to be unmarked first. + * + * When moving, internal ref count is always >= 1 and `kExclusive` bit is set + * getRefCount does not return the extra ref (it may return 0). + * + * An item can only be marked moving when `isInMMContainer` returns true + * and does not have `kExclusive` bit set. + * + * User can also query if an item "isOnlyMoving". This returns true only + * if the refcount is one and only the exlusive bit is set. + * + * Unmarking clears `kExclusive` bit and decreses the interanl refCount by 1. + * `unmarkMoving` does does not depend on `isInMMContainer` + */ + bool markMoving() { + Value linkedBitMask = getAdminRef(); + Value exclusiveBitMask = getAdminRef(); + + auto predicate = [linkedBitMask, exclusiveBitMask](const Value curValue) { + const bool unlinked = !(curValue & linkedBitMask); + const bool alreadyExclusive = curValue & exclusiveBitMask; + + if (unlinked || alreadyExclusive) { + return false; + } + if (UNLIKELY((curValue & kAccessRefMask) == (kAccessRefMask))) { + throw exception::RefcountOverflow("Refcount maxed out."); + } + + return true; + }; + + auto newValue = [exclusiveBitMask](const Value curValue) { + // Set exclusive flag and make the ref count non-zero (to distinguish + // from exclusive case). This extra ref will not be reported to the + // user + return (curValue + static_cast(1)) | exclusiveBitMask; + }; + + return atomicUpdateValue(predicate, newValue); + } + + Value unmarkMoving() noexcept { + XDCHECK(isMoving()); + auto predicate = [](const Value curValue) { + XDCHECK((curValue & kAccessRefMask) != 0); + return true; + }; + + Value retValue; + auto newValue = [&retValue](const Value curValue) { + retValue = + (curValue - static_cast(1)) & ~getAdminRef(); + return retValue; + }; + + auto updated = atomicUpdateValue(predicate, newValue); + XDCHECK(updated); + + return retValue & kRefMask; + } + + bool isMoving() const noexcept { + auto raw = getRaw(); + return (raw & getAdminRef()) && ((raw & kAccessRefMask) != 0); + } + + /** + * This function attempts to mark item for eviction. + * Can only be called on the item that is moving. + * + * Returns true and marks the item for eviction only if item isOnlyMoving. + * Leaves the item marked as moving and returns false otherwise. + */ + bool markForEvictionWhenMoving() { + XDCHECK(isMoving()); + + auto predicate = [](const Value curValue) { + return (curValue & kAccessRefMask) == 1; + }; + + auto newValue = [](const Value curValue) { + XDCHECK((curValue & kAccessRefMask) == 1); + return (curValue - static_cast(1)); + }; + + return atomicUpdateValue(predicate, newValue); } - bool isOnlyExclusive() const noexcept { - // An item is only exclusive when its refcount is zero and only the - // exclusive bit among all the control bits is set. This indicates an item - // is exclusive to the current thread. No other thread is allowed to - // do anything with it. + + bool isOnlyMoving() const noexcept { + // An item is only moving when its refcount is one and only the exclusive + // bit among all the control bits is set. This indicates an item is already + // on its way out of cache. auto ref = getRefWithAccessAndAdmin(); - bool anyOtherBitSet = ref & ~getAdminRef(); - if (anyOtherBitSet) { + Value valueWithoutExclusiveBit = ref & ~getAdminRef(); + if (valueWithoutExclusiveBit != 1) { return false; } return ref & getAdminRef(); @@ -370,6 +466,39 @@ class FOLLY_PACK_ATTR RefcountWithFlags { } private: + /** + * Helper function to modify refCount_ atomically. + * + * If predicate(currentValue) is true, then it atomically assigns result + * of newValueF(currentValue) to refCount_ and returns true. Otherwise + * returns false and leaves refCount_ unmodified. + */ + template + bool atomicUpdateValue(P&& predicate, F&& newValueF) { + Value* const refPtr = &refCount_; + unsigned int nCASFailures = 0; + constexpr bool isWeak = false; + Value curValue = __atomic_load_n(refPtr, __ATOMIC_RELAXED); + while (true) { + if (!predicate(curValue)) { + return false; + } + + const Value newValue = newValueF(curValue); + if (__atomic_compare_exchange_n(refPtr, &curValue, newValue, isWeak, + __ATOMIC_ACQ_REL, __ATOMIC_RELAXED)) { + return true; + } + + if ((++nCASFailures % 4) == 0) { + // this pause takes up to 40 clock cycles on intel and the lock cmpxchgl + // above should take about 100 clock cycles. we pause once every 400 + // cycles or so if we are extremely unlucky. + folly::asm_volatile_pause(); + } + } + } + template static Value getFlag() noexcept { static_assert(flagBit >= kNumAccessRefBits + kNumAdminRefBits, diff --git a/cachelib/allocator/tests/ItemTest.cpp b/cachelib/allocator/tests/ItemTest.cpp index b0f3a2fde..70dd1277f 100644 --- a/cachelib/allocator/tests/ItemTest.cpp +++ b/cachelib/allocator/tests/ItemTest.cpp @@ -83,10 +83,20 @@ TEST(ItemTest, ExpiryTime) { EXPECT_EQ(tenMins, item->getConfiguredTTL()); // Test that writes fail while the item is moving - item->markExclusive(); + result = item->markMoving(); + EXPECT_TRUE(result); + result = item->updateExpiryTime(0); + EXPECT_FALSE(result); + item->unmarkMoving(); + + // Test that writes fail while the item is marked for eviction + item->markAccessible(); + result = item->markForEviction(); + EXPECT_TRUE(result); result = item->updateExpiryTime(0); EXPECT_FALSE(result); - item->unmarkExclusive(); + item->unmarkForEviction(); + item->unmarkAccessible(); // Test that writes fail while the item is not in an MMContainer item->unmarkInMMContainer(); diff --git a/cachelib/allocator/tests/RefCountTest.cpp b/cachelib/allocator/tests/RefCountTest.cpp index b355a48a8..1f31894dd 100644 --- a/cachelib/allocator/tests/RefCountTest.cpp +++ b/cachelib/allocator/tests/RefCountTest.cpp @@ -30,6 +30,7 @@ class RefCountTest : public AllocTestBase { public: static void testMultiThreaded(); static void testBasic(); + static void testMarkForEvictionAndMoving(); }; void RefCountTest::testMultiThreaded() { @@ -81,7 +82,7 @@ void RefCountTest::testBasic() { ASSERT_EQ(0, ref.getRaw()); ASSERT_FALSE(ref.isInMMContainer()); ASSERT_FALSE(ref.isAccessible()); - ASSERT_FALSE(ref.isExclusive()); + ASSERT_FALSE(ref.isMoving()); ASSERT_FALSE(ref.template isFlagSet()); ASSERT_FALSE(ref.template isFlagSet()); @@ -89,7 +90,7 @@ void RefCountTest::testBasic() { ref.markInMMContainer(); ASSERT_TRUE(ref.isInMMContainer()); ASSERT_FALSE(ref.isAccessible()); - ASSERT_FALSE(ref.isExclusive()); + ASSERT_FALSE(ref.isMoving()); ASSERT_EQ(0, ref.getAccessRef()); ASSERT_FALSE(ref.template isFlagSet()); ASSERT_FALSE(ref.template isFlagSet()); @@ -105,13 +106,13 @@ void RefCountTest::testBasic() { // Incrementing past the max will fail auto rawRef = ref.getRaw(); - ASSERT_FALSE(ref.incRef()); + ASSERT_THROW(ref.incRef(), std::overflow_error); ASSERT_EQ(rawRef, ref.getRaw()); // Bumping up access ref shouldn't affect admin ref and flags ASSERT_TRUE(ref.isInMMContainer()); ASSERT_FALSE(ref.isAccessible()); - ASSERT_FALSE(ref.isExclusive()); + ASSERT_FALSE(ref.isMoving()); ASSERT_EQ(RefcountWithFlags::kAccessRefMask, ref.getAccessRef()); ASSERT_TRUE(ref.template isFlagSet()); ASSERT_FALSE(ref.template isFlagSet()); @@ -128,7 +129,7 @@ void RefCountTest::testBasic() { // Bumping down access ref shouldn't affect admin ref and flags ASSERT_TRUE(ref.isInMMContainer()); ASSERT_FALSE(ref.isAccessible()); - ASSERT_FALSE(ref.isExclusive()); + ASSERT_FALSE(ref.isMoving()); ASSERT_EQ(0, ref.getAccessRef()); ASSERT_TRUE(ref.template isFlagSet()); ASSERT_FALSE(ref.template isFlagSet()); @@ -136,7 +137,7 @@ void RefCountTest::testBasic() { ref.template unSetFlag(); ASSERT_TRUE(ref.isInMMContainer()); ASSERT_FALSE(ref.isAccessible()); - ASSERT_FALSE(ref.isExclusive()); + ASSERT_FALSE(ref.isMoving()); ASSERT_EQ(0, ref.getAccessRef()); ASSERT_FALSE(ref.template isFlagSet()); ASSERT_FALSE(ref.template isFlagSet()); @@ -145,33 +146,104 @@ void RefCountTest::testBasic() { ASSERT_EQ(0, ref.getRaw()); ASSERT_FALSE(ref.isInMMContainer()); ASSERT_FALSE(ref.isAccessible()); - ASSERT_FALSE(ref.isExclusive()); + ASSERT_FALSE(ref.isMoving()); ASSERT_EQ(0, ref.getAccessRef()); ASSERT_FALSE(ref.template isFlagSet()); ASSERT_FALSE(ref.template isFlagSet()); // conditionally set flags - ASSERT_FALSE((ref.markExclusive())); + ASSERT_FALSE((ref.markMoving())); ref.markInMMContainer(); - ASSERT_TRUE((ref.markExclusive())); - ASSERT_FALSE((ref.isOnlyExclusive())); + // only first one succeeds + ASSERT_TRUE((ref.markMoving())); + ASSERT_FALSE((ref.markMoving())); ref.unmarkInMMContainer(); + ref.template setFlag(); - // Have no other admin refcount but with a flag still means "isOnlyExclusive" - ASSERT_TRUE((ref.isOnlyExclusive())); + // Have no other admin refcount but with a flag still means "isOnlyMoving" + ASSERT_TRUE((ref.isOnlyMoving())); - // Set some flags and verify that "isOnlyExclusive" does not care about flags + // Set some flags and verify that "isOnlyMoving" does not care about flags ref.markIsChainedItem(); ASSERT_TRUE(ref.isChainedItem()); - ASSERT_TRUE((ref.isOnlyExclusive())); + ASSERT_TRUE((ref.isOnlyMoving())); ref.unmarkIsChainedItem(); ASSERT_FALSE(ref.isChainedItem()); - ASSERT_TRUE((ref.isOnlyExclusive())); + ASSERT_TRUE((ref.isOnlyMoving())); +} + +void RefCountTest::testMarkForEvictionAndMoving() { + { + // cannot mark for eviction when not in MMContainer + RefcountWithFlags ref; + ASSERT_FALSE(ref.markForEviction()); + } + + { + // can mark for eviction when in MMContainer + // and unmarkForEviction return value contains admin bits + RefcountWithFlags ref; + ref.markInMMContainer(); + ASSERT_TRUE(ref.markForEviction()); + ASSERT_TRUE(ref.unmarkForEviction() > 0); + } + + { + // cannot mark for eviction when moving + RefcountWithFlags ref; + ref.markInMMContainer(); + + ASSERT_TRUE(ref.markMoving()); + ASSERT_FALSE(ref.markForEviction()); + + ref.unmarkInMMContainer(); + auto ret = ref.unmarkMoving(); + ASSERT_EQ(ret, 0); + } + + { + // cannot mark moving when marked for eviction + RefcountWithFlags ref; + ref.markInMMContainer(); + + ASSERT_TRUE(ref.markForEviction()); + ASSERT_FALSE(ref.markMoving()); + + ref.unmarkInMMContainer(); + auto ret = ref.unmarkForEviction(); + ASSERT_EQ(ret, 0); + } + + { + // can mark moving when ref count > 0 + RefcountWithFlags ref; + ref.markInMMContainer(); + + ref.incRef(); + + ASSERT_TRUE(ref.markMoving()); + + ref.unmarkInMMContainer(); + auto ret = ref.unmarkMoving(); + ASSERT_EQ(ret, 1); + } + + { + // cannot mark for eviction when ref count > 0 + RefcountWithFlags ref; + ref.markInMMContainer(); + + ref.incRef(); + ASSERT_FALSE(ref.markForEviction()); + } } } // namespace TEST_F(RefCountTest, MutliThreaded) { testMultiThreaded(); } TEST_F(RefCountTest, Basic) { testBasic(); } +TEST_F(RefCountTest, MarkForEvictionAndMoving) { + testMarkForEvictionAndMoving(); +} } // namespace tests } // namespace cachelib } // namespace facebook From 0bdb93505ef6cb86ea0950a3889ee2dacf066cb0 Mon Sep 17 00:00:00 2001 From: generatedunixname226714639793621 Date: Thu, 9 Mar 2023 08:10:36 -0800 Subject: [PATCH 40/44] fbcode/cachelib/allocator/serialize Differential Revision: D43869057 fbshipit-source-id: 84aa3aec8d75f04b640d6aa8393002190c97842f --- cachelib/allocator/serialize/objects.thrift | 138 ++++++++++---------- 1 file changed, 69 insertions(+), 69 deletions(-) diff --git a/cachelib/allocator/serialize/objects.thrift b/cachelib/allocator/serialize/objects.thrift index 8d30ee8d8..61297cdf1 100644 --- a/cachelib/allocator/serialize/objects.thrift +++ b/cachelib/allocator/serialize/objects.thrift @@ -23,136 +23,136 @@ include "cachelib/allocator/datastruct/serialize/objects.thrift" // make sure to communicate that with our users. struct CacheAllocatorMetadata { - 1: required i64 allocatorVersion, // version of cache alloctor - 2: i64 cacheCreationTime = 0, // time when the cache was created. - 3: required i64 accessType = 0, // default chained alloc - 4: required i64 mmType = 0, // default LRU - 5: map> fragmentationSize, - 6: list compactCachePools, - 7: i64 numPermanentItems, - 8: i64 numChainedParentItems, - 9: i64 numChainedChildItems, - 10: i64 ramFormatVersion = 0, // format version of ram cache - 11: i64 numAbortedSlabReleases = 0, // number of times slab release is aborted + 1: required i64 allocatorVersion; // version of cache alloctor + 2: i64 cacheCreationTime = 0; // time when the cache was created. + 3: required i64 accessType = 0; // default chained alloc + 4: required i64 mmType = 0; // default LRU + 5: map> fragmentationSize; + 6: list compactCachePools; + 7: i64 numPermanentItems; + 8: i64 numChainedParentItems; + 9: i64 numChainedChildItems; + 10: i64 ramFormatVersion = 0; // format version of ram cache + 11: i64 numAbortedSlabReleases = 0; // number of times slab release is aborted } struct NvmCacheMetadata { - 1: i64 nvmFormatVersion = 0, - 2: i64 creationTime = 0, - 3: bool safeShutDown = false, - 4: bool encryptionEnabled = false, - 5: bool truncateAllocSize = false, + 1: i64 nvmFormatVersion = 0; + 2: i64 creationTime = 0; + 3: bool safeShutDown = false; + 4: bool encryptionEnabled = false; + 5: bool truncateAllocSize = false; } struct CompactCacheMetadataObject { - 1: required i64 keySize, - 2: required i64 valueSize, + 1: required i64 keySize; + 2: required i64 valueSize; } struct CompactCacheAllocatorObject { - 1: required list chunks, - 2: required CompactCacheMetadataObject ccMetadata, + 1: required list chunks; + 2: required CompactCacheMetadataObject ccMetadata; } struct CompactCacheAllocatorManagerObject { - 1: required map allocators, + 1: required map allocators; } struct MMLruConfig { - 1: required i32 lruRefreshTime, - 2: required bool updateOnWrite, - 3: required i32 lruInsertionPointSpec, - 4: bool updateOnRead = true, - 5: bool tryLockUpdate = false, - 6: double lruRefreshRatio = 0.0, + 1: required i32 lruRefreshTime; + 2: required bool updateOnWrite; + 3: required i32 lruInsertionPointSpec; + 4: bool updateOnRead = true; + 5: bool tryLockUpdate = false; + 6: double lruRefreshRatio = 0.0; } struct MMLruObject { - 1: required MMLruConfig config, + 1: required MMLruConfig config; // number of evictions for this MM object. - 5: i64 evictions = 0, + 5: i64 evictions = 0; - 6: required i64 insertionPoint, - 7: required i64 tailSize, - 8: required DListObject lru, - 9: required i64 compressedInsertionPoint, + 6: required i64 insertionPoint; + 7: required i64 tailSize; + 8: required DListObject lru; + 9: required i64 compressedInsertionPoint; } struct MMLruCollection { - 1: required map> pools, + 1: required map> pools; } struct MM2QConfig { - 1: required i32 lruRefreshTime, - 2: required bool updateOnWrite, - 3: required i32 hotSizePercent, - 4: required i32 coldSizePercent, - 5: bool updateOnRead = true, - 6: bool tryLockUpdate = false, - 7: bool rebalanceOnRecordAccess = true, - 8: double lruRefreshRatio = 0.0, + 1: required i32 lruRefreshTime; + 2: required bool updateOnWrite; + 3: required i32 hotSizePercent; + 4: required i32 coldSizePercent; + 5: bool updateOnRead = true; + 6: bool tryLockUpdate = false; + 7: bool rebalanceOnRecordAccess = true; + 8: double lruRefreshRatio = 0.0; } struct MM2QObject { - 1: required MM2QConfig config, - 13: bool tailTrackingEnabled = false, + 1: required MM2QConfig config; + 13: bool tailTrackingEnabled = false; // number of evictions for this MM object. - 11: i64 evictions = 0, + 11: i64 evictions = 0; // Warm, hot and cold lrus - 12: required MultiDListObject lrus, + 12: required MultiDListObject lrus; } struct MM2QCollection { - 1: required map> pools, + 1: required map> pools; } struct MMTinyLFUConfig { - 1: required i32 lruRefreshTime, - 2: required bool updateOnWrite, - 3: required i32 windowToCacheSizeRatio, - 4: required i32 tinySizePercent, - 5: bool updateOnRead = true, - 6: bool tryLockUpdate = false, - 7: double lruRefreshRatio = 0.0, + 1: required i32 lruRefreshTime; + 2: required bool updateOnWrite; + 3: required i32 windowToCacheSizeRatio; + 4: required i32 tinySizePercent; + 5: bool updateOnRead = true; + 6: bool tryLockUpdate = false; + 7: double lruRefreshRatio = 0.0; } struct MMTinyLFUObject { - 1: required MMTinyLFUConfig config, + 1: required MMTinyLFUConfig config; // number of evictions for this MM object. - 2: i64 evictions = 0, + 2: i64 evictions = 0; // Warm, hot and cold lrus - 3: required MultiDListObject lrus, + 3: required MultiDListObject lrus; } struct MMTinyLFUCollection { - 1: required map> pools, + 1: required map> pools; } struct ChainedHashTableObject { // fields in ChainedHashTable::Config - 1: required i32 bucketsPower, - 2: required i32 locksPower, - 3: i64 numKeys, + 1: required i32 bucketsPower; + 2: required i32 locksPower; + 3: i64 numKeys; // this magic id ensures on a warm roll, user cannot // start the cache with a different hash function - 4: i32 hasherMagicId = 0, + 4: i32 hasherMagicId = 0; } struct MMTTLBucketObject { - 4: i64 expirationTime, - 5: i64 creationTime, - 6: required DListObject dList, + 4: i64 expirationTime; + 5: i64 creationTime; + 6: required DListObject dList; } struct TTLBucketCollection { - 1: required map buckets, - 2: i64 minEpoch = 0, - 3: i64 maxTTL = 0, - 4: i64 interval = 0, + 1: required map buckets; + 2: i64 minEpoch = 0; + 3: i64 maxTTL = 0; + 4: i64 interval = 0; } From 99a05d35a3ed9a782262160e9e2119b90a41f86e Mon Sep 17 00:00:00 2001 From: generatedunixname226714639793621 Date: Thu, 9 Mar 2023 08:16:02 -0800 Subject: [PATCH 41/44] fbcode/cachelib/experimental/objcache Differential Revision: D43869089 fbshipit-source-id: 57d359fa8cfeaca395f636f6d3997a934afca94a --- .../objcache/ObjectCachePersistence.thrift | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/cachelib/experimental/objcache/ObjectCachePersistence.thrift b/cachelib/experimental/objcache/ObjectCachePersistence.thrift index fcddb82ef..e5acba4d5 100644 --- a/cachelib/experimental/objcache/ObjectCachePersistence.thrift +++ b/cachelib/experimental/objcache/ObjectCachePersistence.thrift @@ -17,9 +17,9 @@ namespace cpp2 facebook.cachelib.objcache.serialization struct Item { - 1: byte poolId - 2: i32 creationTime, - 3: i32 expiryTime, - 4: string key, - 5: string payload, + 1: byte poolId; + 2: i32 creationTime; + 3: i32 expiryTime; + 4: string key; + 5: string payload; } From d1ff29c61115f5eebc20984b22a9e67a10adc488 Mon Sep 17 00:00:00 2001 From: generatedunixname226714639793621 Date: Fri, 10 Mar 2023 08:09:44 -0800 Subject: [PATCH 42/44] fbcode/cachelib/ Differential Revision: D43904241 fbshipit-source-id: 3d7484bee3a62495b2f2ee299fd3b0b855a3be58 --- cachelib/shm/shm.thrift | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cachelib/shm/shm.thrift b/cachelib/shm/shm.thrift index 0372d7c8f..702294722 100644 --- a/cachelib/shm/shm.thrift +++ b/cachelib/shm/shm.thrift @@ -17,6 +17,6 @@ namespace cpp2 facebook.cachelib.serialization struct ShmManagerObject { - 1: required byte shmVal, - 3: required map nameToKeyMap, + 1: required byte shmVal; + 3: required map nameToKeyMap; } From 3a8d8b4ad3cb5f6221b19f971b15092bcb0617e8 Mon Sep 17 00:00:00 2001 From: Jiayue Bao Date: Mon, 13 Mar 2023 09:25:25 -0700 Subject: [PATCH 43/44] Add extra notes for object-cache find APIs Summary: Putting extra note of separating read/write traffic when using find APIs. Reviewed By: jaesoo-fb Differential Revision: D43960690 fbshipit-source-id: 56d3a8b65c7b66b3319bee6e7ffdd959826c8dcf --- website/docs/Cache_Library_User_Guides/eviction_policy.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/website/docs/Cache_Library_User_Guides/eviction_policy.md b/website/docs/Cache_Library_User_Guides/eviction_policy.md index 356a3cb19..cce947619 100644 --- a/website/docs/Cache_Library_User_Guides/eviction_policy.md +++ b/website/docs/Cache_Library_User_Guides/eviction_policy.md @@ -23,7 +23,7 @@ The second modification is promotion delay. Normally every access item is moved How often does cachelib refresh a previously accessed item. By default this is 60 seconds. * `updateOnWrite`/`updateOnRead` -Specifies if a LRU promotion happens on read or write or both. As a rule of thumb, for most services that care primarily about read performance, turn on `updateOnRead`. However, if your service cares a lot about retention time of items that are recently written, then turn on `updateOnWrite` as well. +Specifies if a LRU promotion happens on read or write or both. As a rule of thumb, for most services that care primarily about read performance, turn on `updateOnRead`. However, if your service cares a lot about retention time of items that are recently written, then turn on `updateOnWrite` as well. By default, `updateOnRead = true` and `updateOnWrite = false`. * `ipSpec` This essentially turns the LRU into a two-segmented LRU. Setting this to `1` means every new insertion will be inserted 1/2 from the end of the LRU, `2` means 1/4 from the end of the LRU, and so on. From 47e5632d923ba2f8837e45b513041f4004f12e1d Mon Sep 17 00:00:00 2001 From: Igor Chorazewicz Date: Wed, 6 Jul 2022 10:15:17 +0000 Subject: [PATCH 44/44] Add memory usage statistics for allocation classes This includes printing: - allocSize - allocated memory size - memory usage fraction --- cachelib/allocator/Cache.h | 6 ++ cachelib/allocator/CacheAllocator-inl.h | 8 +++ cachelib/allocator/CacheAllocator.h | 3 + cachelib/allocator/memory/AllocationClass.h | 8 --- .../allocator/memory/MemoryAllocatorStats.h | 11 ++++ cachelib/allocator/tests/CacheBaseTest.cpp | 1 + cachelib/cachebench/cache/Cache-inl.h | 9 +++ cachelib/cachebench/cache/Cache.cpp | 6 ++ cachelib/cachebench/cache/Cache.h | 5 ++ cachelib/cachebench/cache/CacheStats.h | 58 +++++++++++++++++++ 10 files changed, 107 insertions(+), 8 deletions(-) diff --git a/cachelib/allocator/Cache.h b/cachelib/allocator/Cache.h index e225ba8a0..082db65f7 100644 --- a/cachelib/allocator/Cache.h +++ b/cachelib/allocator/Cache.h @@ -102,6 +102,12 @@ class CacheBase { // @param poolId the pool id virtual PoolStats getPoolStats(PoolId poolId) const = 0; + // Get Allocation Class specific stats. + // + // @param poolId the pool id + // @param classId the class id + virtual ACStats getACStats(PoolId poolId, ClassId classId) const = 0; + // @param poolId the pool id virtual AllSlabReleaseEvents getAllSlabReleaseEvents(PoolId poolId) const = 0; diff --git a/cachelib/allocator/CacheAllocator-inl.h b/cachelib/allocator/CacheAllocator-inl.h index b8d0256d5..491e9100c 100644 --- a/cachelib/allocator/CacheAllocator-inl.h +++ b/cachelib/allocator/CacheAllocator-inl.h @@ -2237,6 +2237,14 @@ PoolStats CacheAllocator::getPoolStats(PoolId poolId) const { return ret; } +template +ACStats CacheAllocator::getACStats(PoolId poolId, + ClassId classId) const { + const auto& pool = allocator_->getPool(poolId); + const auto& ac = pool.getAllocationClass(classId); + return ac.getStats(); +} + template PoolEvictionAgeStats CacheAllocator::getPoolEvictionAgeStats( PoolId pid, unsigned int slabProjectionLength) const { diff --git a/cachelib/allocator/CacheAllocator.h b/cachelib/allocator/CacheAllocator.h index 32d44c138..3649f30f2 100644 --- a/cachelib/allocator/CacheAllocator.h +++ b/cachelib/allocator/CacheAllocator.h @@ -1175,6 +1175,9 @@ class CacheAllocator : public CacheBase { // return cache's memory usage stats CacheMemoryStats getCacheMemoryStats() const override final; + // return stats for Allocation Class + ACStats getACStats(PoolId pid, ClassId cid) const override final; + // return the nvm cache stats map util::StatsMap getNvmCacheStatsMap() const override final; diff --git a/cachelib/allocator/memory/AllocationClass.h b/cachelib/allocator/memory/AllocationClass.h index b602e4d66..d45a45c6c 100644 --- a/cachelib/allocator/memory/AllocationClass.h +++ b/cachelib/allocator/memory/AllocationClass.h @@ -94,14 +94,6 @@ class AllocationClass { return static_cast(Slab::kSize / allocationSize_); } - // total number of slabs under this AllocationClass. - unsigned int getNumSlabs() const { - return lock_->lock_combine([this]() { - return static_cast(freeSlabs_.size() + - allocatedSlabs_.size()); - }); - } - // fetch stats about this allocation class. ACStats getStats() const; diff --git a/cachelib/allocator/memory/MemoryAllocatorStats.h b/cachelib/allocator/memory/MemoryAllocatorStats.h index 74ebbe64d..65d82e000 100644 --- a/cachelib/allocator/memory/MemoryAllocatorStats.h +++ b/cachelib/allocator/memory/MemoryAllocatorStats.h @@ -54,6 +54,17 @@ struct ACStats { constexpr size_t getTotalFreeMemory() const noexcept { return Slab::kSize * freeSlabs + freeAllocs * allocSize; } + + constexpr double usageFraction() const noexcept { + if (usedSlabs == 0) + return 0.0; + + return activeAllocs / (usedSlabs * allocsPerSlab); + } + + constexpr size_t totalAllocatedSize() const noexcept { + return activeAllocs * allocSize; + } }; // structure to query stats corresponding to a MemoryPool diff --git a/cachelib/allocator/tests/CacheBaseTest.cpp b/cachelib/allocator/tests/CacheBaseTest.cpp index 928fcc0c6..f24978674 100644 --- a/cachelib/allocator/tests/CacheBaseTest.cpp +++ b/cachelib/allocator/tests/CacheBaseTest.cpp @@ -34,6 +34,7 @@ class CacheBaseTest : public CacheBase, public SlabAllocatorTestBase { bool isObjectCache() const override { return false; } const MemoryPool& getPool(PoolId) const override { return memoryPool_; } PoolStats getPoolStats(PoolId) const override { return PoolStats(); } + ACStats getACStats(PoolId, ClassId) const { return ACStats(); }; AllSlabReleaseEvents getAllSlabReleaseEvents(PoolId) const override { return AllSlabReleaseEvents{}; } diff --git a/cachelib/cachebench/cache/Cache-inl.h b/cachelib/cachebench/cache/Cache-inl.h index dc5e878fc..eba901512 100644 --- a/cachelib/cachebench/cache/Cache-inl.h +++ b/cachelib/cachebench/cache/Cache-inl.h @@ -620,10 +620,19 @@ Stats Cache::getStats() const { aggregate += poolStats; } + std::map> allocationClassStats{}; + + for (size_t pid = 0; pid < pools_.size(); pid++) { + auto cids = cache_->getPoolStats(static_cast(pid)).getClassIds(); + for (auto cid : cids) + allocationClassStats[pid][cid] = cache_->getACStats(pid, cid); + } + const auto cacheStats = cache_->getGlobalCacheStats(); const auto rebalanceStats = cache_->getSlabReleaseStats(); const auto navyStats = cache_->getNvmCacheStatsMap().toMap(); + ret.allocationClassStats = allocationClassStats; ret.numEvictions = aggregate.numEvictions(); ret.numItems = aggregate.numItems(); ret.evictAttempts = cacheStats.evictionAttempts; diff --git a/cachelib/cachebench/cache/Cache.cpp b/cachelib/cachebench/cache/Cache.cpp index 70feb0f76..ea9d6b106 100644 --- a/cachelib/cachebench/cache/Cache.cpp +++ b/cachelib/cachebench/cache/Cache.cpp @@ -20,6 +20,12 @@ DEFINE_bool(report_api_latency, false, "Enable reporting cache API latency tracking"); +DEFINE_string( + report_ac_memory_usage_stats, + "", + "Enable reporting statistics for each allocation class. Set to" + "'human_readable' to print KB/MB/GB or to 'raw' to print in bytes."); + namespace facebook { namespace cachelib { namespace cachebench {} // namespace cachebench diff --git a/cachelib/cachebench/cache/Cache.h b/cachelib/cachebench/cache/Cache.h index a86dd06c4..65c70c30c 100644 --- a/cachelib/cachebench/cache/Cache.h +++ b/cachelib/cachebench/cache/Cache.h @@ -44,6 +44,7 @@ #include "cachelib/cachebench/util/NandWrites.h" DECLARE_bool(report_api_latency); +DECLARE_string(report_ac_memory_usage_stats); namespace facebook { namespace cachelib { @@ -324,6 +325,10 @@ class Cache { // return the stats for the pool. PoolStats getPoolStats(PoolId pid) const { return cache_->getPoolStats(pid); } + ACStats getACStats(PoolId pid, ClassId cid) const { + return cache_->getACStats(pid, cid); + } + // return the total number of inconsistent operations detected since start. unsigned int getInconsistencyCount() const { return inconsistencyCount_.load(std::memory_order_relaxed); diff --git a/cachelib/cachebench/cache/CacheStats.h b/cachelib/cachebench/cache/CacheStats.h index d6c9e5358..38c48aa4c 100644 --- a/cachelib/cachebench/cache/CacheStats.h +++ b/cachelib/cachebench/cache/CacheStats.h @@ -21,6 +21,7 @@ #include "cachelib/common/PercentileStats.h" DECLARE_bool(report_api_latency); +DECLARE_string(report_ac_memory_usage_stats); namespace facebook { namespace cachelib { @@ -100,6 +101,8 @@ struct Stats { uint64_t invalidDestructorCount{0}; int64_t unDestructedItemCount{0}; + std::map> allocationClassStats; + // populate the counters related to nvm usage. Cache implementation can decide // what to populate since not all of those are interesting when running // cachebench. @@ -131,6 +134,61 @@ struct Stats { << std::endl; } + if (FLAGS_report_ac_memory_usage_stats != "") { + auto formatMemory = [&](size_t bytes) -> std::tuple { + if (FLAGS_report_ac_memory_usage_stats == "raw") { + return {"B", bytes}; + } + + constexpr double KB = 1024.0; + constexpr double MB = 1024.0 * 1024; + constexpr double GB = 1024.0 * 1024 * 1024; + + if (bytes >= GB) { + return {"GB", static_cast(bytes) / GB}; + } else if (bytes >= MB) { + return {"MB", static_cast(bytes) / MB}; + } else if (bytes >= KB) { + return {"KB", static_cast(bytes) / KB}; + } else { + return {"B", bytes}; + } + }; + + auto foreachAC = [&](auto cb) { + for (auto& pidStat : allocationClassStats) { + for (auto& cidStat : pidStat.second) { + cb(pidStat.first, cidStat.first, cidStat.second); + } + } + }; + + foreachAC([&](auto pid, auto cid, auto stats) { + auto [allocSizeSuffix, allocSize] = formatMemory(stats.allocSize); + auto [memorySizeSuffix, memorySize] = + formatMemory(stats.totalAllocatedSize()); + out << folly::sformat("pid{:2} cid{:4} {:8.2f}{} memorySize: {:8.2f}{}", + pid, cid, allocSize, allocSizeSuffix, memorySize, + memorySizeSuffix) + << std::endl; + }); + + foreachAC([&](auto pid, auto cid, auto stats) { + auto [allocSizeSuffix, allocSize] = formatMemory(stats.allocSize); + + // If the pool is not full, extrapolate usageFraction for AC assuming it + // will grow at the same rate. This value will be the same for all ACs. + auto acUsageFraction = (poolUsageFraction[pid] < 1.0) + ? poolUsageFraction[pid] + : stats.usageFraction(); + + out << folly::sformat( + "pid{:2} cid{:4} {:8.2f}{} usageFraction: {:4.2f}", pid, cid, + allocSize, allocSizeSuffix, acUsageFraction) + << std::endl; + }); + } + if (numCacheGets > 0) { out << folly::sformat("Cache Gets : {:,}", numCacheGets) << std::endl; out << folly::sformat("Hit Ratio : {:6.2f}%", overallHitRatio)