From 387cb4aae7fefbf0fa07ebb0f993936ceaae729c Mon Sep 17 00:00:00 2001 From: Peter Dillinger Date: Mon, 5 Jan 2026 20:47:46 -0800 Subject: [PATCH] Clarify/rename atomic wrapper stuff + blog post (#14213) Summary: * Some existing commentary and motivation around my atomic wrappers in atomic.h was based on a misreading of documentation. seq_cst *is* a safe substitute for acq_rel in all cases. I still like having a distinct type for RelaxedAtomic (as folly does) and a wrapper also for other cases to avoid readability traps like implicit conversion and implicit memory order. This PR is only comment changes and renaming. * Create a blog post about bit fields API to help with lock-free (and low-lock) programming. Pull Request resolved: https://github.com/facebook/rocksdb/pull/14213 Test Plan: esiting tests Reviewed By: xingbowang Differential Revision: D89971581 Pulled By: pdillinger fbshipit-source-id: 9bd1181d692258d668189c2da8bd0e5d98fd6230 --- cache/clock_cache.cc | 4 +- cache/clock_cache.h | 14 +- db/db_wal_test.cc | 2 +- db/db_write_test.cc | 6 +- .../_posts/2025-12-31-bit-fields-api.markdown | 279 ++++++++++++++++++ memtable/inlineskiplist.h | 4 +- memtable/skiplist.h | 6 +- .../block_based/block_based_table_builder.cc | 2 +- util/atomic.h | 53 ++-- util/bit_fields.h | 18 +- util/slice_test.cc | 2 +- 11 files changed, 339 insertions(+), 51 deletions(-) create mode 100644 docs/_posts/2025-12-31-bit-fields-api.markdown diff --git a/cache/clock_cache.cc b/cache/clock_cache.cc index dbd2a5b8fc..70155791a4 100644 --- a/cache/clock_cache.cc +++ b/cache/clock_cache.cc @@ -210,7 +210,7 @@ inline bool ClockUpdate(ClockHandle& h, BaseClockTable::EvictionData* data, // motivates only checking for release counter in high state, not both in high // state.) inline void CorrectNearOverflow(SlotMeta old_meta, - AcqRelBitFieldsAtomic& meta) { + BitFieldsAtomic& meta) { // We clear both top-most counter bits at the same time. constexpr uint32_t kCounterTopBit = uint32_t{1} << (SlotMeta::kCounterNumBits - 1); @@ -1924,7 +1924,7 @@ class AutoHyperClockTable::ChainRewriteLock { } } - AcqRelBitFieldsAtomic* head_ptr_; + BitFieldsAtomic* head_ptr_; NextWithShift saved_head_; }; diff --git a/cache/clock_cache.h b/cache/clock_cache.h index cdee0e93e5..efce8a69e3 100644 --- a/cache/clock_cache.h +++ b/cache/clock_cache.h @@ -413,7 +413,7 @@ struct ClockHandle : public ClockHandleBasicData { // TODO: make these coundown values tuning parameters for eviction? // See above. Mutable for read reference counting. - mutable AcqRelBitFieldsAtomic meta{}; + mutable BitFieldsAtomic meta{}; }; // struct ClockHandle class BaseClockTable { @@ -538,13 +538,13 @@ class BaseClockTable { // TODO: is this separation needed if we don't do background evictions? ALIGN_AS(CACHE_LINE_SIZE) // Number of elements in the table. - AcqRelAtomic occupancy_{}; + Atomic occupancy_{}; // Memory usage by entries tracked by the cache (including standalone) - AcqRelAtomic usage_{}; + Atomic usage_{}; // Part of usage by standalone entries (not in table) - AcqRelAtomic standalone_usage_{}; + Atomic standalone_usage_{}; // Maximum total charge of all elements stored in the table. // (Relaxed: eventual consistency/update is OK) @@ -899,8 +899,8 @@ class AutoHyperClockTable : public BaseClockTable { // See above. The head pointer is logically independent of the rest of // the entry, including the chain next pointer. - AcqRelBitFieldsAtomic head_next_with_shift{kUnusedMarker}; - AcqRelBitFieldsAtomic chain_next_with_shift{kUnusedMarker}; + BitFieldsAtomic head_next_with_shift{kUnusedMarker}; + BitFieldsAtomic chain_next_with_shift{kUnusedMarker}; // For supporting CreateStandalone and some fallback cases. inline bool IsStandalone() const { @@ -1056,7 +1056,7 @@ class AutoHyperClockTable : public BaseClockTable { // To maximize parallelization of Grow() operations, this field is only // updated opportunistically after Grow() operations and in DoInsert() where // it is found to be out-of-date. See CatchUpLengthInfoNoWait(). - AcqRelAtomic length_info_; + Atomic length_info_; // An already-computed version of the usable length times the max load // factor. Could be slightly out of date but GrowIfNeeded()/Grow() handle diff --git a/db/db_wal_test.cc b/db/db_wal_test.cc index 75e13724a7..641daeb0bf 100644 --- a/db/db_wal_test.cc +++ b/db/db_wal_test.cc @@ -1613,7 +1613,7 @@ TEST_F(DBWALTest, SyncWalPartialFailure) { return s; } - AcqRelAtomic syncs_before_failure_{UINT32_MAX}; + Atomic syncs_before_failure_{UINT32_MAX}; protected: class MyTestWritableFile : public FSWritableFileOwnerWrapper { diff --git a/db/db_write_test.cc b/db/db_write_test.cc index f6eeac7c5e..97fb86c14c 100644 --- a/db/db_write_test.cc +++ b/db/db_write_test.cc @@ -741,7 +741,7 @@ TEST_P(DBWriteTest, LockWALConcurrentRecursive) { ExternalSstFileInfo external_info; ASSERT_OK(sst_file_writer.Finish(&external_info)); } - AcqRelAtomic parallel_ingest_completed{false}; + Atomic parallel_ingest_completed{false}; port::Thread parallel_ingest{[&]() { IngestExternalFileOptions ingest_opts; ingest_opts.move_files = true; // faster than copy @@ -750,7 +750,7 @@ TEST_P(DBWriteTest, LockWALConcurrentRecursive) { parallel_ingest_completed.Store(true); }}; - AcqRelAtomic flush_completed{false}; + Atomic flush_completed{false}; port::Thread parallel_flush{[&]() { FlushOptions flush_opts; // NB: Flush with wait=false case is tested above in LockWALInEffect @@ -762,7 +762,7 @@ TEST_P(DBWriteTest, LockWALConcurrentRecursive) { flush_completed.Store(true); }}; - AcqRelAtomic parallel_put_completed{false}; + Atomic parallel_put_completed{false}; port::Thread parallel_put{[&]() { // This can make certain failure scenarios more likely: // sleep(1); diff --git a/docs/_posts/2025-12-31-bit-fields-api.markdown b/docs/_posts/2025-12-31-bit-fields-api.markdown new file mode 100644 index 0000000000..40d1b60f53 --- /dev/null +++ b/docs/_posts/2025-12-31-bit-fields-api.markdown @@ -0,0 +1,279 @@ +--- +title: "BitFields API: Type-Safe Bit Packing for Lock-Free Data Structures" +layout: post +author: pdillinger +category: blog +--- + +Modern concurrent data structures increasingly rely on [atomic operations](https://en.cppreference.com/w/cpp/atomic/atomic) to avoid the overhead of locking. A valuable but under-utilized technique for maximizing the effectiveness of atomic operations is [bit packing](https://en.wikipedia.org/wiki/Bit_field)---fitting multiple logical fields into a single atomic variable for algorithmic simplicity and efficiency. However, language support for bit packing does not guarantee dense packing, and manually managing bit manipulation quickly becomes error-prone, especially when dealing with complex state machines. + +To address this in RocksDB, we have developed a reusable **BitFields API**, a type-safe, zero-overhead abstraction for bit packing in C++. This works in conjunction with clean wrappers for `std::atomic` for powerful and relatively safe bit-packing of atomic data. For broader use, a [variant of the code](https://github.com/facebook/folly/pull/2549) has been proposed for adding to folly. + +## The Problem: Managing Packed Bit Fields + +Consider HyperClockCache, an essentially lock-free cache implementation in RocksDB, which was [refactored to use this BitFields API](https://github.com/facebook/rocksdb/pull/14154). It is a hash table built on *slots* that can each hold a cache entry and relevant metadata. For atomic simplicity and efficiency, all the essential metadata for each slot is packed into a single 64-bit value: +- The reference count and eviction metadata are together encoded into *acquire* and *release* counters, 30 bits each. +- The possible states of {*empty*, *under construction/destruction*, *occupied+visible*, and *occupied+invisible*} are encoded into three state bits (instead of two, for easier decoding and manipulation). +- A *hit* bit is used for secondary cache integration. + +Traditionally, you might write code like this: + +```cpp +// Old approach: manual bit manipulation +constexpr uint64_t kAcquireCounterShift = 0; +constexpr uint64_t kReleaseCounterShift = 30; +constexpr uint64_t kCounterMask = 0x3FFFFFFF; +constexpr uint64_t kHitBitShift = 60; +constexpr uint64_t kOccupiedShift = 61; +constexpr uint64_t kShareableShift = 62; +constexpr uint64_t kVisibleShift = 63; +constexpr uint64_t kStateShift = kOccupiedShift; + +std::atomic meta_; + +bool IsUnderConstruction(uint64_t meta) const { + return (meta & (uint64_t{1} << kOccupiedShift)) && !(meta & (uint64_t{1} << kShareableShift)); +} + +// Getting fields +uint64_t meta = meta_.load(std::memory_order_acquire); +if (IsUnderConstruction(meta)) { + // ... +} else if ((meta >> kVisibleShift) & 1) { + uint32_t refcount = + static_cast(((meta >> kAcquireCounterShift) - + (meta >> kReleaseCounterShift)) & kCounterMask); + // ... +} + + +// Setting fields + +// Set the hit bit (relaxed) +meta_.fetch_or(uint64_t{1} << kHitBitShift, std::memory_order_relaxed); + +// Set both counters to `new_count` (as in eviction processing) +uint64_t meta = meta_.load(std::memory_order_relaxed); +uint64_t new_meta = + (meta & ((uint64_t{1} << kHitBitShift) | (uint64_t{7} << kStateShift))) | + (new_count << kReleaseCounterShift) | + (new_count << kAcquireCounterShift); +bool success = meta_.compare_exchange_strong(meta, new_meta, + std::memory_order_acq_rel); + +// Increment acquire counter by initial_countdown +old_meta = meta_.fetch_add((uint64_t{1} << kAcquireCounterShift) * initial_countdown, + std::memory_order_acq_rel); +``` + +This approach has several problems: +1. **Error-prone**: Easy to get masks and shifts wrong +2. **Maintenance burden**: Changes to field sizes require updating multiple constants +3. **Abstraction challenges**: Even if writing a full set of well-tested getters and setters to hide all the details, details can leak in to do things like update multiple fields in one non-CAS (compare-and-swap) atomic operation. + +## New Solution: BitFields API + +The BitFields API provides a declarative, type-safe way to define bit-packed structures. Here's how the same example looks with BitFields: + +```cpp +// New approach: declarative bit fields. (Each field must reference the +// previous, so that the declaration machinery is simply stateless.) +struct SlotMeta : public BitFields { + using AcquireCounter = UnsignedBitField; + using ReleaseCounter = UnsignedBitField; + using HitFlag = BoolBitField; + using OccupiedFlag = BoolBitField; + using ShareableFlag = BoolBitField; + using VisibleFlag = BoolBitField; + + // Convenience helpers + bool IsUnderConstruction() const { + return Get() && !Get(); + } +}; + +BitFieldsAtomic meta_; + +// Getting fields +SlotMeta state = meta_.Load(); +if (state.IsUnderConstruction()) { + // ... +} else if (state.Get()) { + uint32_t refcount = state.Get() - + state.Get(); + // ... +} + +// Setting fields + +// Set the hit bit (relaxed) +meta_.ApplyRelaxed(SlotMeta::HitFlag::SetTransform()); + +// Set both counters to `new_count` (as in eviction processing) +SlotMeta meta = meta_.LoadRelaxed(); +SlotMeta new_meta = meta; +new_meta.Set(new_count); +new_meta.Set(new_count); +meta_.CasStrongRelaxed(meta, new_meta); + +// Increment acquire counter by initial_countdown +auto add_acquire = + AcquireCounter::PlusTransformPromiseNoOverflow(initial_countdown); +meta_.Apply(add_acquire, &old_meta); + +// Bonus: Atomic multi-field updates without compare-exchange +auto transform = AcquireCounter::PlusTransformPromiseNoOverflow(1) + + ReleaseCounter::PlusTransformPromiseNoOverflow(1); +meta_.Apply(transform); +``` + +## Key Features + +### Type Safety and Self-Documentation + +Each field has a specific type (`bool` for `BoolBitField`, appropriately-sized unsigned int for `UnsignedBitField`) and clear semantic meaning. The field definitions are self-documenting: you can immediately see how many bits each field occupies and in what order. + +### [Zero Overhead](https://en.cppreference.com/w/cpp/language/Zero-overhead_principle) + +Because of heavy use of templates and constexpr operations and the ability to satisfy multiple field reads or writes from a single atomic operation, we have seen no runtime overhead vs. hand-written bit manipulation, in RocksDB. In one case, we verified the assembly code was identical. + +[For folly's LifoSem](https://github.com/facebook/folly/pull/2550), there was one case where an optimization hack with detected overflow from one field to another couldn't be replicated as efficiently with the BitFields API because it would violate overflow checking. For that case I dove into the underlying representation to bypass the BitFields overflow check. + +### Atomic Operations with Transforms + +One of the most powerful features is the ability to combine multiple field updates into a single atomic operation using "transforms", if they are all either (a) some combination of addition and subtraction, (b) bitwise-and, or (c) bitwise-or. For example: + +```cpp +// Clear several but not all fields atomically +auto and_transform = Field1::AndTransform(0) + + Field2::ClearTransform() + + Field4::ClearTransform(); +atomic_bitfields.Apply(and_transform, &old_state, &new_state); +... +// Set more than one boolean field atomically +auto or_transform = Field2::SetTransform() + + Field4::SetTransform(); +atomic_bitfields.Apply(or_transform, &old_state, &new_state); +... +auto add_transform = Field1::PlusTransformPromiseNoOverflow(1) + + Field3::MinusTransformPromiseNoUnderflow(1); +atomic_bitfields.Apply(add_transform, &old_state, &new_state); +``` + +Each `Apply()` generates a single atomic operation (e.g., `fetch_add` or `fetch_or`) that updates all the specified fields, and optionally returns both the old and new values. This enables a number of hacks for atomic updates without CAS. + +### Overflow Protection + +The API includes built-in overflow detection in debug builds: + +```cpp +// An assertion will fail in debug builds if the counter overflows +auto transform = Counter::PlusTransformPromiseNoOverflow(value); +atomic.Apply(transform); +``` + +For fields at the top of the underlying representation (where overflow doesn't affect other fields), overflow is explicitly ignored. (A compile time error is generated if you try to use `PlusTransformPromiseNoOverflow` on a field at the top of the representation or `PlusTransformIgnoreOverflow` on a field not at the top of the representation.) + +```cpp +// For wraparound counters +auto transform = Counter::PlusTransformIgnoreOverflow(value); +``` + +This capability is used in a folly data structure called LifoSem, which [I have proposed to refactor](https://github.com/facebook/folly/pull/2550) to a proposed BitFields API variant for folly. + +### Compare-and-Swap (CAS) Support + +The atomic wrappers provide full CAS support for lock-free algorithms: + +```cpp +SlotMeta expected = current_state; +SlotMeta desired = expected.With(new_value).With(true); +if (meta_.CasStrong(expected, desired)) { + // Successfully updated + ... +} +``` + +### Atomic wrappers + +The BitFields API includes two atomic wrappers: `RelaxedBitFieldsAtomic` and `BitFieldsAtomic`. However, RocksDB also has versions of these wrappers for regular `std::atomic` variables that help with memory ordering discipline: `RelaxedAtomic` and `Atomic` in `util/atomic.h`. + +These wrappers help in a couple of ways: +* **Self-document intended memory order**: An atomic field generally has a single memory order that all or most operations should use, typically either `std::memory_order_relaxed` or `std::memory_order_acq_rel`. +* **More intentional memory orders and atomic operations**: The standard library's implicit conversions and default memory ordering (`memory_order_seq_cst`) make it easy to accidentally use sequential consistency with acquire/release ordering or even relaxed, which could hurt performance, and tend to hide where atomic operations are actually happening (e.g. implicit vs. explicit load). + +For example, instead of writing: +```cpp +std::atomic stat_counter; +stat_counter++; // Uses memory_order_seq_cst implicitly - maybe inefficient +``` + +You write: +```cpp +RelaxedAtomic stat_counter; +stat_counter.FetchAddRelaxed(1); // Explicitly relaxed - appropriate for a diagnostic counter +``` + +Or for data providing synchronization: +```cpp +Atomic refcount; +refcount.FetchAdd(1); // Standard acquire-release semantics for coordinating with other threads +``` + +These wrappers complement the BitFields atomic wrappers by providing the same ordering discipline for non-packed atomic variables throughout much of RocksDB, creating a more readable and less clunky approach to concurrent programming. Migrating remaining uses of `std::atomic` is an ongoing effort. + +## Real-World Usage in RocksDB + +The BitFields API was developed along with the revamped parallel compression in RocksDB, but with the intention to also clean up the HyperClockCache (HCC) implementation. With that migration complete, we can see the benefits. Specifically, **by packing more of the state machine into a single atomic value, the parallel algorithms became both simpler and more efficient.** Concurrent algorithms that could have blown up in their state space with elaborate interleavings between threads trying not to block each other, e.g. because of multi-step consensus on work assignments, were instead able to quickly and more easily make progress, e.g. with atomically clear work assignments. + +### Before: Manual Bit Manipulation + +The old HCC code was difficult to read and maintain. Many of the common read and update operations had manually written helper functions, but it was not practical to develop the full set of functions needed for rare cases. Consider this code that clears the "visible" flag on a slot when an entry is erased from subsequent lookups but might still be referenced: + +```cpp +// Old HCC code, without atomic wrappers +uint64_t old_meta = + h->meta.fetch_and(~(uint64_t{ClockHandle::kStateVisibleBit} + << ClockHandle::kStateShift), std::memory_order_acq_rel); +// Apply update to local copy +uint64_t new_meta = old_meta & ~(uint64_t{ClockHandle::kStateVisibleBit} + << ClockHandle::kStateShift); + +// New HCC code +SlotMeta old_meta, new_meta; +h->meta.Apply(SlotMeta::VisibleFlag::ClearTransform(), &old_meta, &new_meta); +``` + +Or this assertion that the acquire and release counters are different: + +```cpp +// Old HCC code +uint64_t old_meta = ...; +assert(((old_meta >> ClockHandle::kAcquireCounterShift) & + ClockHandle::kCounterMask) != + ((old_meta >> ClockHandle::kReleaseCounterShift) & + ClockHandle::kCounterMask)); + +// New HCC code without single-purpose helper functions +SlotMeta old_meta = ...; +assert(old_meta.Get() != + old_meta.Get()); + +// New HCC code, with single-purpose helper functions +SlotMeta old_meta = ...; +assert(old_meta.GetAcquireCounter() != old_meta.GetReleaseCounter()); +``` + +Some hand-written helper functions or using directives are still useful for brevity, but even without them all the bit manipulation details are hidden in the BitFields implementation. + +## Future Directions + +We hope the proposed folly version is accepted to make the BitFields API available for broader usage. Additionally, some quality-of-life improvements are likely possible, perhaps including easier declaration and usage syntax, hopefully without delving into boost-like macro hell. Better runtime and compile time checks might also be possible. + +## Conclusion + +The BitFields API demonstrates that zero-overhead abstractions can significantly improve code quality without sacrificing performance. By providing type safety, self-documentation, and convenience features around bit manipulation and atomic operations, it makes lock-free programming more accessible and maintainable. Bit-packed atomics are arguably essential for *slaying the complexity dragon* of efficient lock-free and low-lock algorithms, because they reduce explosion in algorithm states. + +For RocksDB specifically, the migration to BitFields has made the HyperClockCache implementation substantially easier to understand and modify, while maintaining the same high-performance characteristics. Combined with the recent [parallel compression revamp](/blog/2025/10/08/parallel-compression-revamp.html), these improvements showcase our ongoing commitment to writing clean, efficient, and maintainable code. + +The BitFields API is available in RocksDB's util/bit_fields.h and can be adapted for use in other projects requiring efficient, type-safe bit packing. For those building high-performance concurrent systems, it offers a compelling alternative to manual bit manipulation—proving that safe abstractions and peak performance are not mutually exclusive. diff --git a/memtable/inlineskiplist.h b/memtable/inlineskiplist.h index a25436af49..d39091ec6d 100644 --- a/memtable/inlineskiplist.h +++ b/memtable/inlineskiplist.h @@ -372,7 +372,7 @@ struct InlineSkipList::Node { private: // next_[0] is the lowest level link (level 0). Higher levels are // stored _earlier_, so level 1 is at next_[-1]. - AcqRelAtomic next_[1]; + Atomic next_[1]; }; template @@ -813,7 +813,7 @@ char* InlineSkipList::AllocateKey(size_t key_size) { template typename InlineSkipList::Node* InlineSkipList::AllocateNode(size_t key_size, int height) { - auto prefix = sizeof(AcqRelAtomic) * (height - 1); + auto prefix = sizeof(Atomic) * (height - 1); // prefix is space for the height - 1 pointers that we store before // the Node instance (next_[-(height - 1) .. -1]). Node starts at diff --git a/memtable/skiplist.h b/memtable/skiplist.h index 979cffd111..594c6ec43c 100644 --- a/memtable/skiplist.h +++ b/memtable/skiplist.h @@ -197,14 +197,14 @@ struct SkipList::Node { private: // Array of length equal to the node height. next_[0] is lowest level link. - AcqRelAtomic next_[1]; + Atomic next_[1]; }; template typename SkipList::Node* SkipList::NewNode( const Key& key, int height) { - char* mem = allocator_->AllocateAligned( - sizeof(Node) + sizeof(AcqRelAtomic) * (height - 1)); + char* mem = allocator_->AllocateAligned(sizeof(Node) + + sizeof(Atomic) * (height - 1)); return new (mem) Node(key); } diff --git a/table/block_based/block_based_table_builder.cc b/table/block_based/block_based_table_builder.cc index 74c90edea0..c4e6895a21 100644 --- a/table/block_based/block_based_table_builder.cc +++ b/table/block_based/block_based_table_builder.cc @@ -318,7 +318,7 @@ struct BlockBasedTableBuilder::ParallelCompressionRep { // simplify the interesting interleavings that have to be considered and // accommodated. struct State : public BitFields {}; - ALIGN_AS(CACHE_LINE_SIZE) AcqRelBitFieldsAtomic atomic_state; + ALIGN_AS(CACHE_LINE_SIZE) BitFieldsAtomic atomic_state; // The first field is a bit for each ring buffer slot (max 32) for whether // that slot is ready to be claimed for writing by a worker thread. Because diff --git a/util/atomic.h b/util/atomic.h index 94575fc7ca..209be20f3f 100644 --- a/util/atomic.h +++ b/util/atomic.h @@ -13,28 +13,28 @@ namespace ROCKSDB_NAMESPACE { // Background: // std::atomic is somewhat easy to misuse: -// * Implicit conversion to T using std::memory_order_seq_cst, along with -// memory order parameter defaults, make it easy to accidentally mix sequential -// consistency ordering with acquire/release memory ordering. See -// "The single total order might not be consistent with happens-before" at -// https://en.cppreference.com/w/cpp/atomic/memory_order +// * Implicit conversion to T makes it easy to use an unnecessarily strong +// memory ordering (std::memory_order_seq_cst) and to hide atomic operations +// that should be evident on reading the code. +// * Similarly, defaulting to std::memory_order_seq_cst for atomic operations +// makes it easy to use unnecessarily strong orderings. (It's always safe if +// some ordering is safe, but it's better to be intentional and thoughtful when +// carefully optimizing code with atomics.) Legitimate needs for seq_cst vs. +// acq_rel are rare, such as drawing inferences across two atomics in +// implementing hazard pointers. // * It's easy to use nonsensical (UB) combinations like store with -// std::memory_order_acquire. -// * It is unlikely that anything in RocksDB will need std::memory_order_seq_cst -// because sequential consistency for the user, potentially writing from -// multiple threads, is provided by explicit versioning with sequence numbers. -// If threads A & B update separate atomics, it's typically OK if threads C & D -// see those updates in different orders. +// std::memory_order_acquire. Getting these right in development is an +// unnecessary cognitive overhead even if they are caught by UBSAN. // -// For such reasons, we provide wrappers below to make safe usage easier. +// For such reasons, we provide wrappers below to make clear and explicit +// usage of atomics easier. -// Wrapper around std::atomic to avoid certain bugs (see Background above). +// Wrapper around std::atomic for better code clarity (see Background above). // -// This relaxed-only wrapper is intended for atomics that do not need -// ordering constraints with other data reads/writes aside from those -// necessary for computing data values or given by other happens-before -// relationships. For example, a cross-thread counter that never returns -// the same result can be a RelaxedAtomic. +// This relaxed-only wrapper is intended for atomics that are not used to +// synchronize other data across threads (only the atomic data), so can always +// used relaxed memory ordering. For example, a cross-thread counter that never +// returns the same result can be a RelaxedAtomic. template class RelaxedAtomic { public: @@ -72,14 +72,21 @@ class RelaxedAtomic { std::atomic v_; }; -// Wrapper around std::atomic to avoid certain bugs (see Background above). +// A reasonably general-purpose wrapper around std::atomic for better code +// clarity (see Background above). // -// Except for some unusual cases requiring sequential consistency, this is -// a general-purpose atomic. Relaxed operations can be mixed in as appropriate. +// Operations use std::memory_order_acq_rel by default (or just acquire or just +// release for read-only and write-only operations), but relaxed operations are +// also available and can be mixed in when appropriate. +// +// Future: add std::memory_order_seqcst variants like StoreSeqCst if/when +// there's a need for them (rare). No distinct type is needed because the +// distinction between acq_rel and seq_cst is more about where it is used in +// combination with other atomics than the atomic itself. template -class AcqRelAtomic : public RelaxedAtomic { +class Atomic : public RelaxedAtomic { public: - explicit AcqRelAtomic(T initial = {}) : RelaxedAtomic(initial) {} + explicit Atomic(T initial = {}) : RelaxedAtomic(initial) {} void Store(T desired) { RelaxedAtomic::v_.store(desired, std::memory_order_release); } diff --git a/util/bit_fields.h b/util/bit_fields.h index 1f2eb50b66..00a43ed901 100644 --- a/util/bit_fields.h +++ b/util/bit_fields.h @@ -19,7 +19,7 @@ namespace ROCKSDB_NAMESPACE { // fields into atomic variables to reduce the need for locking in concurrent // code and/or to simplify reasoning on and accommodation of different // interesting, bug-prone interleavings. Convenient atomic wrappers -// (RelaxedAtomic, AcqRelAtomic) are provided below to aid usage with atomics, +// (RelaxedAtomic, Atomic) are provided below to aid usage with atomics, // especially for CAS updates, but it is even possible to combine operations on // multiple bit fields into a single non-CAS atomic operation using Transforms // below. @@ -333,10 +333,11 @@ struct UnsignedBitField { } }; -// A handy wrapper for a relaxed atomic on some BitFields type (unlike -// RelaxedAtomic for arithmetic types). For encapsulation, usual arithmetic -// atomic operations are only available by calling Apply[Relaxed]() on -// Transforms returned from field classes. Extending an example from BitFields: +// A handy wrapper for a relaxed atomic on some BitFields type, like +// RelaxedAtomic but without direct arithmetic operations. For encapsulation, +// usual arithmetic atomic operations are only available by calling +// ApplyRelaxed() on Transforms returned from field classes. Extending an +// example from BitFields: // // auto transform = Field2::ClearTransform() + Field4::ClearTransform(); // MyState old_state; @@ -421,14 +422,15 @@ class RelaxedBitFieldsAtomic { }; // A handy wrapper for an aquire-release atomic (also relaxed semantics -// available) on some BitFields type. See RelaxedBitFieldsAtomic for more info. +// available) on some BitFields type. See RelaxedBitFieldsAtomic and +// Atomic in atomic.h for more info. template -class AcqRelBitFieldsAtomic : public RelaxedBitFieldsAtomic { +class BitFieldsAtomic : public RelaxedBitFieldsAtomic { public: using Base = RelaxedBitFieldsAtomic; using U = typename BitFieldsT::U; - explicit AcqRelBitFieldsAtomic(BitFieldsT initial = {}) : Base(initial) {} + explicit BitFieldsAtomic(BitFieldsT initial = {}) : Base(initial) {} void Store(BitFieldsT desired) { Base::v_.store(desired.underlying, std::memory_order_release); diff --git a/util/slice_test.cc b/util/slice_test.cc index cbc7289117..9106ec3c6e 100644 --- a/util/slice_test.cc +++ b/util/slice_test.cc @@ -590,7 +590,7 @@ TEST(BitFieldsTest, BitFields) { ASSERT_EQ(after.Get(), 0U); } { - AcqRelBitFieldsAtomic acqrel{state}; + BitFieldsAtomic acqrel{state}; ASSERT_EQ(state, acqrel.Load()); acqrel.Store(state2); ASSERT_EQ(state2, acqrel.Load());