Use less memory when serializing versioned epoch stakes #2520
Conversation
jstarry
force-pushed
the
fix/versioned-epoch-stakes-stream-serde
branch
from
b5fa1be to
fdb537c
Compare
jstarry
force-pushed
the
fix/versioned-epoch-stakes-stream-serde
branch
from
fdb537c to
a1180a4
Compare
| { | ||
| match self { | ||
| Self::Stake(stakes) => stakes.serialize(serializer), | ||
| Self::Account(stakes) => Stakes::<&Stake>::from(stakes).serialize(serializer), |
There was a problem hiding this comment.
Before we did a shallow clone here
| { | ||
| match self { | ||
| Self::Stake(stakes) => stakes.serialize(serializer), | ||
| Self::Account(stakes) => serialize_stake_accounts_to_stake_format(stakes, serializer), |
There was a problem hiding this comment.
Now we are doing a memory efficient serialization approach which doesn't need to allocate a new stakes map before serialization
| /// remains allocated during serialization. | ||
| #[cfg_attr(feature = "frozen-abi", derive(AbiExample, AbiEnumVisitor))] | ||
| #[derive(Debug, Clone)] | ||
| pub enum SerdeStakesToStakeFormat { |
There was a problem hiding this comment.
Yeah unfortunately BankFieldsToSerialize and ExtraFieldsToSerialize both need to be pub right now for some snapshot functions and structs so VersionedEpochStakes and SerdeStakesToStakeFormat both need to be pub as well. I think we could lower the visibility of everything in a follow up. I'll give it a shot in a follow up PR.
runtime/src/stakes/serde_stakes.rs
Outdated
| /// the stake data. Serialization works by building a `Stakes<&Stake>` map which | ||
| /// borrows `&Stake` from `StakeAccount` entries in `Stakes<StakeAccount>`. Note | ||
| /// that `Stakes<&Stake>` still copies `Pubkey` keys so the `Stakes<&Stake>` | ||
| /// data structure still allocates a fair amount of memory but the memory only |
There was a problem hiding this comment.
Where are the pubkeys copied? Isn't Stakes a bunch of nested Arcs + im::HashMap?
There was a problem hiding this comment.
Yeah you're right, this is an outdated comment, will clean it up
runtime/src/stakes/serde_stakes.rs
Outdated
| /// the stake data. Serialization works by building a `Stakes<&Stake>` map which | ||
| /// borrows `&Stake` from `StakeAccount` entries in `Stakes<StakeAccount>`. Note | ||
| /// that `Stakes<&Stake>` still copies `Pubkey` keys so the `Stakes<&Stake>` | ||
| /// data structure still allocates a fair amount of memory but the memory only |
There was a problem hiding this comment.
Where are the pubkeys copied? Isn't Stakes a bunch of nested Arcs + im::HashMap?
Problem
When serializing versioned epoch stakes, we do a shallow clone of the stake delegations map when serializing stake accounts in the stake format. Even this shallow clone uses more memory than necessary.
Summary of Changes
Use a streaming serializing approach when serializing stake accounts in the stake format similar to #2455
Fixes #