fix hill-climb monitor expected_count after shrink

After a successful _remove_thread_scaling_locked, the monitor set
expected_count via thread_count(), which returns the pre-shrink
count because the removal is deferred (only _remove_count is
incremented; the actual _threads erase happens asynchronously when
a worker claims the removal).  This caused two problems:

1. On the next cycle, the deferred removal makes thread_count()
   return count-1, which differs from expected_count (count),
   triggering the external-scaling-interference detector and
   wasting the cycle.

2. If the worker hasn't exited yet, thread_count() still equals
   expected_count, so the monitor evaluates throughput with the
   pre-shrink thread count still in effect, potentially reversing
   direction before the shrink's effect is measurable.

Fix: compute expected_count arithmetically (count-1 for shrink,
count+1 for grow) using the authoritative pre-operation count
read under _scaling_mutex, reflecting the intended outcome rather
than the racy current state.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
Antonio S. Musumeci 2026-04-10 22:21:20 -04:00
parent 5f50f76b66
commit 26c10d8a2e

View file

@ -718,7 +718,13 @@ ThreadPool::monitor_routine(void *arg_)
btp->_last_scale_time_usecs.store(now,
std::memory_order_relaxed);
acted_last_cycle = true;
expected_count = btp->thread_count();
// Use count + 1 rather than thread_count() so the
// expected value reflects the intended outcome of our
// action, not the racy current state. For grow this
// is equivalent (push_back is synchronous) but for
// consistency and to avoid an extra lock acquisition,
// compute it arithmetically.
expected_count = count + 1;
syslog(LOG_DEBUG,
"threadpool (%s): hill-climb grow to %zu threads "
"(ema %.1f -> %.1f)",
@ -735,7 +741,13 @@ ThreadPool::monitor_routine(void *arg_)
btp->_last_scale_time_usecs.store(now,
std::memory_order_relaxed);
acted_last_cycle = true;
expected_count = btp->thread_count();
// _remove_thread_scaling_locked only increments
// _remove_count and enqueues a wakeup pill — it does
// not synchronously erase from _threads. Using
// thread_count() here would return the pre-shrink
// count, causing the next cycle to misattribute the
// deferred removal to external scaling interference.
expected_count = count - 1;
syslog(LOG_DEBUG,
"threadpool (%s): hill-climb shrink to %zu threads "
"(ema %.1f -> %.1f)",