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

perf: optimize process_withdrawals and process_sync_committee_updates #486

Merged
merged 6 commits into from
Nov 30, 2023

Conversation

MegaRedHand
Copy link
Collaborator

No description provided.

@MegaRedHand MegaRedHand requested a review from a team as a code owner November 29, 2023 20:48
@MegaRedHand MegaRedHand force-pushed the optimize-withdrawals-sync_committee_updates branch from b63413c to eb2067c Compare November 29, 2023 22:40
state.validators
|> Stream.filter(&Predicates.is_active_validator(&1, epoch))
|> Stream.map(fn %Validator{effective_balance: effective_balance} -> effective_balance end)
|> Enum.sum()
Copy link
Collaborator

Choose a reason for hiding this comment

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

is there any benefit in using stream here considering that you are going to do a Enum.sum afterwards?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

We would need benchmarks to know whether Stream or Enum is more efficient for this case, but at least it should consume less memory, as it can sum the result in batches, instead of first transforming the whole list and then reducing it.

Copy link
Collaborator

@mpaulucci mpaulucci left a comment

Choose a reason for hiding this comment

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

🎉

@MegaRedHand MegaRedHand merged commit 914cb3b into main Nov 30, 2023
9 checks passed
@MegaRedHand MegaRedHand deleted the optimize-withdrawals-sync_committee_updates branch November 30, 2023 16:59
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.

2 participants