threadpool: add ranged Submit overload by andrewtoth · Pull Request #34576 · bitcoin/bitcoin

andrewtoth · 2026-02-12T17:39:40Z

The current ThreadPool::Submit is not very efficient when we have a use case where we need to submit multiple tasks immediately. The Submit method must take the lock for each task, and notifies only a single worker thread. This will cause lock contention with the awakened worker thread trying to take the lock and the caller trying to submit the next task.

Introduce a Submit overload, which takes the lock once and submits a range of tasks, then notifies all worker threads after the lock is released.

This is needed for #31132 to be able to use ThreadPool.

DrahtBot · 2026-02-12T17:40:09Z

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Reviews

See the guideline for information on the review process.

Type	Reviewers
ACK	l0rinc, sedited, willcl-ark, rkrux
Stale ACK	sipa, furszy

If your review is incorrectly listed, please copy-paste  into the comment that the bot should ignore.

Conflicts

Reviewers, this pull request conflicts with the following ones:

#31132 (validation: fetch block inputs on parallel threads 3x faster IBD by andrewtoth)

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.

furszy

Concept ACK

src/util/threadpool.h

furszy · 2026-02-19T19:11:20Z

src/util/threadpool.h

+     *          uncaught exceptions, as they would otherwise be silently discarded.
+     */
+    template <class F>
+    [[nodiscard]] util::Expected<std::vector<std::future<std::invoke_result_t<F>>>, SubmitError> SubmitMany(std::vector<F>&& fns) noexcept EXCLUSIVE_LOCKS_REQUIRED(!m_mutex)


General question to think about (no need to answer here):

Is there any use-case for submitting callables with different return types? Because if there is, then we are forced to use a variadic-based approach.

Can we add a new overload if the need arises?

src/util/threadpool.h

andrewtoth · 2026-02-20T03:33:44Z

Thanks @furszy and @willcl-ark. Taken your suggestions and rebased.

furszy

ACK 3295552

sipa

Code review ACK 3295552

src/test/threadpool_tests.cpp

andrewtoth · 2026-02-28T00:19:56Z

Rebased due to #34562. Addressed nit (#34576 (comment)). Reworded commit message. Only test changes since last push.

git range-diff 739f75c0980ef30f676183323f4cb41fb9272b71..3295552c0a895fa650c0e0c67600c384fdbcde50 9cad97f6cdf1bd660732cd10e844a6a7e0771ea0..43c5c77fcb6f68fee5249c3b8666cd86dd1c9038

src/test/threadpool_tests.cpp

furszy

ACK fdddacc

rkrux

Couple points in the test.

src/test/threadpool_tests.cpp

andrewtoth · 2026-03-07T15:12:12Z

Thanks for the review @rkrux. I took your suggestions. Test only changes style changes.

git diff fdddaccc8c9cbd241ad44100e68d49e7bebd0bc4..2852489f4878d3de0e72de77367fae845543ec4c

furszy

reACK 2852489

rkrux

lgtm ACK 2852489

Thanks for accepting the suggestions.

sedited

ACK 2852489

Can you fix the include order and some of the formatting issues as reported by clang-format-diff?

I also fuzzed this a bit with the following patch, but quickly stoppped finding new coverage:

Click to expand diff

