[EventEngine] Respect requested thread pool size (grpc#34904)

The current fixed minimum starting thread count (2 threads), combined with the fixed thread spawn rate limit of 1/sec, was causing test issues with some new EventEngine integrations. For example, this test had a race wherein the alarm destructor was expected to run within 1 second. In 5% of runs, the 2 EventEngine threads were otherwise occupied, and it would take around 1 second to spawn a new thread. https://github.com/grpc/grpc/blob/1e15d00ec4d6eba6bd20621fc72ad51d0f62b654/test/cpp/common/alarm_test.cc#L418-L435
emrahayanoglu · Nov 9, 2023 · 920882f · 920882f
1 parent 3964acc
commit 920882f
Show file tree

Hide file tree

Showing 4 changed files with 5 additions and 10 deletions.
diff --git a/src/core/BUILD b/src/core/BUILD
@@ -1615,7 +1615,6 @@ grpc_cc_library(
         "no_destruct",
         "notification",
         "time",
-        "useful",
         "//:backoff",
         "//:event_engine_base_hdrs",
         "//:gpr",

diff --git a/src/core/lib/event_engine/posix_engine/posix_engine.cc b/src/core/lib/event_engine/posix_engine/posix_engine.cc
@@ -361,7 +361,7 @@ PosixEnginePollerManager::~PosixEnginePollerManager() {
 
 PosixEventEngine::PosixEventEngine(std::shared_ptr<PosixEventPoller> poller)
     : connection_shards_(std::max(2 * gpr_cpu_num_cores(), 1u)),
-      executor_(MakeThreadPool(grpc_core::Clamp(gpr_cpu_num_cores(), 2u, 16u))),
+      executor_(MakeThreadPool(grpc_core::Clamp(gpr_cpu_num_cores(), 4u, 16u))),
       timer_manager_(std::make_shared<TimerManager>(executor_)) {
   g_timer_fork_manager->RegisterForkable(
       timer_manager_, TimerForkCallbackMethods::Prefork,
@@ -374,7 +374,7 @@ PosixEventEngine::PosixEventEngine(std::shared_ptr<PosixEventPoller> poller)
 
 PosixEventEngine::PosixEventEngine()
     : connection_shards_(std::max(2 * gpr_cpu_num_cores(), 1u)),
-      executor_(MakeThreadPool(grpc_core::Clamp(gpr_cpu_num_cores(), 2u, 16u))),
+      executor_(MakeThreadPool(grpc_core::Clamp(gpr_cpu_num_cores(), 4u, 16u))),
       timer_manager_(std::make_shared<TimerManager>(executor_)) {
   g_timer_fork_manager->RegisterForkable(
       timer_manager_, TimerForkCallbackMethods::Prefork,

diff --git a/src/core/lib/event_engine/thread_pool/thread_pool_factory.cc b/src/core/lib/event_engine/thread_pool/thread_pool_factory.cc
@@ -17,12 +17,9 @@
 
 #include <memory>
 
-#include <grpc/support/cpu.h>
-
 #include "src/core/lib/event_engine/forkable.h"
 #include "src/core/lib/event_engine/thread_pool/thread_pool.h"
 #include "src/core/lib/event_engine/thread_pool/work_stealing_thread_pool.h"
-#include "src/core/lib/gpr/useful.h"
 #include "src/core/lib/gprpp/no_destruct.h"
 
 namespace grpc_event_engine {
@@ -39,9 +36,8 @@ class ThreadPoolForkCallbackMethods {
 };
 }  // namespace
 
-std::shared_ptr<ThreadPool> MakeThreadPool(size_t /* reserve_threads */) {
-  auto thread_pool = std::make_shared<WorkStealingThreadPool>(
-      grpc_core::Clamp(gpr_cpu_num_cores(), 2u, 16u));
+std::shared_ptr<ThreadPool> MakeThreadPool(size_t reserve_threads) {
+  auto thread_pool = std::make_shared<WorkStealingThreadPool>(reserve_threads);
   g_thread_pool_fork_manager->RegisterForkable(
       thread_pool, ThreadPoolForkCallbackMethods::Prefork,
       ThreadPoolForkCallbackMethods::PostforkParent,

diff --git a/src/core/lib/event_engine/windows/windows_engine.cc b/src/core/lib/event_engine/windows/windows_engine.cc
@@ -99,7 +99,7 @@ struct WindowsEventEngine::TimerClosure final : public EventEngine::Closure {
 
 WindowsEventEngine::WindowsEventEngine()
     : thread_pool_(
-          MakeThreadPool(grpc_core::Clamp(gpr_cpu_num_cores(), 2u, 16u))),
+          MakeThreadPool(grpc_core::Clamp(gpr_cpu_num_cores(), 4u, 16u))),
       iocp_(thread_pool_.get()),
       timer_manager_(thread_pool_),
       iocp_worker_(thread_pool_.get(), &iocp_) {