Skip to content

Add caching to CBlockHeader::GetHash

SlowRiot requested to merge SlowRiot/bitcoin-cash-node:cache_blockhash into master

Currently the block header GetHash() function runs through a fairly complex process when called, including serialisation, allocations, creation of a stream etc. This occurs every time the hash is retrieved.

Changes

This MR adds a cache of the hash result. This should allow for a small speedup in cases where the hash is accessed multiple times.

The hash is lazy-evaluated - it is stored only once GetHash() is called. If the hash is never requested, computation time is not wasted pre-calculating it, and as it is stored in a std::optional, storage requirement is minimal if it is not used.

Getters & setters, cache invalidation

To ensure the cache is invalidated when any of the variables that are used to calculate it change, direct access to the members of CBlockHeader are made private, and getters and setters are added. Each setter invalidates the cache.

I have also added a convenience constructor for the most common block construction case, which allows the setting of all the main variables on initialisation.

A common pattern is to increment the nonce, so rather than repeatedly using the verbose block.SetNonce(block.GetNonce() + 1) that results, I have added an IncrementNonce convenience function.

Last, in the code sometimes SetNull() is called on the hashPrevBlock member, so I have added a function to do this without having to copy through get and set.

Code that used to access the members of the class directly, now accesses them via the new access functions.

Thread safety

The block header may occasionally be accessed by multiple threads - this can be demonstrated by the cache_blockhash_assert branch, which is set up to trigger an abort if the block is accessed from multiple different threads (when configured with cmake -GNinja -DASSERT_BLOCKHEADER_THREAD_CONSISTENCY), as a rather coarse way to identify potential access across threads. Functional tests trigger the assert.

As a result of this and the discussion below, I decided to use a mutex to guard the optional cached value; a shared lock is obtained when reading the hash value, an exclusive lock is used when either setting it, or modifying any variables that affect it. Lock macros from sync.h are used.

Benchmark results

Benchmarks introduced in !1452 (merged) are used here, which is why this MR is on top of that one.

Results calculated by the script added in !1453 (merged): image

Benchmarks show significant speedups around block hash, proof of work, and transaction checks, with a small improvement in CoinSelection, and a small decline in performance in MatchGCSFilter.

Test plan

ninja check-bitcoin
ninja check-functional
src/bench/bench_bitcoin -filter="Deserialize.*"
src/bench/bench_bitcoin -filter="CheckBlock.*"
src/bench/bench_bitcoin -filter="CheckProof.*"
Edited by SlowRiot

Merge request reports