lighthouse icon indicating copy to clipboard operation
lighthouse copied to clipboard

Optimise proposer index calculation in block processing

Open paulhauner opened this issue 2 years ago • 2 comments

Issue Addressed

NA

Proposed Changes

Avoid recomputing the active validator indices when computing the proposer index. Since we only have the shuffled active validator indices in our committee cache, this function maps an index in the unshuffled active validators to the shuffled active validators using unshuffling.

This is probably not a huge saving in the current scheme of block processing when we consider how long the THC takes, but I thought that this might be nice for tree-states. It avoids the iteration of computing the active validator indices, which I understand is more expensive with tree-states.

Benchmarks

Benchmarks on a recent mainnet block using #3252 to get timing.

Without this PR

lcli transition-blocks --block-path /tmp/block-0x6c69.ssz --pre-state-path /tmp/pre-state-0x6c69.ssz --runs 15 --exclude-cache-builds --exclude-post-block-thc --no-signature-verification 2>&1 | grep "Process block"
[2022-06-12T01:23:01Z DEBUG lcli::transition_blocks] Process block: 5.976771ms
[2022-06-12T01:23:01Z DEBUG lcli::transition_blocks] Process block: 5.956646ms
[2022-06-12T01:23:01Z DEBUG lcli::transition_blocks] Process block: 6.010116ms
[2022-06-12T01:23:01Z DEBUG lcli::transition_blocks] Process block: 5.975266ms
[2022-06-12T01:23:02Z DEBUG lcli::transition_blocks] Process block: 6.02316ms
[2022-06-12T01:23:02Z DEBUG lcli::transition_blocks] Process block: 6.034038ms
[2022-06-12T01:23:02Z DEBUG lcli::transition_blocks] Process block: 6.037002ms
[2022-06-12T01:23:02Z DEBUG lcli::transition_blocks] Process block: 5.97694ms
[2022-06-12T01:23:02Z DEBUG lcli::transition_blocks] Process block: 5.934441ms
[2022-06-12T01:23:02Z DEBUG lcli::transition_blocks] Process block: 5.943318ms
[2022-06-12T01:23:03Z DEBUG lcli::transition_blocks] Process block: 6.365686ms
[2022-06-12T01:23:03Z DEBUG lcli::transition_blocks] Process block: 5.890999ms
[2022-06-12T01:23:03Z DEBUG lcli::transition_blocks] Process block: 6.002739ms
[2022-06-12T01:23:03Z DEBUG lcli::transition_blocks] Process block: 5.989115ms
[2022-06-12T01:23:03Z DEBUG lcli::transition_blocks] Process block: 8.056736ms

With this PR:

lcli transition-blocks --block-path /tmp/block-0x6c69.ssz --pre-state-path /tmp/pre-state-0x6c69.ssz --runs 15 --exclude-cache-builds --exclude-post-block-thc --no-signature-verification 2>&1 | grep "Process block"
[2022-06-12T01:22:04Z DEBUG lcli::transition_blocks] Process block: 2.988006ms
[2022-06-12T01:22:04Z DEBUG lcli::transition_blocks] Process block: 3.044924ms
[2022-06-12T01:22:04Z DEBUG lcli::transition_blocks] Process block: 3.378032ms
[2022-06-12T01:22:05Z DEBUG lcli::transition_blocks] Process block: 3.015221ms
[2022-06-12T01:22:05Z DEBUG lcli::transition_blocks] Process block: 3.012321ms
[2022-06-12T01:22:05Z DEBUG lcli::transition_blocks] Process block: 3.068026ms
[2022-06-12T01:22:05Z DEBUG lcli::transition_blocks] Process block: 2.990344ms
[2022-06-12T01:22:05Z DEBUG lcli::transition_blocks] Process block: 4.755704ms
[2022-06-12T01:22:05Z DEBUG lcli::transition_blocks] Process block: 3.004696ms
[2022-06-12T01:22:06Z DEBUG lcli::transition_blocks] Process block: 3.127911ms
[2022-06-12T01:22:06Z DEBUG lcli::transition_blocks] Process block: 3.162573ms
[2022-06-12T01:22:06Z DEBUG lcli::transition_blocks] Process block: 3.3767ms
[2022-06-12T01:22:06Z DEBUG lcli::transition_blocks] Process block: 3.1083ms
[2022-06-12T01:22:06Z DEBUG lcli::transition_blocks] Process block: 3.038865ms
[2022-06-12T01:22:07Z DEBUG lcli::transition_blocks] Process block: 3.382932ms

Additional Info

NA

paulhauner avatar Jun 12 '22 01:06 paulhauner

On tree-states I've implemented an optimisation that avoids calculating the proposer index at all. Using the existing proposer shuffling cache the index is checked once during block verification and then plumbed through block processing with a ConsensusContext. I'm not opposed to merging this change as well/instead, but an alternative would be just to port the ConsensusContext from tree-states to unstable. It also carries the block root through from initial verification

michaelsproul avatar Jun 12 '22 02:06 michaelsproul

Oh yeah cool! We could potentially use this method for building the proposer shuffling cache. We can think more about it next week :)

paulhauner avatar Jun 12 '22 02:06 paulhauner

I'm going to close this since it's become much less useful since we're avoiding re-computation of the proposer index. The spiciness of reverse shuffling doesn't seem worth it now.

paulhauner avatar Nov 14 '22 05:11 paulhauner