Skip to content

Conversation

@twoeths
Copy link
Contributor

@twoeths twoeths commented Jan 2, 2021

resolves #1789, resolves #1878, and resolves #1761

  • Create a CachedValidatorsBeaconState that cache validators using persistent vector (see Improve performance twoeths/persistent-ts#1) and use it everywhere
  • Persistent vector is an immutable data structure, similar to the persistent-merkle-tree we used: it shares a lot of data across beacon state since validators are rarely changed and it has cheap clone()
  • Persistent vector loop speed is comparable to regular javascript array loop while it's slow if we loop through TreeBacked<BeaconState>.validators
  • When we want to mutate validators inside state, use setValidator() and addValidator() instead of accessing validators directly
  • Average epoch transition time in my local environment for Pyrmont sync was reduced from 3.5s-4.2s to 1.2s-1.6s. This should speed up the validator exit process too.

Testing

  • Pyrmont: synced from slot 0 to head and stay synced for more than 4 hours (tested 2 times)
  • Don't see any memory differences during Pyrmont sync
  • Performance tests
test master this branch
prepareEpochProcessState 1500 100
process block of 16 validator exits 6000 200
transition - 32 slots 2800 1100
transition - 64 slots 5300 2200
transition - 128 slots 11000 4300

TODO

@qlty-cloud-legacy
Copy link

qlty-cloud-legacy bot commented Jan 2, 2021

Code Climate has analyzed commit 4118dfd and detected 6 issues on this pull request.

Here's the issue category breakdown:

Category Count
Complexity 6

View more on Code Climate.

@twoeths twoeths marked this pull request as ready for review January 3, 2021 07:25
Copy link
Contributor

@dapplion dapplion left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome work! Looks great. I would vote for @chainsafe/persistent-ts to be eventually spin-out into a different repo

Copy link
Member

@wemeetagain wemeetagain left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is great, we should merge for now.

Going forward, we should take some time to possibly re-organize the beacon state, validator cache, epoch context, epoch process, etc. as needed.

This is mainly an issue of how we organize the various datastructures (BeaconState, CachedValidatorsBeaconState, EpochContext, EpochProcess, IStateContext). I'm not sure what the right organization is (possibly no re-organization needed) but what we have seems to be getting more cluttered as we've added more and more optimizations. We have several nested datastructures that are all related to the beacon state / epoch state.

My hunch is that we may be able to expand the CachedValidatorBeaconState to be more of a general CachedBeaconState at the 'top-level', like IStateContext, which exposes the BeaconState + extra methods, with various caches 'under the hood' (each cache using 'persistent-ts' or similar for cheap cloning).

@dapplion dapplion merged commit 74375f3 into master Jan 8, 2021
@dapplion dapplion deleted the tuyen/validators-as-persistent-vector branch January 8, 2021 23:35
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.

Explore Epoch Transition Speedups POC: Create a StateTransitionBeaconState Improve validator exit process

5 participants