perf: batch salvage of 5 verified micro-fixes (cache leaks, per-call waste, blocking sleep, O(n²) drain) - #35758
Merged
kshitijk4poor merged 6 commits intoMay 31, 2026
Conversation
…ounded growth _message_text_cache was a plain dict with no size limit. Every unique message_id whose text was fetched (for reply-context lookups) stayed in memory permanently, causing unbounded growth in long-running deployments with active group chats. Replace with an OrderedDict and evict the least-recently-used entry whenever the cache exceeds _FEISHU_MESSAGE_TEXT_CACHE_SIZE (512). Cache hits call move_to_end() to refresh LRU order. Mirrors the identical pattern already used by _pending_processing_reactions in the same class.
…ded growth The _guid_cache dict grows without bound as new contacts/groups are resolved. In a long-running gateway instance with many unique targets this becomes a slow memory leak. Replace the plain dict with an OrderedDict capped at 500 entries. When the cap is exceeded the oldest (least-recently-used) entries are evicted.
…nnect poll
PAIN BEFORE:
Inside _handle_auth_error_and_retry() (a sync function that runs on the MCP
event loop thread), there was a blocking polling loop:
while time.monotonic() < deadline:
if srv.session is not None and srv._ready.is_set():
break
time.sleep(0.25) # BLOCKS THE ENTIRE EVENT LOOP
Since _handle_auth_error_and_retry is invoked from tool handlers that run ON
the MCP event loop, time.sleep(0.25) blocked ALL concurrent MCP operations
(including other tools, keepalive heartbeats, OAuth refreshes) for 250ms per
iteration. With a 15-second deadline, worst case = 60 * 250ms = 15 seconds
of fully blocked concurrency.
WHAT WAS FIXED:
Extracted the blocking poll into an async helper _await_ready() that uses
asyncio.sleep(0.25) (non-blocking), and runs it via _run_on_mcp_loop().
_run_on_mcp_loop() properly awaits the coroutine on the event loop without
blocking the caller's thread. Added exception handling around the poll so
stuck reconnects still fall through to the error path.
The sync _handle_auth_error_and_retry now:
1. Fires reconnect signal (threadsafe)
2. Calls _run_on_mcp_loop(_await_ready(), timeout=15) — non-blocking
3. Returns; the event loop handles the polling
File: tools/mcp_tool.py
Lines: _handle_auth_error_and_retry() (~1886-1920)
Found by: exhaustive multi-pass audit (10 strategies, 1901 files, 913K lines)
…her recovery N43 — Silent plugin/bundle errors: - Plugin command dispatch: logger.debug() -> logger.warning() - Bundle dispatch: logger.debug() -> logger.warning() Plugin/auth failures are no longer invisible to operators. N42 — O(n^2) pending_watchers recovery: - Both recovery loops (startup + per-message) used while+pop(0) which is O(n) per pop - Replaced with enumerate() over the list + periodic asyncio.sleep(0) yield points - Clears the list after iteration instead of per-pop - Batch size of 100 balances throughput vs event-loop responsiveness
kshitijk4poor
force-pushed
the
salvage/perf-microfixes-batch
branch
from
May 31, 2026 07:25
7fc05c9 to
a2d9b76
Compare
kshitijk4poor
changed the title
alt-glitch
added
type/perf
kshitijk4poor
force-pushed
the
salvage/perf-microfixes-batch
branch
2 times, most recently
from
May 31, 2026 07:43
a3c6678 to
7b7d18a
Compare
…align test fixtures + AUTHOR_MAP Self-review follow-up on top of the salvaged perf fixes: - gateway/run.py (both watcher-drain sites): the salvaged O(n^2) fix (NousResearch#32708) replaced `while pending_watchers: pop(0)` with iterate-then- `watchers.clear()`, but `watchers` aliased the registry's live list. A watcher appended by a concurrent session during the `await asyncio.sleep(0)` yield would be cleared without ever being scheduled. Detach the batch atomically (`pending_watchers = []`) before iterating. - gateway/platforms/bluebubbles.py: normalize the salvaged _guid_cache LRU (NousResearch#30523) to match feishu/codebase precedent — module-level `_GUID_CACHE_SIZE` constant, `while len > cap`, and drop the redundant post-insert `move_to_end` (a fresh insert is already most-recent). - gateway/platforms/feishu.py: drop the same redundant post-insert `move_to_end` from the salvaged _message_text_cache LRU (NousResearch#23706). - scripts/release.py: add AUTHOR_MAP entries for the salvaged commits' authors (amathxbt NousResearch#22155, ErnestHysa NousResearch#32636/NousResearch#32708) so the contributor audit passes when these commits land on main. - tests/tools/test_tool_output_limits.py: autouse fixture resets the new module-level limits cache between tests. - tests/gateway/test_feishu.py: hand-built adapter fixture seeded _message_text_cache as a plain dict; it's now an OrderedDict, so the fixture type had to match.
kshitijk4poor
force-pushed
the
salvage/perf-microfixes-batch
branch
from
May 31, 2026 07:45
7b7d18a to
123d556
Compare
kshitijk4poor
enabled auto-merge (rebase)
May 31, 2026 07:45
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
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.
Batch salvage of 5 small, independent, verified
type/perffixes (user-requested). Each is a self-contained leak / per-call-waste / blocking-call fix in its own subsystem — non-overlapping files, all ≤38 lines. Original authorship preserved via cherry-pick.Included PRs
_message_text_cache(was an unbounded plain dict keyed by message_id)_guid_cache(was unbounded)tool_output_limits: cache limits for process lifetime instead of re-reading config on every tool calltime.sleep(0.25)withawait asyncio.sleep(0.25)on the MCP looppending_watcherswithoutlist.pop(0)(avoids O(n²) on large recovery), add event-loop yield points, upgrade plugin/bundle dispatch failure logs from debug→warningFollow-up commit (mine)
One small test-consistency commit on top, required by two of the changes on current main:
test_tool_output_limits.py: autouse fixture resets the new module-level limits cache between tests (otherwise the first test's read leaks into the rest → 5 failures).test_feishu.py: hand-built adapter fixture seeded_message_text_cacheas a plaindict; it's now anOrderedDictand_fetch_message_textcalls.move_to_end(), so the fixture type had to match.Verification
mainby reading the actual code at each touched site — confirmed each problem still exists (not already fixed / not stale).tests/gateway/test_feishu.py,tests/gateway/test_bluebubbles.py,tests/tools/test_tool_output_limits.py,tests/tools/test_mcp_circuit_breaker.py— 276 passed.gateway.runpasses.Deliberately excluded during triage
_refresh_detached_session()(which calls_move_to_finished()→with self._lock) insideself._lock, a non-reentrantthreading.Lock. The original three-phase snapshot pattern exists specifically to avoid this.git applyis clean but the fix is semantically wrong._check_disk_usage_warningmoved from line ~181 → ~123). Needs a rebase._process_batch_worker) — diff dedentsbatch_results_to_write.append(trajectory_entry)out of theif successblock, buttrajectory_entryis only assigned inside it →UnboundLocalErroron the first failed prompt. Needs a re-roll.malloc_trim(0)per dispatch) — not platform-gated, runs on the hottest path every tool dispatch, high throughput risk + wastedCDLLload on macOS. Should be opt-in + Linux-gated + throttled.