Merge bitcoin/bitcoin#23819: ConnectBlock: don't serialize block hash twice

eb8b22d5176d7abc6f93b4473df446105ca595e6 block_connected: re-use previous GetTimeMicros (William Casarin)
80e1c55687aae61767f1ade0826746cda00d6a24 block_connected: don't serialize block hash twice (William Casarin)

Pull request description:

  In the validation:block_connected tracepoint, we call block->GetHash(), which
  ends up calling CBlockHeader::GetHash(), executing around 8000 serialization
  instructions. We don't need to do this extra work, because block->GetHash() is
  already called further up in the function. Let's save that value as a local
  variable and re-use it in our tracepoint so there is no unnecessary tracepoint
  overhead.

  Shave off an extra 100 or so instructions from the validation:block_connected
  tracepoint by reusing a nearby GetTimeMicros(). This brings the tracepoint down
  to 54 instructions.  Still high, but much better than the previous ~154 and
  8000 instructions which it was originally.

  Signed-off-by: William Casarin <jb55@jb55.com>

ACKs for top commit:
  0xB10C:
    ACK eb8b22d5176d7abc6f93b4473df446105ca595e6
  laanwj:
    Code review ACK eb8b22d5176d7abc6f93b4473df446105ca595e6
  theStack:
    re-ACK eb8b22d5176d7abc6f93b4473df446105ca595e6

Tree-SHA512: 92ae585e487554e0f73042a8abaa239f630502c1d198e010bd7c1de252d882bccb627bbf0e4faec09c1253e782b145bcf153f9fee78cdb8456188044a96f8267

1 files changed, 7 insertions, 4 deletions

diff --git a/src/validation.cpp b/src/validation.cpp
index b7a4002e48..5b52638fc5 100644
--- a/src/validation.cpp
+++ b/src/validation.cpp
@@ -1923,7 +1923,10 @@ bool CChainState::ConnectBlock(const CBlock& block, BlockValidationState& state,
 {
     AssertLockHeld(cs_main);
     assert(pindex);
-    assert(*pindex->phashBlock == block.GetHash());
+
+    uint256 block_hash{block.GetHash()};
+    assert(*pindex->phashBlock == block_hash);
+
     int64_t nTimeStart = GetTimeMicros();
 
     // Check it again in case a previous version let a bad block in
@@ -1957,7 +1960,7 @@ bool CChainState::ConnectBlock(const CBlock& block, BlockValidationState& state,
 
     // Special case for the genesis block, skipping connection of its transactions
     // (its coinbase is unspendable)
-    if (block.GetHash() == m_params.GetConsensus().hashGenesisBlock) {
+    if (block_hash == m_params.GetConsensus().hashGenesisBlock) {
         if (!fJustCheck)
             view.SetBestBlock(pindex->GetBlockHash());
         return true;
@@ -2214,12 +2217,12 @@ bool CChainState::ConnectBlock(const CBlock& block, BlockValidationState& state,
     LogPrint(BCLog::BENCH, "    - Index writing: %.2fms [%.2fs (%.2fms/blk)]\n", MILLI * (nTime5 - nTime4), nTimeIndex * MICRO, nTimeIndex * MILLI / nBlocksTotal);
 
     TRACE6(validation, block_connected,
-        block.GetHash().data(),
+        block_hash.data(),
         pindex->nHeight,
         block.vtx.size(),
         nInputs,
         nSigOpsCost,
-        GetTimeMicros() - nTimeStart // in microseconds (µs)
+        nTime5 - nTimeStart // in microseconds (µs)
     );
 
     return true;