generates erasure codes in-place using mutable references into shreds' payload #4609
Conversation
behzadnouri
force-pushed
the
make-merkle-shreds-from-data-in-place
branch
3 times, most recently
from
b066dff to
1492e51
Compare
behzadnouri
force-pushed
the
make-merkle-shreds-from-data-in-place
branch
from
1492e51 to
cb2040a
Compare
…' payload When making Merkle shreds from data, parity shards are first generated externally in a vector of vectors: https://github.com/anza-xyz/agave/blob/6ff4dee5f/ledger/src/shred/merkle.rs#L1325 and then copied into coding shreds payload: https://github.com/anza-xyz/agave/blob/6ff4dee5f/ledger/src/shred/merkle.rs#L1346 There are also many intermediate vector allocations in the process. The commit avoids this and minimizes allocations by first initializing all data and coding shreds in all erasure batches in a single vector. Then the erasure codes are generated and populated in-place using mutable references into the coding shreds' payload.
behzadnouri
force-pushed
the
make-merkle-shreds-from-data-in-place
branch
from
cb2040a to
b107d27
Compare
| .take(num_data_shreds); | ||
| make_shreds_data(&mut common_header_data, data_header, chunks).map(Shred::ShredData) | ||
| }); | ||
| if let Some(Shred::ShredData(shred)) = shreds.last() { |
There was a problem hiding this comment.
For my understanding, at this point there must be at least one data shred here right? Is it better to expect?
There was a problem hiding this comment.
I believe that is true.
Though it is a fair sanity check, I am a bit hesitant to add new panics into the code, in case some subtle condition is missed.
I will add a debug_assert though in a follow up PR.
| // Split the data into erasure batches and initialize | ||
| // data shreds from chunks of each batch. | ||
| let mut shreds = Vec::<ShredData>::new(); | ||
| // data and coding shreds for each batch. | ||
| while data.len() >= 2 * chunk_size || data.len() == chunk_size { |
There was a problem hiding this comment.
For my understanding the || data.len() == chunk_size here is to avoid having to perform the proof size search in the last fec set calculation below?
There was a problem hiding this comment.
Also curious about this case chunk_size < data.len() < 2 * chunk_size:
I understand that generally bigger FEC sets are better, so If we have 63 shreds worth of entries the code below will create one FEC set with 63:63 rather than doing something like 32:32 + 31:32
Is there any downside to always doing this? i.e removing the loop and making 1 big set of sizedata.len() / data_buffer_size + another padding set if necessary.
There was a problem hiding this comment.
the || data.len() == chunk_size here is to avoid having to perform the proof size search in the last fec set calculation below?
right, though I guess it is very unlikely to be the case.
Is there any downside to always doing this?
The only downside I can think of right now is that with larger erasure batches the Merkle tree and so the Merkle proof becomes larger. So if the Merkle proof has one additional entry, it will take an additional 20 bytes off each shred.
In other words, if you have 2 erasure batches of both 32: 32, and the data shreds in both are full (no more room in their data buffer), then the aggregated data does not fit in a single 64:64 batch. Because each of those shreds in the 64:64 batch use an additional 20 bytes for the Merkle proof and have smaller capacity.
Problem
When making Merkle shreds from data, parity shards are first generated externally in a vector of vectors:
https://github.com/anza-xyz/agave/blob/6ff4dee5f/ledger/src/shred/merkle.rs#L1325
and then copied into coding shreds payload:
https://github.com/anza-xyz/agave/blob/6ff4dee5f/ledger/src/shred/merkle.rs#L1346
There are also many intermediate vector allocations in the process.
Summary of Changes
The commit avoids this and minimizes allocations by first initializing all data and coding shreds in all erasure batches in a single vector. Then the erasure codes are generated and populated in-place using mutable references into the coding shreds' payload.
This change is similar to #4356.
This commit modifies the leader code when generating shreds, whereas #4356 updated the non-leader side when recovering shreds from erasure codes.