-
Notifications
You must be signed in to change notification settings - Fork 2.6k
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
[Optimize] Adjust blocking tasks in the primary #3121
Conversation
@@ -630,7 +630,7 @@ impl<N: Network> Primary<N> { | |||
let BatchSignature { batch_id, signature } = batch_signature; | |||
|
|||
// Retrieve the signer. | |||
let signer = spawn_blocking!(Ok(signature.to_address()))?; | |||
let signer = signature.to_address(); |
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.
note: this operation was found to be fast enough to not require blocking
32f3774
to
93768f6
Compare
@@ -1114,7 +1119,7 @@ impl<N: Network> Primary<N> { | |||
/// Stores the certified batch and broadcasts it to all validators, returning the certificate. | |||
async fn store_and_broadcast_certificate(&self, proposal: &Proposal<N>, committee: &Committee<N>) -> Result<()> { | |||
// Create the batch certificate and transmissions. | |||
let (certificate, transmissions) = proposal.to_certificate(committee)?; | |||
let (certificate, transmissions) = tokio::task::block_in_place(|| proposal.to_certificate(committee))?; |
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.
note: this is done via block_in_place
instead of spawn_blocking
in order to not have to clone the Proposal
, which could get expensive enough to defeat the purpose
Signed-off-by: ljedrz <ljedrz@gmail.com>
93768f6
to
86acd1d
Compare
@@ -1840,7 +1845,7 @@ mod tests { | |||
); | |||
} | |||
|
|||
#[tokio::test] | |||
#[tokio::test(flavor = "multi_thread")] |
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.
note: this is a requirement for using block_in_place
These changes were tested using |
This PR adjusts the placement of blocking tasks in the
primary
module based on how long the blocking operations can take, so that we avoid blocking the async executor.