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

Improve BLS multi-thread worker pool strategy #2421

Merged
merged 6 commits into from Apr 26, 2021

Conversation

dapplion
Copy link
Contributor

@dapplion dapplion commented Apr 25, 2021

Motivation

Some weeks ago we landed a naive version of a worker thread pool to verify BLS signatures. It did the essential well, to help the node breathe and survive in Prater. But some obvious work can be done to improve the current impl.

Description

Extend usage

Use the BLS worker pool for all gossip validation, not only attestations.

Reduce latency cost

Communication between the main thread and workers has significant latency, similar to the cost of a single BLS sig verification. To reduce the impact of this latency in the throughput of validation I've update the pool to allow batching work. It will send packages of multiple groups of signature sets to be verified individually but sending the whole package once. This splits the latency cost between multiple operation. However this strategy only works if all workers are busy so the queue can accumulate work.

Add metrics

By using our own custom Thread Pool instead of threads implementation it's good to add some metrics to track:

  • Average signature verification time. Is blst as efficient as expected in production?
  • Job wait time. Does the queue structure delay signature verification significantly?
  • Job package size. Does the "reduce latency cost" strategy above work? Are work packages big or too small?

@codeclimate
Copy link

codeclimate bot commented Apr 25, 2021

Code Climate has analyzed commit 9baf22a and detected 0 issues on this pull request.

View more on Code Climate.

Copy link
Member

@wemeetagain wemeetagain left a comment

Choose a reason for hiding this comment

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

🚀

@wemeetagain wemeetagain merged commit 6d99105 into master Apr 26, 2021
@wemeetagain wemeetagain deleted the dapplion/improve-bls-verifier branch April 26, 2021 21:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants