lighthouse icon indicating copy to clipboard operation
lighthouse copied to clipboard

Published 20 hours ago • verified publisher icon sigp

Deduplicate block root computation

Open paulhauner opened this issue 2 years ago • 1 comments

Issue Addressed

NA

Proposed Changes

This PR removes duplicated block root computation.

Computing the SignedBeaconBlock::canonical_root has become more expensive since the merge as we need to compute the merke root of each transaction inside an ExecutionPayload.

Computing the root for a mainnet block is taking ~10ms on my i7-8700K CPU @ 3.70GHz (no sha extensions). Given that our median seen-to-imported time for blocks is presently 300-400ms, removing a few duplicated block roots (~30ms) could represent an easy 10% improvement. When we consider that the seen-to-imported times include operations after the block has been placed in the early attester cache, we could expect the 30ms to be more significant WRT our seen-to-attestable times.

Additional Info

NA

paulhauner avatar Sep 20 '22 01:09 paulhauner

This is also part of the consensus context on tree-states, maybe we should work together to pull that out and consolidate it with this and https://github.com/sigp/lighthouse/pull/3263?

michaelsproul avatar Sep 21 '22 03:09 michaelsproul

This is also part of the consensus context on tree-states, maybe we should work together to pull that out and consolidate it with this and #3263?

Sounds reasonable. This does a fair amount of work in the networking stack to remove duplicate hashes there too, does the consensus context extend down into the networking side of things?

paulhauner avatar Sep 22 '22 06:09 paulhauner

does the consensus context extend down into the networking side of things?

I can figure that out myself! I'm going to start playing with tree-states now :)

paulhauner avatar Sep 22 '22 08:09 paulhauner

As you may have already discovered, it does not do any magic in networking land 😅

Maybe we can merge this PR almost as-is and I can open a follow-up PR with the consensus context. I've been thinking it would be cool to have a bunch of per-slot and per-epoch caches that get attached to the context to accelerate various things. And we could build these caches off the hot path of block verification by doing it for the next slot after the processing of a new block (maybe in state advance)

michaelsproul avatar Sep 22 '22 08:09 michaelsproul

bors r+

paulhauner avatar Sep 23 '22 03:09 paulhauner

Pull request successfully merged into unstable.

Build succeeded:

bors[bot] avatar Sep 23 '22 05:09 bors[bot]