Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: consume new ssz batch hash branch #6939

Closed
wants to merge 26 commits into from

Conversation

twoeths
Copy link
Contributor

@twoeths twoeths commented Jul 9, 2024

Motivation

  • reduce hash_tree_root time in lodestar, especially at epoch transition time
  • this leverage the new hashtree which and new as-sha256 which support batch hash api

Description

  • consume te/batch_hash_tree_root ssz branch, the critical update there is using hashtree and minimal memory allocation to compute tree root
  • ssz PR: feat: batch hash tree root ssz#378
  • this follows the approach 1 as noted here
  • copy the implementation of ListValidatorTreeViewDU from the ssz branch, the main thing is to compute validators' roots in batch during commit() phase
  • hashTreeRoot() is improved in 2 ways:
    • batch hash validators, execute HashComputation[] in batch
    • memory optimization, a lot of metrics are better (see below)

part of ChainSafe/ssz#355
closes #6598

TODOs

  • Investigate why do hashTreeRoot() inside state transition causes more memory and gc? right now we do the hashTreeRoot() in beacon-node instead. Will retry when we have a published version because right now I only consume local ssz branch on test nodes. Update: resolved in fix: batch hash tree root in state transition #7032
  • after a BeaconState hashTreeRoot() is computed, if we call some read-only operations, like state.balances.length or state.validators.length, it will rebind and recompute the tree root again. Need to find a way to avoid that, see ViewDU container rebinds node with read-only operations ssz#379

@twoeths
Copy link
Contributor Author

twoeths commented Jul 9, 2024

posting some great metrics (the test mainnet node) captured from feat1 - 6h rate interval

  • Prepare Next Epoch time is ~600ms faster than beta/unstable and ~500ms faster on mainnet
Screenshot 2024-07-09 at 11 17 36
  • Beacon block gossip validation time is 50ms faster on mainnet node and 60ms faster on holesky
Screenshot 2024-07-09 at 11 19 24
  • Execution api notifyNewPayload is 30ms - 40ms faster on mainnet and 60ms faster on holesky
Screenshot 2024-07-09 at 11 24 28
  • Block transition is ~5ms faster on both holesky and mainnet
Screenshot 2024-07-09 at 11 22 19
  • This also reflected in the Avg block state transition time in verifyBlocksStateTransitionOnly.ts: 35ms faster on mainnet and 50ms faster on holesky
Screenshot 2024-07-09 at 11 30 18

@twoeths
Copy link
Contributor Author

twoeths commented Jul 9, 2024

  • "till become head" is ~30ms faster than stable on mainnet, somehow it's the same to beta mainnet node
Screenshot 2024-07-09 at 13 01 48
  • it's ~90ms faster on holesky (both beta + mainnet)
Screenshot 2024-07-09 at 13 02 36
  • "avg block recv to import delay" is consistent, it's 50ms - 60ms better than both beta and mainnet node
Screenshot 2024-07-09 at 13 18 04

these metrics are not the accumulation of all above better metrics because they also depend on blob I guess

@twoeths
Copy link
Contributor Author

twoeths commented Jul 17, 2024

a side effect from this branch is "Process block commit step avg time" is usually higher than stable (<= 5ms) because we also compute validators' root when commit. This metric and "Process block avg time" are not as important as "Avg block recv to state transition delay" (which is >=50ms better than stable/unstable), because we have to compute state root anyway

twoeths and others added 4 commits September 11, 2024 09:39
* fix: remove get*Iter() apis from ssz

* feat: decompose validators at epoch transition

* fix: improve beforeProcessEpoch() using state.validators.forEachValue()

* fix: only populate isCompoundingValidatorArr if electra

* fix: use forEach() to improve processEth1Data

* fix: use forEach() for getEffectiveBalanceIncrementsZeroInactive()

* chore: minimal change of beforeProcessEpoch() compared to te/batch_hash_tree_root
* feat: implement balances tree cache

* fix: set balancesTreeCache when clone EpochCache
@twoeths twoeths mentioned this pull request Oct 1, 2024
12 tasks
@twoeths
Copy link
Contributor Author

twoeths commented Oct 17, 2024

closing in favor of #7171

@twoeths twoeths closed this Oct 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Big state hashTreeRoot() time in lodestar
1 participant