-
Notifications
You must be signed in to change notification settings - Fork 4k
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
removes lazy-static thread-pool from sigverify-shreds #30787
removes lazy-static thread-pool from sigverify-shreds #30787
Conversation
Instead the thread-pool is passed explicitly from higher in the call stack so that solana-labs#30786 can use the same thread-pool for shred deduplication.
44b3dba
to
ed46faa
Compare
Codecov Report
@@ Coverage Diff @@
## master #30787 +/- ##
=========================================
- Coverage 81.4% 81.3% -0.1%
=========================================
Files 724 724
Lines 203081 203098 +17
=========================================
- Hits 165459 165319 -140
- Misses 37622 37779 +157 |
@@ -121,7 +136,7 @@ fn verify_packets( | |||
.filter_map(|(slot, pubkey)| Some((slot, pubkey?.to_bytes()))) | |||
.chain(std::iter::once((Slot::MAX, [0u8; 32]))) | |||
.collect(); | |||
let out = verify_shreds_gpu(packets, &leader_slots, recycler_cache); | |||
let out = verify_shreds_gpu(thread_pool, packets, &leader_slots, recycler_cache); |
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.
Passing CPU thread pool into verify_shreds_gpu
.. barf.
By the way, not blaming you for the precedent that was already here.
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.
at least the grossness is out in the open now, rather than hidden away tidily behind that global 😉
@@ -284,7 +299,9 @@ mod tests { | |||
batches[0][1].buffer_mut()[..shred.payload().len()].copy_from_slice(shred.payload()); | |||
batches[0][1].meta_mut().size = shred.payload().len(); | |||
|
|||
let thread_pool = ThreadPoolBuilder::new().num_threads(3).build().unwrap(); |
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.
why 3?
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.
It is a test; so I just picked some small number.
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.
@@ -121,7 +136,7 @@ fn verify_packets( | |||
.filter_map(|(slot, pubkey)| Some((slot, pubkey?.to_bytes()))) | |||
.chain(std::iter::once((Slot::MAX, [0u8; 32]))) | |||
.collect(); | |||
let out = verify_shreds_gpu(packets, &leader_slots, recycler_cache); | |||
let out = verify_shreds_gpu(thread_pool, packets, &leader_slots, recycler_cache); |
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.
at least the grossness is out in the open now, rather than hidden away tidily behind that global 😉
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
…t of #30787) (#30827) * removes lazy-static thread-pool from sigverify-shreds (#30787) Instead the thread-pool is passed explicitly from higher in the call stack so that #30786 can use the same thread-pool for shred deduplication. (cherry picked from commit c6e7aaf) # Conflicts: # core/src/sigverify_shreds.rs # ledger/src/sigverify_shreds.rs * resolves mergify merge conflicts --------- Co-authored-by: behzad nouri <behzadnouri@gmail.com>
…t of solana-labs#30787) removes lazy-static thread-pool from sigverify-shreds solana-labs#30787 Instead the thread-pool is passed explicitly from higher in the call stack so that solana-labs#30786 can use the same thread-pool for shred deduplication. (cherry picked from commit c6e7aaf)
…t of solana-labs#30787) removes lazy-static thread-pool from sigverify-shreds solana-labs#30787 Instead the thread-pool is passed explicitly from higher in the call stack so that solana-labs#30786 can use the same thread-pool for shred deduplication. (cherry picked from commit c6e7aaf)
…t of solana-labs#30787) removes lazy-static thread-pool from sigverify-shreds solana-labs#30787 Instead the thread-pool is passed explicitly from higher in the call stack so that solana-labs#30786 can use the same thread-pool for shred deduplication. (cherry picked from commit c6e7aaf)
Problem
The lazy-static thread-pool is not accessible from outside the
ledger/src/sigverfiy_shreds.rs
file.Summary of Changes
The commit passes the thread-pool explicitly from higher in the call stack so that #30786 can use the same thread-pool for shred deduplication.