-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Attestation aggregation: optimizations and benchmarks #7938
Attestation aggregation: optimizations and benchmarks #7938
Conversation
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed in 7 days if no further activity occurs. Thank you for your contributions. |
…ysmaticlabs/prysm into maxcover-optimizations-and-benchmarks
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.
Went through this a few times and feel really good about it. It is also very well gated behind flags and I know you put in a lot of thought into this. Excited to see it integrated into runtime and for future PR's!
Thank you! |
What type of PR is this?
What does this PR do? Why is it needed?
In this PR:
optMaxCoverAttestationAggregation()
method is added. See https://hackmd.io/@farazdagi/in-place-attagg for details on design of that optimized aggregation procedure. Once this feature is tested,optMaxCoverAttestationAggregation()
will replace existingBitlist
based implementation.opt_max_cover
) forattestation-aggregation-strategy
flag has been added (flag is tracked via Feature flag tracking: attestation-aggregation-strategy=opt_max_cover #8365). So, new aggregation is used when one of the following is set:attestation-aggregation-strategy=opt_max_cover
--dev
--dev
mode of beacon node.PRs extracted from this PR:
Which issues(s) does this PR fix?
Fixes #7871
Other notes for review
Aggregation()
function, because it updates attestations in place, so for each bench mark run you need to create another copy of attestations lists (and when usingb.StopTimer()/b.StartTimer()
inside thefor i := 0; i < b.N; i++
loop -- so that each case gets its own copy -- benchmarks seem to hang.