Skip to content

Commit

Permalink
migrate fields of type folly::SharedMutex to be marked as mutable in …
Browse files Browse the repository at this point in the history
…fbcode (assorted)

Summary:
The nested-name lock-holder `folly::SharedMutex::ReadHolder` accepts `folly::SharedMutex const&`, i.e., as ref-to-const, and it internally does a `const_cast` to remove the `const`. This internal `const_cast` is necessary because members `folly::SharedMutex::lock_shared` and related member functions are not `const`-qualified.

This lock-holder interface is a convenience since a shared-lock on a shared-mutex instance are commonly acquired in `const`-qualified class member functions, where the shared-mutex instance is a class data member and access to it from a `const`-qualified member function implicitly has only `const`-access to the shared-mutex instance.

But `std::shared_lock` does not have this convenience. It is the canonical lock-holder for shared locks and we wish to migrate all uses of `folly::SharedMutex::ReadHolder` to `std::shared_lock`. So we must mark relevant shared-mutex instances with `mutable`.

As a small shortcut, we search for and mark all class data members of type `folly::SharedMutex` as `mutable`.

Reviewed By: luciang

Differential Revision: D52921205

fbshipit-source-id: 966782bbe1372f83b720ce9a1dddaf1642288964
  • Loading branch information
yfeldblum authored and facebook-github-bot committed Jan 24, 2024
1 parent 23a888e commit 767e458
Show file tree
Hide file tree
Showing 3 changed files with 6 additions and 6 deletions.
4 changes: 2 additions & 2 deletions cachelib/allocator/CacheAllocator.h
Original file line number Diff line number Diff line change
Expand Up @@ -2124,7 +2124,7 @@ class CacheAllocator : public CacheBase {
typename Item::PtrCompressor compressor_;

// Lock to synchronize addition of a new pool and its resizing/rebalancing
folly::SharedMutex poolsResizeAndRebalanceLock_;
mutable folly::SharedMutex poolsResizeAndRebalanceLock_;

// container for the allocations which are currently being memory managed by
// the cache allocator.
Expand Down Expand Up @@ -2167,7 +2167,7 @@ class CacheAllocator : public CacheBase {

// lock to serilize access of isCompactCachePool_ array, including creation of
// compact cache pools
folly::SharedMutex compactCachePoolsLock_;
mutable folly::SharedMutex compactCachePoolsLock_;

// mutex protecting the creation and destruction of workers poolRebalancer_,
// poolResizer_, poolOptimizer_, memMonitor_, reaper_
Expand Down
4 changes: 2 additions & 2 deletions cachelib/benchmarks/SpeedUpExistenceCheckBenchmark.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -90,14 +90,14 @@ struct BucketWithKey {
struct SharedMutex {
auto getReadLock() { return folly::SharedMutex::ReadHolder{l}; }
auto getWriteLock() { return std::unique_lock{l}; }
folly::SharedMutex l{};
mutable folly::SharedMutex l{};
};

struct alignas(folly::hardware_destructive_interference_size)
SharedMutexAligned {
auto getReadLock() { return folly::SharedMutex::ReadHolder{l}; }
auto getWriteLock() { return std::unique_lock{l}; }
folly::SharedMutex l{};
mutable folly::SharedMutex l{};
};

struct SpinLock {
Expand Down
4 changes: 2 additions & 2 deletions cachelib/compact_cache/CCache.h
Original file line number Diff line number Diff line change
Expand Up @@ -555,8 +555,8 @@ class CompactCache : public ICompactCache {
RemoveCb removeCb_;
ReplaceCb replaceCb_;
ValidCb validCb_;
facebook::cachelib::Cohort cohort_; /**< resize cohort synchronization */
folly::SharedMutex resizeLock_; /**< Lock to prevent resize conflicts. */
facebook::cachelib::Cohort cohort_; /**< resize cohort synchronization */
mutable folly::SharedMutex resizeLock_; /**< Lock to synchronize resize. */
const size_t bucketsPerChunk_;
util::FastStats<CCacheStats> stats_;
const bool allowPromotions_; /**< Whether promotions are allowed on read
Expand Down

0 comments on commit 767e458

Please sign in to comment.