Skip to content

[RFC] BlockMap and CChain Concurrency Improvement#34654

Draft
alexanderwiederin wants to merge 12 commits intobitcoin:masterfrom
alexanderwiederin:blockmap-chain-concurrency
Draft

[RFC] BlockMap and CChain Concurrency Improvement#34654
alexanderwiederin wants to merge 12 commits intobitcoin:masterfrom
alexanderwiederin:blockmap-chain-concurrency

Conversation

@alexanderwiederin
Copy link

@alexanderwiederin alexanderwiederin commented Feb 23, 2026

RFC: BlockMap and CChain Concurrency Improvement

Follow-up to #34424.

This RFC applies a stable and recent + copy-on-write design to BlockMap, adds an internal Mutex to CChain and demonstrates how cs_main locks can be removed.

Motivation

The current BlockMap and CChain are protected by cs_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 nested stlab::copy_on_write so updates copy only recent, keeping stable shared.

Changes

  1. Stores block hash directly in CBlockIndex (required due to loss of address stability)
  2. Implements copy-on-write BlockMap with stable + recent architecture
  3. Adds internal mutex to CChain
  4. Removes cs_main locks across multiple call sites, using getchaintips as an illustrative example

Note: the lock removals in this RFC are non-exhaustive

Request for Comments

If you have opinions on the following design decisions, please weigh in:

  • Storing block hashes directly in CBlockIndex is required due to loss of address stability from the copy-on-write design, is the memory cost an acceptable tradeoff?
  • Are there simpler alternatives to copy-on-write here? A plain mutex might be sufficient, but full BlockMap iteration would hold the lock for extended periods.

@DrahtBot
Copy link
Contributor

♻️ Automatically closing for now based on heuristics. Please leave a comment, if this was erroneous.
Generally, please focus on creating high-quality, original content that demonstrates a clear
understanding of the project's requirements and goals.

📝 Moderators: If this is spam, please replace the title with ., so that the thread does not appear in
search results.

@DrahtBot DrahtBot closed this Feb 23, 2026
@DrahtBot
Copy link
Contributor

DrahtBot commented Feb 23, 2026

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Reviews

See the guideline for information on the review process.
A summary of reviews will appear here.

Conflicts

Reviewers, this pull request conflicts with the following ones:

  • #34489 (index: batch db writes during initial sync by furszy)
  • #34451 (rpc: fix race condition in gettxoutsetinfo by w0xlt)
  • #34416 (Add nullptr-check to CChain::Contains(), tests by optout21)
  • #33752 (rest: Query predecessor headers using negative count param by A-Manning)
  • #33477 (Rollback for dumptxoutset without invalidating blocks by fjahr)
  • #32554 (bench: replace embedded raw block with configurable block generator by l0rinc)
  • #32317 (kernel: Separate UTXO set access from validation functions by sedited)
  • #30342 (kernel, logging: Pass Logger instances to kernel objects by ryanofsky)
  • #28690 (build: Introduce internal kernel library by sedited)

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:

  • exisitng -> existing [misspelling in comment "If block already exists, returns (pointer to exisitng, false)."]
  • Chec -> Check [misspelling in comment "Chec if block exists in the map."]

2026-03-07 16:51:13

@alexanderwiederin
Copy link
Author

alexanderwiederin commented Feb 23, 2026

This was closed in error.

alexanderwiederin and others added 5 commits March 5, 2026 09:28
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
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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Architecture & Performance

Development

Successfully merging this pull request may close these issues.

3 participants