From 9983eecdfbaf8e16f6ac2e22d1a16a5b37d8ac4d Mon Sep 17 00:00:00 2001 From: Andrew Kryczka Date: Sun, 27 Feb 2022 11:36:54 -0800 Subject: [PATCH] Dedicate cacheline for DB mutex (#9637) Summary: We found a case of cacheline bouncing due to writers locking/unlocking `mutex_` and readers accessing `block_cache_tracer_`. We discovered it only after the issue was fixed by https://github.com/facebook/rocksdb/issues/9462 shifting the `DBImpl` members such that `mutex_` and `block_cache_tracer_` were naturally placed in separate cachelines in our regression testing setup. This PR forces the cacheline alignment of `mutex_` so we don't accidentally reintroduce the problem. Pull Request resolved: https://github.com/facebook/rocksdb/pull/9637 Reviewed By: riversand963 Differential Revision: D34502233 Pulled By: ajkr fbshipit-source-id: 46aa313b7fe83e80c3de254e332b6fb242434c07 --- db/db_impl/db_impl.h | 5 ++++- monitoring/instrumented_mutex.h | 8 ++++++++ 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/db/db_impl/db_impl.h b/db/db_impl/db_impl.h index 9c07e81fc..c37a46c3e 100644 --- a/db/db_impl/db_impl.h +++ b/db/db_impl/db_impl.h @@ -1187,7 +1187,10 @@ class DBImpl : public DB { // WriteToWAL need different synchronization: log_empty_, alive_log_files_, // logs_, logfile_number_. Refer to the definition of each variable below for // more description. - mutable InstrumentedMutex mutex_; + // + // `mutex_` can be a hot lock in some workloads, so it deserves dedicated + // cachelines. + mutable CacheAlignedInstrumentedMutex mutex_; ColumnFamilyHandleImpl* default_cf_handle_; InternalStats* default_cf_internal_stats_; diff --git a/monitoring/instrumented_mutex.h b/monitoring/instrumented_mutex.h index 1e72815bf..e05c6addd 100644 --- a/monitoring/instrumented_mutex.h +++ b/monitoring/instrumented_mutex.h @@ -51,6 +51,14 @@ class InstrumentedMutex { int stats_code_; }; +class ALIGN_AS(CACHE_LINE_SIZE) CacheAlignedInstrumentedMutex + : public InstrumentedMutex { + using InstrumentedMutex::InstrumentedMutex; + char padding[(CACHE_LINE_SIZE - sizeof(InstrumentedMutex) % CACHE_LINE_SIZE) % + CACHE_LINE_SIZE] ROCKSDB_FIELD_UNUSED; +}; +static_assert(sizeof(CacheAlignedInstrumentedMutex) % CACHE_LINE_SIZE == 0); + // RAII wrapper for InstrumentedMutex class InstrumentedMutexLock { public: