Server-wide ThreadPool#300
Conversation
This test will be useful for verifying that the ThreadPool threads, can handle other requests while waiting for callback to return. Also use the opportunity to refactor some of the tests to remove the try-catch blocks.
|
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. ReviewsSee the guideline for information on the review process. ConflictsReviewers, 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. |
ProxyServer<Thread> previously bundled two concerns: the Cap'n Proto server capability (getName, reference-counted lifetime) and core thread management (OS thread, Waiter, m_thread_ready promise chain, post<T>()). Extract the thread management concern into a new WorkerThread struct. ProxyServer<Thread> becomes a thin wrapper that owns a WorkerThread and adds the capability layer on top. This deduplication prepares for the ThreadPool implementation where WorkerThread objects are owned directly without Cap'n Proto capability overhead. Key design choices: - WorkerThread uses EventLoop& not EventLoopRef: lifetime is bounded by owner, which always holds the appropriate reference to keep the loop alive. - WorkerThread::post<T>(fn, on_done) uses a template OnDone parameter (defaulting to nullptr_t with if constexpr guard) to avoid requiring copy-constructibility, allowing callers to capture move-only values. - ProxyServer<Thread> gains a second constructor wrapping an existing thread (ThreadContext&) for use as a callback-thread capability. - makeThread() simplified: thread creation now inside WorkerThread ctor.
Add ThreadPool class holding a fixed number of WorkerThreads dispatched round-robin. Wire it into EventLoop via a new num_pool_threads constructor parameter (default 0, no pool). When num_pool_threads > 0, m_thread_pool is created after m_task_set so WorkerThread destructors (which join their threads) run before the task set is destroyed. Pool threads are named '<exe>-N (pool I)' and are not Cap'n Proto capabilities - clients never hold Thread::Client references to them. PassField dispatch integration follows in the next commit.
When a Context argument has no thread set, dispatch the request to the EventLoop's thread pool (if configured) instead of failing. Also adds a num_pool_threads parameter to TestSetup and a test verifying pool lifecycle and direct work dispatch.
|
Thanks for the PR! Seems like there are a lot of nice things here: New recursive async test looks worthwhile (though I think it might be possible to simplify by using passFn/std::function instead of callback/Callback). Test cleanups and WorkerThread refactor also seem helpful and make code more readable. Things I'd change:
Overall, I think the solution for providing more automatic threading will combine different aspects of all this approach, bd1f80c, and #283. I'm planning on merging #283 first in its current form (and bumping the libmultiprocess subtree in bitcoin core) and this PR could be a good followup to that. |
3 participants
This PR adds a
ThreadPoolowned by theEventLoop, enabling any server connection to dispatch work to a shared pool. TheThreadPoolis tested with callbacks and recursive IPC calls.Motivation
makePoolmethod onThreadMap#283).Difference from #283
Unlike the approach in #283, external clients don't need to send a pool creation request — the pool is available server-wide from the start. This lays groundwork for a future where
ThreadMapaccess is restricted to privileged connections only.Draft while I finish testing and gauge interest in a server-wide
ThreadPool.