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: ReuseListIterator for getAll() api #390

Merged
merged 6 commits into from
Jul 31, 2024

Conversation

twoeths
Copy link
Contributor

@twoeths twoeths commented Jul 31, 2024

Motivation

  • avoid possible array allocation with getAll() api

#383 make it possible to provide an Array for getAll() api, however it could still spike due to array allocation

Description

  • Implement ReusableListIterator which is a LinkedList, when it grows it only append some more items instead of allocating a big array
  • Implement getAll*Iter() api to View and ViewDU to allow consumer to use ReusableListIterator
  • On average, ReusableListIterator is only a bit faster than Array however the key thing is to reduce any spikes as much as possible due to gc

Testing

  • This branch (lg1k) in the last 24h, 10m rate interval, there are ~3 spikes
Screenshot 2024-07-31 at 11 20 18
  • te/batch_hash_tree_root, md64, 10m rate interval, there are ~18 spikes
Screenshot 2024-07-31 at 11 20 59
  • This branch (lg1k) in the last 24h, 12h rate interval. It's only a little bit faster than te/batch_hash_tree_root
Screenshot 2024-07-31 at 11 24 35
  • te/batch_hash_tree_root, md64, 12h rate interval
Screenshot 2024-07-31 at 11 25 16

@twoeths
Copy link
Contributor Author

twoeths commented Jul 31, 2024

note that there are still spikes in lodestar, see ChainSafe/lodestar#6979

@twoeths twoeths marked this pull request as ready for review July 31, 2024 08:25
@twoeths twoeths requested a review from a team as a code owner July 31, 2024 08:25
@twoeths twoeths merged commit 04f8a16 into te/batch_hash_tree_root Jul 31, 2024
7 checks passed
@twoeths twoeths deleted the te/reusable_list_iterator branch July 31, 2024 08:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant