[RFC] BlockMap and CChain Concurrency Improvement#34654
[RFC] BlockMap and CChain Concurrency Improvement#34654alexanderwiederin wants to merge 12 commits intobitcoin:masterfrom
BlockMap and CChain Concurrency Improvement#34654Conversation
|
♻️ Automatically closing for now based on heuristics. Please leave a comment, if this was erroneous. 📝 Moderators: If this is spam, please replace the title with |
|
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. ReviewsSee the guideline for information on the review process. ConflictsReviewers, this pull request conflicts with the following ones:
If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first. LLM Linter (✨ experimental)Possible typos and grammar issues:
2026-03-07 16:51:13 |
|
This was closed in error. |
5f2abda to
5611387
Compare
BlockMap and CChain Concurrency Improvement
cde5cf3 to
b46e12b
Compare
Introduce copy-on-write wrapper from stlab library to enable efficient value semantics with shared ownership. Uses atomic reference counting to share instances and only copies on modification when non-unique. Based on Adobe's stlab library, distributed under Boost Software License 1.0 Original source: https://github.com/stlab/stlab-copy-on-write/tree/abb4445 Co-authored-by: janb84 <608446+janb84@users.noreply.github.com>
replace the external pointer `phashBlock` with an owned `m_block_hash` member in CBlockIndex. Previously the hash was stored in the BlockMap key and CBlockIndex held a pointer to it, creating a lifetime dependency on the map's stable addressing. Storing the hash directly makes CBlockIndex self-contained, which is a prerequisite for moving to a copy-on-write BlockMap with shared_ptr<CBlockIndex>. Add a new constructor CBlockIndex(const uint256& hash) and delegate the existing CBlockHeader constructor to it. Remove all phashBlock assignemnts and replace with m_block_hash.
Replace the previous BlockMap (unordered_map<uint256, CBlockIndex>) with a new copy-on-write BlockMap that stores shared_pointer<CBlockIndex>. This is a prefactor for lock-free block index reads. The new BlockMap uses a two-tier structure (stable + recent) backed by stlab::copy-on-write. New entries are inserted into a small recent map and promoted to the stable COW map when the recent map exceeds a threshold. Copies of the BlockMap are cheap (refcount bump) and produce consistent point-in-time snapshots. Call sites are updated from value semantics (&senty.second) to pointer semantica (entry.second). No locking changes in this commit; cs_main guards are preserved as-is.
LookupBlockIndex and GetAllBlockIndices now take a snapshot copy instead of requiring cs_main. Add m_block_index_mutex to serialize writes in AddToBlockIndex and InsertBlockIndex. Split AddToBlockIndex into separate lock scopes so m_block_index_mutex and cs_main are not held simultaneously.
Remove cs_main locks that are no longer needed when calling BlockManager::LookupBlockIndex. Changes: - Remove cs_main locks from index/base.cpp, kernel/bitcoinkernel.cpp, net_processing.cpp, node/interfaces.cpp, node/miner.cpp, rpc/gettxoutproof.cpp, rpc/blockchain.cpp and rest.cpp - Remove unused validation.h and sync.h includes from kernel/coinstats.cpp - Update expected circular dependency path for kernel/coinstats due to removal of validation.h include
b46e12b to
b563960
Compare
Introduce a mutex to protect CChain's internal vector from concurrent access, eliminating the need for external locking with cs_main. Changes: - Add m_mutex member to CChain class and mark vChain as GUARDED_BY(m_mutex) - Add LOCK(m_mutex) to all public methods accessing vChain - Refactor Contains() and Next() to avoid calling operator[] (which would acquire the lock twice) by directly acessing vChain under a single lock - Update FindFork() to use local height variable and inline the Contains() check to avoid nested locking
Remove EXCLUSIVE_LOCKS_REQURIED annotations from ActiveChain(), ActiveHeight() and ActiveTip() methods, as CChain now handles its own thread safety internally with m_mutex. This change allows callers to access the active chain without holding cs_main.
Now that CChain handles its own thread safety internally via m_mutex, callers no longer need to hold cs_main when accessing chain data. Removes the redundant LOCK(cs_main) and WITH_LCOK(cs_main) annotations from call sites across RPC, indexes, node interfaces, REST and tests.
Replace cs_main lock with targeted locking and use BlockIndexSnapshot() to iterate over the block index.
b563960 to
5849975
Compare
RFC:
BlockMapandCChainConcurrency ImprovementFollow-up to #34424.
This RFC applies a
stableandrecent+ copy-on-write design toBlockMap, adds an internal Mutex toCChainand demonstrates howcs_mainlocks can be removed.Motivation
The current
BlockMapandCChainare protected bycs_main, creating bottlenecks for multi-threaded access.BlockMap Design
TL;DR: Split the block index into
stable(bulk of history) +recent(~1,000 blocks). Use nestedstlab::copy_on_writeso updates copy onlyrecent, keepingstableshared.Changes
BlockMapwithstable+recentarchitectureCChaincs_mainlocks across multiple call sites, usinggetchaintipsas an illustrative exampleNote: the lock removals in this RFC are non-exhaustive
Request for Comments
If you have opinions on the following design decisions, please weigh in:
CBlockIndexis required due to loss of address stability from the copy-on-write design, is the memory cost an acceptable tradeoff?