Conversation
Assigned allocation slots can remain after all accessible primary items have been removed. Use the existing primary-item count to skip the current reaper slab walk when that count is zero, without adding state or writer work. Periodic wakeups and nonempty traversal are unchanged. A concurrent insertion may be handled on a later pass, consistent with the existing weak traversal semantics; lookup expiration and synchronized ownership checks are unchanged. Add registered tests for empty allocations, reactivation, pinned expired parents with chained items, live retention and concurrent slab traversal.
meta-cla
Bot
added
the
CLA Signed
|
@rlyerly has imported this pull request. If you are a Meta employee, you can view this in D119975083. |
rlyerly
left a comment
rlyerly
left a comment
There was a problem hiding this comment.
Few smaller cleanups & questions. Thanks!
| ASSERT_TRUE(reaper.start(std::chrono::milliseconds(10), "reaper-test")); | ||
| ASSERT_TRUE(waitFor([&] { return reaper.getStats().numTraversals > 0; })); | ||
| EXPECT_EQ(reaper.getStats().numVisitedItems, 0); | ||
| for (size_t i = 0; i < 20; ++i) { |
There was a problem hiding this comment.
What's the point of doing this 20 times? Seems like 2 times is enough to guarantee we go between skipping & traversing
| template <typename Predicate> | ||
| bool waitFor(Predicate predicate) { | ||
| const auto deadline = | ||
| std::chrono::steady_clock::now() + std::chrono::seconds(5); | ||
| while (!predicate()) { | ||
| if (std::chrono::steady_clock::now() >= deadline) { | ||
| return false; | ||
| } | ||
| std::this_thread::sleep_for(std::chrono::milliseconds(1)); | ||
| } | ||
| return true; | ||
| } |
There was a problem hiding this comment.
Please use eventuallyTrue or ASSERT_EVENTUALLY_TRUE instead.
| EXPECT_EQ(cache.getAccessContainerNumKeys(), 0); | ||
| } | ||
|
|
||
| TEST(ReaperTest, NonemptyCacheStillSkipsSlabDuringConcurrentTraversal) { |
There was a problem hiding this comment.
Not exactly sure what this test is trying to accomplish, can you give me some details?
There was a problem hiding this comment.
Much of the finer detail of this is lost on me - the system I am developing is guided by me, but the output is often machine-only - my intention is to test on many different codebases as I develop it - I've had good feedback thus far, but I'm unable to elaborate much - if the amendments are of no use, that's feedback too, and I will take on board. This particular codebase produced almost nothing in the way of potential enhancements beyond this suggestion.
There was a problem hiding this comment.
To be frank, I expect this to have minimal impact on most deployments. This elides work when caches are empty, but almost no real production user of Cachelib has an empty cache. Closing for now, feel free to reopen if you want to drive this further.
2 participants
Summary
The reaper can continue scanning assigned allocation slots after all accessible primary items have been removed. Use the existing primary-item count to skip that periodic slab walk when the cache is empty. Nonempty traversal and periodic wakeups are unchanged.
Why
Assigned slabs can remain after the accessible population reaches zero, but walking those slots cannot find an accessible item to expire. The existing atomic count supplies the signal without another flag, lock, or per-item update.
Concurrency
The count observation can race with insertion/publication. A concurrently inserted item may be handled on a subsequent periodic pass, consistent with the existing weak slab traversal. Foreground expiration checks and synchronized removal/ownership behavior are unchanged.
Performance
In a local default-style idle workload (64 MiB configured cache, approximately 16 MiB assigned slots, 5-second reaper interval, 12-second window):
This saves about 5 ms CPU over the 12-second idle window. It measures idle/background reaper work, not foreground cache throughput; the process CPU measurement includes other process bookkeeping. Absolute savings are modest and depend on assigned slab population, reaper frequency and hardware. A concise confirmation on the final rebased source verified zero empty-cache visits, active nonempty traversal and reactivation.
Testing
python3 build/fbcode_builder/getdeps.py --allow-system-packages --scratch-path /root/wob3/getdeps --num-jobs 4 build cachelib --src-dir /root/wob3/CacheLib --build-type RelWithDebInfopython3 build/fbcode_builder/getdeps.py --allow-system-packages --scratch-path /root/wob3/getdeps --num-jobs 2 test cachelib --src-dir /root/wob3/CacheLib --build-type RelWithDebInfo --no-testpilot --timeout 300 --retry 0(private ext4 /tmp and DAC-bypass capabilities removed for filesystem/permission tests).clang-format-21 --dry-run --Werror cachelib/allocator/CacheAllocator.h cachelib/allocator/tests/ReaperTest.cppgit diff --check upstream/main...HEADScope
No cache or eviction policy changes, new persistent state, or NVM changes. No foreground hit/miss behavior change is intended. Numerical CPU results are workload and hardware dependent.
The opportunities were identified and validated using WOB/WOB3 techniques.
WOB is an experimental method of refocussing workloads in data-heavy code. I am stress testing various codebases, to develop the method further, and cachelib seemed like a worthwhile candidate. This single result was a small win, so I am sharing. Feedback would be greatly appreciated.
WOB is human-designed, human-led, and machine assisted.