Skip to content
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

Proposer builds block in parallel. (Consensus vs Execution) #12297

Merged
merged 5 commits into from
May 1, 2023

Conversation

terencechain
Copy link
Member

Supersedes #12251

This pull request adds the functionality of constructing consensus and execution parts of blocks in parallel using work groups. The goal is for the process of constructing blocks to be faster and more efficient. By utilizing work groups, different parts of the block can be constructed simultaneously, significantly reducing processing time

@terencechain terencechain requested a review from a team as a code owner April 18, 2023 00:05
@prestonvanloon
Copy link
Member

Could you put this into a new method and use a feature flag? Seems like a risky change to not be able to opt out.

@prestonvanloon
Copy link
Member

Could you add any benchmarks or data to help us understand how significant of a reduction in the processing time?

Copy link
Member

@prestonvanloon prestonvanloon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Requesting changes until we have more data to support the claim of significant reduction in block creation. #12302 will help with this data collection for builder routes.

@terencechain terencechain self-assigned this Apr 19, 2023
terencechain and others added 3 commits April 18, 2023 19:46
Comment on lines 89 to 93
idx, err := helpers.BeaconProposerIndex(ctx, head)
if err != nil {
return nil, fmt.Errorf("could not calculate proposer index %v", err)
}
sBlk.SetProposerIndex(idx)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note to the reviewer. The position of setting proposer index has moved up. It was originally after setting attestation, now it's moved up to after setting parent root. It makes the refactoring cleaner

@prestonvanloon
Copy link
Member

Since this PR, we've added more data collection to our cluster to show that setExecutionData takes 200ms when using builder. The other operations of the block construction consist of about 110ms worth of work. Therefore, this PR should give block construction an average savings of approximately 100ms when using builder route. When using a local block construction, setExecutionData takes only 14ms due to eager caching from the EL. Could we apply some form of eager caching from the builder?

@terencechain
Copy link
Member Author

terencechain commented Apr 26, 2023

Could we apply some form of eager caching from the builder?

@prestonvanloon It is the earliest we can get the header, anything earlier will probably result in an error from the relayer

@terencechain terencechain added OK to merge Ready For Review A pull request ready for code review labels May 1, 2023
@prylabs-bulldozer prylabs-bulldozer bot merged commit 5b8084b into develop May 1, 2023
@delete-merged-branch delete-merged-branch bot deleted the build-blk-parallel-2 branch May 1, 2023 14:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Ready For Review A pull request ready for code review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants