reduces intermediate vector allocations when recovering shreds #4289
Conversation
When recovering shreds, below two .collect::<Vec<_>>() are unnecessarily allocating a vector and can be skipped by using iterators: https://github.com/anza-xyz/agave/blob/40be9b6d1/ledger/src/shred/merkle.rs#L987 https://github.com/anza-xyz/agave/blob/40be9b6d1/ledger/src/blockstore.rs#L1089 Also we can reduce allocations by collecting a Vec<Vec<Shred>> instead of below append: https://github.com/anza-xyz/agave/blob/40be9b6d1/ledger/src/blockstore.rs#L844
behzadnouri
force-pushed
the
shred-recover-in-place
branch
from
089037a to
80daa82
Compare
| metrics.num_recovered += recovered_shreds | ||
| .iter() | ||
| .filter(|shred| shred.is_data()) | ||
| .count(); |
There was a problem hiding this comment.
this is moved inside the filter_map below.
| // Recovery rules: | ||
| // 1. Only try recovery around indexes for which new data or coding shreds are received | ||
| // 2. For new data shreds, check if an erasure set exists. If not, don't try recovery | ||
| // 3. Before trying recovery, check if enough number of shreds have been received | ||
| // 3a. Enough number of shreds = (#data + #coding shreds) > erasure.num_data | ||
| for (erasure_set, working_erasure_meta) in erasure_metas.iter() { |
There was a problem hiding this comment.
Might be easier to review by hiding the whitespace changes:
There was a problem hiding this comment.
Looks correct.
Is it possible to keep it an iterator at every step?
Not sure if it's desirable here but it seems like it should be doable without a Box?
merkle::recover -> shred::recover -> recover_shreds -> try_shred_recovery -> flatten -> filter_map -> collect
The references interfere, because we need a mutable reference here: but that is already passed to |
|
ah got it. |
Problem
When recovering shreds, below two
.collect::<Vec<_>>()are unnecessarily allocating a vector and can be skipped by using iterators:https://github.com/anza-xyz/agave/blob/40be9b6d1/ledger/src/shred/merkle.rs#L987
https://github.com/anza-xyz/agave/blob/40be9b6d1/ledger/src/blockstore.rs#L1089
Also we can reduce allocations by collecting a
Vec<Vec<Shred>>instead of belowappend:https://github.com/anza-xyz/agave/blob/40be9b6d1/ledger/src/blockstore.rs#L844
Summary of Changes
The commit reduces intermediate vector allocations when recovering shreds.