-
Notifications
You must be signed in to change notification settings - Fork 896
Optimize staked_nodes() for 3-4x performance improvement #8516
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
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -1,6 +1,5 @@ | ||||||
| use { | ||||||
| crate::vote_state_view::VoteStateView, | ||||||
| itertools::Itertools, | ||||||
| serde::{ | ||||||
| de::{MapAccess, Visitor}, | ||||||
| ser::Serializer, | ||||||
|
|
@@ -140,16 +139,20 @@ impl VoteAccounts { | |||||
| pub fn staked_nodes(&self) -> Arc<HashMap</*node_pubkey:*/ Pubkey, /*stake:*/ u64>> { | ||||||
| self.staked_nodes | ||||||
| .get_or_init(|| { | ||||||
| Arc::new( | ||||||
| self.vote_accounts | ||||||
| .values() | ||||||
| .filter(|(stake, _)| *stake != 0u64) | ||||||
| .map(|(stake, vote_account)| (*vote_account.node_pubkey(), stake)) | ||||||
| .into_grouping_map() | ||||||
| .aggregate(|acc, _node_pubkey, stake| { | ||||||
| Some(acc.unwrap_or_default() + stake) | ||||||
| }), | ||||||
| ) | ||||||
| // Pre-allocate HashMap with estimated capacity to reduce reallocations | ||||||
| let mut staked_nodes = | ||||||
| HashMap::with_capacity(self.vote_accounts.len().saturating_div(2)); | ||||||
|
||||||
| HashMap::with_capacity(self.vote_accounts.len().saturating_div(2)); | |
| HashMap::with_capacity_and_hasher(self.vote_accounts.len().saturating_div(2), PubkeyHasherBuilder::default()); |
That should speed it up even more. 🙂 You'll need to add the PubkeyHasherBuilder generic to the return type in the function signature.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PubkeyHasherBuilder is behind "rand" feature. vote package doesn't enable rand feature.
What's your experience on enable rand feature? Would it be a more broader change and require more testing to ensure no regression?
How about doing it as a separate optimization in a follow-up pr?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds good, we can do it separately.
Regarding adding the rand feature - in this PR #7307 I added the rand feature to solana-pubkey in solana-accounts-db. The only randomness that comes with it is randomization of 8 byte subslice of the pubkey to be used as a hash map key:
https://github.com/anza-xyz/solana-sdk/blob/a7e12b1d4af8fba2a43d447af31aebdc3dbe8a1d/address/src/lib.rs#L13-L14
https://github.com/anza-xyz/solana-sdk/blob/a7e12b1d4af8fba2a43d447af31aebdc3dbe8a1d/address/src/hasher.rs#L57-L81
There is no other module being pulled by this feature. So I don't think there should be any unexpected impact on the validator.
Uh oh!
There was an error while loading. Please reload this page.