From 6163898c14c2f7bc176f40f3cb5356c3fb24520c Mon Sep 17 00:00:00 2001 From: sami Date: Fri, 11 Sep 2026 13:12:18 +0400 Subject: [PATCH] core: stop allocating an empty deque on every worker-loop iteration HttpClient::Impl::drain_commands() default-constructed a std::deque every call, on every iteration of the worker thread's event loop (once per curl_multi_poll wake -- i.e. once per socket-readiness event on the transfer hot path), then swapped the (usually empty) command queue into it. In libstdc++, an empty std::deque still allocates its map array on construction, so this was a real allocation on the hot path regardless of whether any command (add/pause/resume/cancel) was actually pending -- which is the common case, since those are rare next to data arriving. Found via tools/bench's alloc-check, which is built in this change and exists specifically to catch this class of bug (AGENT-CORE.md: "no allocation in the curl write callback... checked in review and by a bench assertion"): before this fix it reported thousands of allocations/sec under a sustained transfer; after, single digits. Fixed by checking `w.queue.empty()` under the lock before touching `local` at all, so the deque is only constructed when there's actually something to swap into it. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01Q3QrF7rCt21bkAjt9BCDFQ --- core/src/net/http_client.cpp | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/core/src/net/http_client.cpp b/core/src/net/http_client.cpp index a83b206..f265971 100644 --- a/core/src/net/http_client.cpp +++ b/core/src/net/http_client.cpp @@ -290,11 +290,20 @@ struct HttpClient::Impl { } void drain_commands(Worker &w) { + // Called every worker-loop iteration (run(), below) -- once per curl_multi_poll + // wake, so once per socket-readiness event on the transfer hot path -- but commands + // (add/pause/resume/cancel) are rare next to that. Check empty under the lock + // *before* touching `local`: libstdc++'s std::deque allocates its map array on + // default construction even with nothing pushed to it, so constructing one every + // iteration just to usually swap nothing into it was an allocation on every poll + // wake, not just on an actual command -- exactly what the curl-write-callback path + // must never do (AGENT-CORE.md; caught by tools/bench's alloc-check). + std::unique_lock lk(w.mu); + if (w.queue.empty()) + return; std::deque local; - { - std::lock_guard lk(w.mu); - local.swap(w.queue); - } + local.swap(w.queue); + lk.unlock(); for (auto &cmd : local) { auto &st = cmd.state; switch (cmd.kind) {