-
Notifications
You must be signed in to change notification settings - Fork 2.7k
Batch signature verification #5023
Changes from 1 commit
a6a9e5c
fdc73e2
00cfe23
0d5f934
0b9d1d8
cd0b459
62d9dca
5ec6d6b
8fba16a
a25d32b
69d4e82
31b2942
3c54bc3
d8169a8
d01a788
044126a
e7bc120
8c25cf0
3ba39cf
4170f13
6f29854
1d4f716
2a358e3
20ae706
fd56193
d90cf62
b477ef9
1de6a2c
ded9f5c
1482b96
5eb30aa
46267fb
6b83175
f8e67ef
83b73f7
e16798e
0c54540
b1847a3
e439fed
ffe2250
5285c2b
e210b4b
0701c19
86bdcbe
1abe308
63f9df3
76a0689
4f835c6
4d55306
bbdec24
bfafa25
58e2cb1
65e86a8
70d3803
3eb55ff
87cf296
de66ee1
a593928
52e5ada
baf967b
4d5888e
394497d
d015c60
c2c9d2d
25c1fc1
6ae4dc5
b85e037
d68aebb
ab32c60
ba8ce75
0db24f4
b35c3aa
3d10f24
34499e4
d522248
0d9d09f
6c4f6b5
48fb7d6
7f3ebae
766ec69
49e9c67
a3979f2
b4cb58f
234ec99
1db62b0
f7097d6
e823c63
056515e
86d7efc
0e2b455
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
- Loading branch information
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -20,8 +20,6 @@ use sp_core::{ed25519, sr25519, crypto::Pair, traits::CloneableSpawn}; | |
| use std::sync::{Arc, atomic::{AtomicBool, Ordering as AtomicOrdering}}; | ||
| use futures::{future::FutureExt, task::FutureObj, channel::oneshot}; | ||
|
|
||
| const SR25519_BATCH_SIZE: usize = 64; | ||
|
|
||
| #[derive(Debug, Clone)] | ||
| struct Ed25519BatchItem { | ||
| signature: ed25519::Signature, | ||
|
|
@@ -91,22 +89,6 @@ impl BatchVerifier { | |
| self.spawn_verification_task(move || ed25519::Pair::verify(&signature, &message, &pub_key)); | ||
| } | ||
|
|
||
| fn spawn_sr25519_verification(&mut self) { | ||
| if self.sr25519_items.len() == 0 { | ||
| return; | ||
| } | ||
|
|
||
| let sr25519_batch = std::mem::replace(&mut self.sr25519_items, vec![]); | ||
|
|
||
| self.spawn_verification_task(move || { | ||
| let messages = sr25519_batch.iter().map(|item| &item.message[..]).collect(); | ||
| let signatures = sr25519_batch.iter().map(|item| &item.signature).collect(); | ||
| let pub_keys = sr25519_batch.iter().map(|item| &item.pub_key).collect(); | ||
|
|
||
| sr25519::verify_batch(messages, signatures, pub_keys) | ||
| }); | ||
| } | ||
|
|
||
| /// Push sr25519 signature to verify. | ||
| pub fn push_sr25519( | ||
| &mut self, | ||
|
|
@@ -115,35 +97,27 @@ impl BatchVerifier { | |
| message: Vec<u8>, | ||
| ) { | ||
| self.sr25519_items.push(Sr25519BatchItem { signature, pub_key, message }); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Seems rather doable to have a max batch size that trigger a spawn verification task every n sr25519 items.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. See benchmarks above, with 500 transactions in block this gives almost maximum performance gain (21%). And with less transactions in block we don't need this optimisation much.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What is the difference between batching and spawning a task per verification as for ed25519?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ~4% for more or less full block with batching using much less cpu |
||
|
|
||
| // if self.sr25519_items.len() >= SR25519_BATCH_SIZE { | ||
| // self.spawn_sr25519_verification(); | ||
| // } | ||
| } | ||
|
|
||
| /// Verify all previously pushed signatures since last call and return | ||
| /// aggregated result. | ||
| pub fn verify_and_clear(&mut self) -> bool { | ||
| use parking_lot::{Mutex, Condvar}; | ||
|
|
||
| // if self.sr25519_items.len() > 0 { | ||
| // self.spawn_sr25519_verification() | ||
| // } | ||
| let pending = std::mem::replace(&mut self.pending_tasks, vec![]); | ||
bkchr marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| // let pending = std::mem::replace(&mut self.pending_tasks, vec![]); | ||
| if pending.len() > 0 { | ||
| let pair = Arc::new((Mutex::new(()), Condvar::new())); | ||
| let pair_clone = pair.clone(); | ||
| self.scheduler.spawn_obj(FutureObj::new(async move { | ||
| futures::future::join_all(pending).await; | ||
| pair_clone.1.notify_one(); | ||
| }.boxed())).expect("Scheduler should not fail"); | ||
|
|
||
| // if pending.len() > 0 { | ||
| // let pair = Arc::new((Mutex::new(()), Condvar::new())); | ||
| // let pair_clone = pair.clone(); | ||
| // self.scheduler.spawn_obj(FutureObj::new(async move { | ||
| // futures::future::join_all(pending).await; | ||
| // pair_clone.1.notify_one(); | ||
| // }.boxed())).expect("Scheduler should not fail"); | ||
|
|
||
| // let (mtx, cond_var) = &*pair; | ||
| // let mut mtx = mtx.lock(); | ||
| // cond_var.wait(&mut mtx); | ||
| // } | ||
| let (mtx, cond_var) = &*pair; | ||
| let mut mtx = mtx.lock(); | ||
| cond_var.wait(&mut mtx); | ||
| } | ||
|
|
||
| let messages = self.sr25519_items.iter().map(|item| &item.message[..]).collect(); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. should we start the sr batch before waiting for all ed completion? that way ed can run in background a bit longer.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Great catch, yep! Mixed is ed25519 and sr25519 is not benchmarked though |
||
| let signatures = self.sr25519_items.iter().map(|item| &item.signature).collect(); | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.