Refactor LifoSem to use new bit field atomics API#2550
Open
pdillinger wants to merge 2 commits intofacebook:mainfrom
Open
Refactor LifoSem to use new bit field atomics API#2550pdillinger wants to merge 2 commits intofacebook:mainfrom
pdillinger wants to merge 2 commits intofacebook:mainfrom
Conversation
Summary: Inspired by work in RocksDB for HyperClockCache and parallel compression revamp, this change includes * BitFields.h - An API for declaring encapsulated bit field types with guaranteed layout, variant layout capability, and multi-field update capability * BitFieldsAtomic.h - Provides an extension of AtomicStruct for bit fields types, adding capability for non-CAS atomic updates to multiple fields (when possible). Together these lower the barriers to entry and maintenance hurdles associated with bit-packing atomic data in data structures and algorithms. Bit-packing data into atomics not only increases the scope of problems that can be solved with lock-free or low-locking usage of atomics, it can greatly simplify the algorithmic complexity. Packing more data fields into a single atomic means fewer overheads and hazards associated with interleaved updates to separate fields. In other words, with lean design and sufficient foresight, bit-packed atomic solutions can often be simpler *and* more efficient than solutions involving more atomics or synchronization primitives. Also see follow-up diff D88903968 that refactors existing structure LifoSem to use this new API. Differential Revision: D88903954
Summary: LifoSem is an existing synchronization data structure that bit-packs several logical data fields into a single atomic variable. This is a great example to convert over to use the new bit field atomic APIs because it shows the capabilities of the new APIs to minimize fragile boilerplate and maintenance complexity, and to make intention more clear from the code itself. This data structure uses one performance optimization hack that I was not able to replicate with the bit field atomic abstractions while preserving original performance. So in this case (`Ugly-but-optimized`) I preserve the hack by dipping into the underlying atomic (with some guardrails against breaking changes elsewhere). Note that this structure does use seq_cst memory order implicitly in some places that surely could be acq_rel, but I'm leaving that alone in this change. Differential Revision: D88903968
|
@pdillinger has exported this pull request. If you are a Meta employee, you can view the originating Diff in D88903968. |
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
1 participant
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.
Summary:
LifoSem is an existing synchronization data structure that bit-packs several logical data fields into a single atomic variable. This is a great example to convert over to use the new bit field atomic APIs because it shows the capabilities of the new APIs to minimize fragile boilerplate and maintenance complexity, and to make intention more clear from the code itself.
This data structure uses one performance optimization hack that I was not able to replicate with the bit field atomic abstractions while preserving original performance. So in this case (
Ugly-but-optimized) I preserve the hack by dipping into the underlying atomic (with some guardrails against breaking changes elsewhere).Note that this structure does use seq_cst memory order implicitly in some places that surely could be acq_rel, but I'm leaving that alone in this change.
Differential Revision: D88903968