rpcthreads was introduced with a default of 4 in 2013 in 21eb5adadbe3110a8708f2570185566e1f137a49
rpcworkqueue was introduced with a default of 16 in 2015 in 40b556d3742a1f65d67e2d4c760d0b13fe8be5b7
Resolves: #29386
Just bump the ancient default values. There is no perfect default that would fit everybody. This could lead to https://bikeshed.com/
`rpcthreads` was introduced with a default of 4 in 2013 in
21eb5adadbe3110a8708f2570185566e1f137a49
`rpcworkqueue` was introduced with a default of 16 in 2015 in
40b556d3742a1f65d67e2d4c760d0b13fe8be5b7
Resolves: https://github.com/bitcoin/bitcoin/issues/29386
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.
For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/31215.
See the guideline for information on the review process.
| Type | Reviewers |
|---|---|
| ACK | andrewtoth, storopoli |
| Concept ACK | laanwj |
| Approach ACK | tdb3 |
If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.
Reviewers, this pull request conflicts with the following ones:
If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.
13@@ -14,8 +14,17 @@ namespace util {
14 class SignalInterrupt;
15 } // namespace util
16
17-static const int DEFAULT_HTTP_THREADS=4;
18-static const int DEFAULT_HTTP_WORKQUEUE=16;
19+/**
20+ * The default value for `-rpcthreads`. This number of threads will be created at startup.
21+ */
22+static const int DEFAULT_HTTP_THREADS=16;
num cores / 2
i’ve always been kind of hestitant about adapting these values to the system automatically; it doesn’t only depend on the CPU, but also the expected usage.
For a “fair” work queue it would indeed make sense to pick the number of CPU cores. However, bitcoin core’s RPC threads often sleep for various reasons. Eg waiting for I/O, waiting for a lock in another thread, etc. So it’s fine to have it a bit higher than the number of cores. i’d definitely cap the minimum at 4, even for single-core CPUs.
i’m also not sure people running bitcoin core on say, giant 64+-core systems necessarily want 64+ RPC threads. They might be running it on the background as one of many processes, not intending to do a lot of parallel processing with bitcoin core at all. (there’s also a limited gain in having more than a certain number of threads because they’ll just end up getting in each other’s way with lock contention)
ACK on increasing the default work queue size. i don’t think the RPC work queue uses that much memory. It’s fine to set the default bound a lot higher.
Not sure about the threads.
Approach ACK
Performed a very basic sanity check (on signet) comparing 6463117a29294f6ddc9fafecfd1e9023956cc41b (commit under head) to head (e56fc7ce6a92eae7e80657d9f57a148cc002358d).
Ran bitcoin-cli gettxoutsetinfo 8x simultaneously on a system with 4 cores and timed the result (not claiming this to be the best test, just a quick check). The idea was to see if a core-constrained system would see a performance degradation from the PR change.
Without the increase of rpcthreads and rpcworkqueue, times of 57.63s, 59.75s, and 58.80s were observed. With the increase introduced in the PR, times of 55.41s, 53.08s, 56.91s were observed.
So no performance degradation, and a bit of a performance increase was observed.
Would like to take a look at some other results before full ACK, but seems like a reasonable change.
ACK e56fc7ce6a92eae7e80657d9f57a148cc002358d
Bumping the default threads and work queue enables a lot of workflows by default, like using it with a local address indexer. Right now these users have to manually configure the higher limits. The higher limits won’t increase memory unless they are being used. And these defaults can be set lower on memory constrained systems to disable using them.
The higher limits won’t increase memory unless they are being used. And these defaults can be set lower on memory constrained systems to disable using them.
Not entirely true, every thread has some memory and overhead, even if it’s not doing anything. On 32-bit systems the virtual memory space for the stack space of each thread is also significant.