Parallelize accountant #106
Conversation
Before this change, parallel transaction processing required locking the full accountant. Since we only call one method, process_verified_transaction, the global lock equates to doing no parallelization at all. With this change, we only lock the data that's being written to.
|
So technically, transactions posted by the leader don't need to commute, but we'd need to lock the historian before applying any changes, and if we did that in the current code, you'd lose all parallelization. |
Codecov Report
@@ Coverage Diff @@
## master #106 +/- ##
==========================================
+ Coverage 95.69% 95.83% +0.13%
==========================================
Files 16 16
Lines 1324 1344 +20
==========================================
+ Hits 1267 1288 +21
+ Misses 57 56 -1
Continue to review full report at Codecov.
|
src/accountant.rs
Outdated
|
|
||
| if let Some(ref payment) = plan.final_payment() { | ||
| apply_payment(&mut self.balances, payment); | ||
| apply_payment(&mut self.balances.write().unwrap(), payment); |
There was a problem hiding this comment.
will it try to get write lock for balances twice?
There was a problem hiding this comment.
Yes, in the case the transaction requires no witnesses, it'll lock balances for the from cell and again for the to cell. Once we switch to per cell locks, it'll be two separate locks anyway.
There was a problem hiding this comment.
seems like it could be problem:
https://doc.rust-lang.org/std/sync/struct.RwLock.html#panics-1
There was a problem hiding this comment.
No, a subtlety of Rust, by not putting balances into variable, it's only in scope while the righthand side of that expression is evaluated. If I put the first instance into a variable, then the second would immediately cause a panic.
There was a problem hiding this comment.
Ok cool. yea it's not super clear.
This allows us to use read-locks for balances most of the time. We only lock the full table if we need to add one.
This will allow for additional concurrency as well as give the server a means of garbage-collecting old signatures.
|
With per-cell locking and sorting signatures by last_id, the benchmark is now processing 4k transactions in just under 5ms |
…bs#106) Bumps [webpack-dev-server](https://github.com/webpack/webpack-dev-server) from 3.10.3 to 3.11.0. - [Release notes](https://github.com/webpack/webpack-dev-server/releases) - [Changelog](https://github.com/webpack/webpack-dev-server/blob/master/CHANGELOG.md) - [Commits](webpack/webpack-dev-server@v3.10.3...v3.11.0) Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
… Public IP (solana-labs#106) Co-authored-by: Eric Semeniuc <[email protected]>
This is not yet integrated into AccountantSkel, so don't expect the top-level demo benchmark to change, but includes a microbenchmark that demonstrates that performance doubles compared to the single-threaded version.
Also, before adding to AccountantSkel, we need to ensure all transactions commute.