-
Notifications
You must be signed in to change notification settings - Fork 878
fixed fec sets #5771
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
fixed fec sets #5771
Conversation
|
Currently seeing ~7% padding overhead running with 4eb25e2 This is similar to what Jump has observed using the same entry coalesce bytes target. |
|
As far as what is driving the need for padding, here are some logs that shed light: There are a couple of "bad" cases that lead to more padding:
A few potential options to get better here:
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #5771 +/- ##
=========================================
- Coverage 82.9% 82.9% -0.1%
=========================================
Files 830 830
Lines 376347 376498 +151
=========================================
+ Hits 312282 312386 +104
- Misses 64065 64112 +47 🚀 New features to boost your workflow:
|
|
Running the same experiment ( This code:
Master code:
|
21892c7 to
7c32454
Compare
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.
suggest moving the shredding logic to another file. merkle.rs is already huge enough.
|
|
||
| // Wait up to `ENTRY_COALESCE_DURATION` to try to coalesce entries into a 32 shred batch | ||
| let data_shred_bytes = | ||
| ShredData::capacity(Some((6, true, false))).expect("Failed to get capacity") as u64; |
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.
this is all constant, no need recomputing this every time
| ) { | ||
| // Fetch the next entry. | ||
| let Ok((try_bank, (entry, tick_height))) = receiver.recv_deadline( | ||
| coalesce_start + max_coalesce_time(serialized_batch_byte_count, max_batch_byte_count), |
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.
suggest we try to maybe wake up some 5 ms early to avoid OS being annoying and waking this thread 5 ms too late instead.
f52cb83 to
b6d5360
Compare
|
I think hyperoptimizing this is not necessary, as blocks become larger padding will disappear in the overall traffic. We just need to pack more TXs into the blocks in general. |








Problem
We would like to move to fixed (32:32) FEC set sizes to simplify multiple areas of the code.
This PR is simply a draft to prove out the concept and is not meant to be merged.
If we decide to pursue this, we will need to:
Summary of Changes