Add reader/writer locks to cache stripe for reduced contention - #12601
Closed
bryancall wants to merge 1 commit into
Closed
Add reader/writer locks to cache stripe for reduced contention#12601bryancall wants to merge 1 commit into
bryancall wants to merge 1 commit into
Conversation
This commit implements reader/writer locks for cache directory operations to significantly reduce lock contention under high concurrency. Changes: - Added ts::shared_mutex dir_mutex to StripeSM for directory operations - Created CacheDirSharedLock and CacheDirExclusiveLock RAII wrappers - Converted critical Cache.cc read paths to use shared locks for directory.probe() - Multiple readers can now access directory concurrently Performance Impact: - Throughput: 17,520 req/s -> 44,218 req/s (+152%, 2.5x improvement) - Mean latency: 55.94ms -> 22.23ms (-60%, 2.5x faster) - Cache lock overhead: 42.81ms -> 9.10ms (-79%) Test configuration: 1M requests, 1K concurrent clients, non-cacheable origin This is a partial implementation covering Cache.cc read paths. Further optimization possible by converting CacheRead.cc and CacheWrite.cc. Files modified: - src/iocore/cache/StripeSM.h: Added dir_mutex member - src/iocore/cache/P_CacheInternal.h: Added lock wrapper classes - src/iocore/cache/Cache.cc: Converted 3 critical paths to shared locks Documentation: - CACHE_RWLOCK_ANALYSIS.md: Design analysis and implementation strategy - CACHE_RWLOCK_BENCHMARK_RESULTS.md: Detailed benchmark results and analysis
bryancall
force-pushed
the
cache-rwlock-optimization
branch
from
October 22, 2025 23:28
8b652df to
aa99c83
Compare
masaori335
reviewed
Oct 22, 2025
|
|
||
| #### Read-Only Operations (can use shared locks): | ||
| ``` | ||
| Cache.cc:345 - stripe->directory.probe() [cache lookup] |
Contributor
There was a problem hiding this comment.
I tried this approach before and faced a problem. If I understand correctly, Directory::probe needs the write lock when it found invalid Dir and call dir_delete_entry. This is why I'm waiting RCU/Hazard Pointer.
trafficserver/src/iocore/cache/CacheDir.cc
Lines 522 to 527 in 0411aa7
masaori335
reviewed
Oct 22, 2025
| if (!lock.is_locked() || (od = stripe->open_read(key)) || stripe->directory.probe(key, stripe, &result, &last_collision)) { | ||
| CACHE_DIR_TRY_LOCK_SHARED(dir_lock, stripe->dir_mutex); | ||
| if (!lock.is_locked() || !dir_lock.is_locked() || (od = stripe->open_read(key)) || | ||
| stripe->directory.probe(key, stripe, &result, &last_collision)) { |
Contributor
There was a problem hiding this comment.
Having mutex and dir_mutex might be good approach. When we call Directory::probe with read lock of dir_mutex, but it needs write operation (it's rare case I assume), we can wait for mutex lock. Obviously, we have to be careful for dead lock.
masaori335
reviewed
Oct 22, 2025
| - High-performance BRAVO algorithm implementation | ||
| - Optimized fast-path for readers (lock-free in common case) | ||
| - Prevents writer starvation with adaptive policy | ||
| - More complex but potentially much faster |
Contributor
There was a problem hiding this comment.
This lock contention problem of Stripe Mutex was main motivation of introducing BRAVO.
Contributor
Author
|
Closing this PR - the approach needs more analysis and won't make the 10.2.0 release. May revisit in a future release. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This commit implements reader/writer locks for cache directory operations to significantly reduce lock contention under high concurrency.
Changes:
Performance Impact:
Test configuration: 1M requests, 1K concurrent clients, non-cacheable origin
This is a partial implementation covering Cache.cc read paths. Further optimization possible by converting CacheRead.cc and CacheWrite.cc.
Files modified:
Documentation: