Avoid absl::BlockingCounter use for the single-threaded WorkerPool #8852
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
b84b6ba to
552168b
Compare
552168b to
23ae71f
Compare
jez
approved these changes
May 8, 2025
| @@ -14,6 +14,11 @@ class WorkerPool { | |||
| using Task = std::function<void()>; | |||
| static std::unique_ptr<WorkerPool> create(int size, spdlog::logger &logger); | |||
| virtual void multiplexJob(std::string_view taskName, Task t) = 0; | |||
There was a problem hiding this comment.
Can we document this one too, something like
"does not block after launching the workers, usually useful so that the main thread can 8000 have a consumer queue that streams the results of the worker threads as they produce them"
| @@ -26,6 +26,7 @@ cc_library( | |||
| "//common/enforce_no_timer", | |||
| "//common/exception", | |||
| "//common/os", | |||
| "@com_google_absl//absl/synchronization", | |||
There was a problem hiding this comment.
I don't know whether it's worth it to also remove the absl/synch dep from common/BUILD (or even whether it's possible).
There was a problem hiding this comment.
I'll give it a try!
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
In some uses of
WorkerPool::multiplexJobwe allocate anabsl::BlockingCounterand wait on it after the call tomultiplexJobto ensure that all workers finished processing our job. This is a fine pattern, but in the case of a worker pool created with0workers, the blocking counter is totally unneeded asmultiplexJobwill execute our task directly.We can avoid the
absl::BlockingCounteruse entirely if we add a specialization ofmultiplexJobfor this case specifically, and that's what this PR does.WorkerPool::multiplexJobWaitmanages the allocation and use of the counter in the multi-threaded case, and avoids it all when we're running single-threaded.Motivation
Performance improvements when we're running with an empty worker pool, which happens frequently during slow path preemption in LSP.
Test plan
Existing tests.