diff --git a/src/test/fuzz/threadpool.cpp b/src/test/fuzz/threadpool.cpp
index a5b01db138..59e2f1b7a8 100644
--- a/src/test/fuzz/threadpool.cpp
+++ b/src/test/fuzz/threadpool.cpp
@@ -72 +72 @@ FUZZ_TARGET(threadpool, .init = setup_threadpool_test) EXCLUSIVE_LOCKS_REQUIRED(
-    const uint32_t num_tasks = fuzzed_data_provider.ConsumeIntegralInRange<uint32_t>(0, 1024);
+    uint32_t num_tasks = fuzzed_data_provider.ConsumeIntegralInRange<uint32_t>(0, 1024);
@@ -85,0 +86,25 @@ FUZZ_TARGET(threadpool, .init = setup_threadpool_test) EXCLUSIVE_LOCKS_REQUIRED(
+        const bool submit_batch = fuzzed_data_provider.ConsumeBool();
+
+        if (submit_batch) {
+            const uint32_t batch_size = fuzzed_data_provider.ConsumeIntegralInRange<uint32_t>(0, 100);
+            i += batch_size;
+            if (will_throw) {
+                expected_fail_tasks += batch_size;
+                auto batch_futures = *Assert(g_pool.Submit(std::vector<ThrowTask>(batch_size)));
+                for (auto& batch_future : batch_futures) {
+                    futures.emplace(std::move(batch_future));
+                }
+            } else {
+                std::vector<CounterTask> tasks;
+                tasks.reserve(batch_size);
+                for (uint32_t j = 0; j < batch_size; ++j) {
+                    expected_task_counter++;
+                    tasks.emplace_back(task_counter);
+                }
+                auto batch_futures = *Assert(g_pool.Submit(std::move(tasks)));
+                for (auto& batch_future : batch_futures) {
+                    futures.emplace(std::move(batch_future));
+                }
+            }
+            continue;
+        }

andrewtoth-exo · 2026-03-10T01:00:45Z

Thanks for the review and for fuzzing @sedited. I pushed the formatting changes.

git diff 2852489f4878d3de0e72de77367fae845543ec4c..e62bfc241e90b60424365924f676ea85755ae935

src/test/threadpool_tests.cpp

sedited

Re-ACK e62bfc2

rkrux

reACK e62bfc2

git range-diff 2852489...e62bfc2

furszy

utACK e62bfc2

l0rinc

The current version is probably fine, but it seems to me we can make it more accurate by using range reference type instead of the value type - and iterating with auto&& so the overload handles more callable/range shapes correctly. And maybe handle empty with a lockless short-circuit.

l0rinc · 2026-03-10T14:24:11Z

src/util/threadpool.h

+            LOCK(m_mutex);
+            if (m_workers.empty()) return util::Unexpected{SubmitError::Inactive};
+            if (m_interrupt) return util::Unexpected{SubmitError::Interrupted};
+            for (auto& fn : fns) {


e62bfc2 threadpool: add ranged Submit overload:

What should the intended behavior be for an empty fns range?
It might be worth making that explicit and short-circuiting the locking, and maybe covering it with a small test.

I initially did short-circuit it, but it was brought up that it is a behavior change from non-ranged Submit #34576 (comment). I think best to keep behavior identical, so we throw even if fns is empty.

src/util/threadpool.h

Co-authored-by: l0rinc <pap.lorinc@gmail.com>

andrewtoth · 2026-03-10T20:28:14Z

Thanks for your review @l0rinc. I have taken most of your suggestions.

fixed style nit in test
changed lvalue to rvalue ref in fns loop
added PackagedTask alias
updated RangeValueFuture with range_value_t to RangeFuture with range_reference_t
fixed style nit in docstring

git diff e62bfc241e90b60424365924f676ea85755ae935..79571b918130e66436b2d43489835c38bb3ae3e3

l0rinc · 2026-03-10T23:00:39Z

ACK 79571b9

sedited

Re-ACK 79571b9

willcl-ark

ACK 79571b9

I like the template constraints via requires(), very neat :)

I did not test with e.g. 31132 picked, but the code and tests here look good to me.

rkrux · 2026-03-11T12:06:45Z

ACK 79571b9

git range-diff e62bfc2...79571b9

furszy reviewed Feb 12, 2026

View reviewed changes

willcl-ark reviewed Feb 17, 2026

View reviewed changes

src/util/threadpool.h Outdated Show resolved Hide resolved

andrewtoth mentioned this pull request Feb 18, 2026

validation: fetch block inputs on parallel threads 3x faster IBD #31132

Open

3 tasks

andrewtoth force-pushed the threadpool_submitmany branch from af11966 to c985542 Compare February 19, 2026 18:41

furszy reviewed Feb 19, 2026

View reviewed changes

andrewtoth force-pushed the threadpool_submitmany branch 2 times, most recently from cdecfa7 to 1593245 Compare February 20, 2026 03:32

DrahtBot added the CI failed label Feb 20, 2026

andrewtoth force-pushed the threadpool_submitmany branch from 1593245 to 3295552 Compare February 20, 2026 03:37

furszy reviewed Feb 20, 2026

View reviewed changes

DrahtBot mentioned this pull request Feb 20, 2026

ThreadPool follow-ups, proactive shutdown and HasReason dependency cleanup #34562

Merged

DrahtBot removed the CI failed label Feb 20, 2026

sipa reviewed Feb 20, 2026

View reviewed changes

src/test/threadpool_tests.cpp Outdated Show resolved Hide resolved

andrewtoth changed the title ~~threadpool: add SubmitMany~~ threadpool: add ranged Submit overload Feb 22, 2026

DrahtBot added the Needs rebase label Feb 26, 2026

andrewtoth force-pushed the threadpool_submitmany branch from 3295552 to 43c5c77 Compare February 28, 2026 00:17

DrahtBot added CI failed and removed Needs rebase labels Feb 28, 2026

furszy reviewed Feb 28, 2026

View reviewed changes

src/test/threadpool_tests.cpp Outdated Show resolved Hide resolved

andrewtoth force-pushed the threadpool_submitmany branch from 43c5c77 to fdddacc Compare February 28, 2026 15:51

furszy reviewed Feb 28, 2026

View reviewed changes

DrahtBot requested a review from sipa February 28, 2026 16:00

DrahtBot removed the CI failed label Feb 28, 2026

rkrux reviewed Mar 5, 2026

View reviewed changes

src/test/threadpool_tests.cpp Outdated Show resolved Hide resolved

src/test/threadpool_tests.cpp Outdated Show resolved Hide resolved

andrewtoth force-pushed the threadpool_submitmany branch from fdddacc to 2852489 Compare March 7, 2026 15:10

furszy reviewed Mar 7, 2026

View reviewed changes

rkrux reviewed Mar 9, 2026

View reviewed changes

sedited approved these changes Mar 9, 2026

View reviewed changes

andrewtoth force-pushed the threadpool_submitmany branch from 2852489 to e62bfc2 Compare March 10, 2026 00:59

sedited reviewed Mar 10, 2026

View reviewed changes

src/test/threadpool_tests.cpp Outdated Show resolved Hide resolved

sedited approved these changes Mar 10, 2026

View reviewed changes

DrahtBot requested review from furszy and rkrux March 10, 2026 07:40

rkrux approved these changes Mar 10, 2026

View reviewed changes

furszy reviewed Mar 10, 2026

View reviewed changes

fanquake added this to the 32.0 milestone Mar 10, 2026

l0rinc suggested changes Mar 10, 2026

View reviewed changes

threadpool: add ranged Submit overload

79571b9

Co-authored-by: l0rinc <pap.lorinc@gmail.com>

andrewtoth force-pushed the threadpool_submitmany branch from e62bfc2 to 79571b9 Compare March 10, 2026 20:23

sedited approved these changes Mar 11, 2026

View reviewed changes

DrahtBot requested review from furszy and rkrux March 11, 2026 11:19

willcl-ark approved these changes Mar 11, 2026

View reviewed changes

l0rinc approved these changes Mar 11, 2026

View reviewed changes

sedited merged commit 524aa1e into bitcoin:master Mar 11, 2026
26 checks passed

Conversation

andrewtoth commented Feb 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

DrahtBot commented Feb 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Reviews

Conflicts

Uh oh!

furszy left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

furszy Feb 19, 2026

Choose a reason for hiding this comment

Uh oh!

andrewtoth Feb 20, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

andrewtoth commented Feb 20, 2026

Uh oh!

furszy left a comment

Choose a reason for hiding this comment

Uh oh!

sipa left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

andrewtoth commented Feb 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

furszy left a comment

Choose a reason for hiding this comment

Uh oh!

rkrux left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

andrewtoth commented Mar 7, 2026

Uh oh!

furszy left a comment

Choose a reason for hiding this comment

Uh oh!

rkrux left a comment

Choose a reason for hiding this comment

Uh oh!

sedited left a comment

Choose a reason for hiding this comment

Uh oh!

andrewtoth-exo commented Mar 10, 2026

Uh oh!

Uh oh!

sedited left a comment

Choose a reason for hiding this comment

Uh oh!

rkrux left a comment

Choose a reason for hiding this comment

Uh oh!

furszy left a comment

Choose a reason for hiding this comment

Uh oh!

l0rinc left a comment

Choose a reason for hiding this comment

Uh oh!

l0rinc Mar 10, 2026

Choose a reason for hiding this comment

Uh oh!

andrewtoth Mar 10, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

andrewtoth commented Mar 10, 2026

andrewtoth commented Feb 12, 2026 •

edited

Loading

DrahtBot commented Feb 12, 2026 •

edited

Loading

andrewtoth commented Feb 28, 2026 •

edited

Loading