rayon-threadlimit: Deprecate get_max_thread_count()#7071
rayon-threadlimit: Deprecate get_max_thread_count()#7071steviez merged 2 commits intoanza-xyz:masterfrom
Conversation
8727438 to
1b426b7
Compare
192a1f1 to
3e93022
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #7071 +/- ##
=========================================
- Coverage 83.2% 83.2% -0.1%
=========================================
Files 854 854
Lines 374653 374660 +7
=========================================
+ Hits 312058 312061 +3
- Misses 62595 62599 +4 🚀 New features to boost your workflow:
|
bw-solana
left a comment
There was a problem hiding this comment.
My only concern is that we might silently rug someone that is using the SOLANA_RAYON_THREADS env var.
I'm wondering if we can detect when that is set while initializing MAX_RAYON_THREADS and print a warn or something
Oops, realized I never responded to this piece. |
This function provided a hack to change threadpool sizes to avoid observed performance issues. The users of this function have since been updated to have CLI args wired up to control the threadpool sizes. So, the hack is no longer necessary and can be deprecated
212e6f3 to
c678eef
Compare
This function provided a hack to change threadpool sizes to avoid observed performance issues. The users of this function have since been updated to have CLI args wired up to control the threadpool sizes. So, the hack is no longer necessary and can be deprecated
Problem
See #105 for more context, but the TLDR is that
solana-rayon-threadlimitwas a workaround for a time when we did't have finer controls over individual threadpool sizes. CLI args have been wired up in many locations so there is no need for the environment variable hack thatsolana-rayon-threadlimitprovides.Summary of Changes
Deprecate
get_max_thread_count()and remove all usage from our codebase.Some subsequent PR's will get us towards deprecating
get_thread_count()and thus the entiresolana-rayon-threadlimitcrate. I could also see us trying to limit callers ofnum_cpus::get()in the future to a few specific places, but small steps for now 😄