From 26c10d8a2e3ec02238250ee62b38d8447da3ddb2 Mon Sep 17 00:00:00 2001 From: "Antonio S. Musumeci" Date: Fri, 10 Apr 2026 22:21:20 -0400 Subject: [PATCH] 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) --- vendored/libfuse/include/thread_pool.hpp | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/vendored/libfuse/include/thread_pool.hpp b/vendored/libfuse/include/thread_pool.hpp index 6bda4be1..a6616336 100644 --- a/vendored/libfuse/include/thread_pool.hpp +++ b/vendored/libfuse/include/thread_pool.hpp @@ -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)",