-
Notifications
You must be signed in to change notification settings - Fork 248
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
Parallel chiapos #1453
Parallel chiapos #1453
Conversation
@@ -23,14 +25,29 @@ fn pos_bench<PosTable>( | |||
) where | |||
PosTable: Table, | |||
{ | |||
ThreadPoolBuilder::new() |
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.
I think you're missing a #[cfg(feature = "parallel")]
here?
It fails to compile subspace-proof-of-space
separately:
error[E0433]: failed to resolve: use of undeclared type `ThreadPoolBuilder`
--> crates/subspace-proof-of-space/benches/pos.rs:28:5
|
28 | ThreadPoolBuilder::new()
| ^^^^^^^^^^^^^^^^^ use of undeclared type `ThreadPoolBuilder`
For more information about this error, try `rustc --explain E0433`.
error: could not compile `subspace-proof-of-space` (bench "pos") due to previous error
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.
And if I bench anyway (cargo bench --features chia --bench pos
) it panics
thread 'main' panicked at 'called `Result::unwrap()` on an `Err` value: ThreadPoolBuildError { kind: GlobalPoolAlreadyInitialized }', crates/subspace-proof-of-space/benches/pos.rs:32:10
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.
Fixed, though I'm not even sure if it should be limited to 4 cores by default (I wanted to have more representative results, using all cores would be faster).
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.
This is so cool! Both proving from memory and disk improved drastically for me.
proving/disk time: [5.5822 s 5.8284 s 6.0547 s]
thrpt: [1.6516 elem/s 1.7157 elem/s 1.7914 elem/s]
proving/memory time: [197.67 ms 199.90 ms 203.87 ms]
thrpt: [4.9051 elem/s 5.0025 elem/s 5.0590 elem/s]
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.
LGTM thanks
This PR introduces parallelism in chiapos that further massively reduces table construction time.
Proving before:
Proving after:
Note that this is less efficient than single-threaded version in terms of CPU utilization and memory usage, we only use it for proving where latency is critical.
This has a breaking change for the farmer since order of qualities/proofs wasn't canonical before, now it is and can be different from what was expected in the past (and no migration code was written for this). As far as consensus verification is concerned, nothing has changed.
Code contributor checklist: