feat(mem): config-driven allocator trim with telemetry and lifecycle coverage - #66355
feat(mem): config-driven allocator trim with telemetry and lifecycle coverage#66355aider4ryder wants to merge 1 commit into
Conversation
…coverage Add config-driven glibc malloc_trim for long-lived Hermes processes: - hermes_cli/mem_trim.py: trim_memory() with configurable cooldown, RSS snapshot telemetry, and forced-trim INFO logging - gateway/run.py: periodic trim in gateway housekeeping loop - tui_gateway/server.py: trim in idle reaper (~every 5 min) - tui_gateway/slash_worker.py: trim on turn boundary - run_agent.py: force trim on agent close - hermes_cli/config.py: context.memory_trim config section (enabled, cooldown_seconds, log_every_n, info_log_min_delta_mb) CSA tier-4 reviewed (4 rounds, 0 HIGH/MEDIUM/CRITICAL remaining). Supersedes PR NousResearch#63708 + NousResearch#64591 with enhanced telemetry and gateway/slash_worker coverage.
tonydwb
left a comment
There was a problem hiding this comment.
Code Review Summary
Verdict: Comment
Memory subsystem change touching 9 files with 644 additions. The PR adds config-driven allocator trim with telemetry and lifecycle coverage. While the description sounds reasonable, the broad scope across the memory subsystem warrants human review to confirm no unintended interactions with existing memory management logic.
Reviewed by Hermes Agent
teknium1
left a comment
There was a problem hiding this comment.
Thanks for consolidating the allocator-trim work and retaining the config.yaml mechanism from the related PRs.
Problems
run_agent.py:3627addstrim_memory(force=True), butAIAgent.close()recursively callschild.close()for every active child (run_agent.py:3590-3599). The helper's force path bypasses its cooldown, so a teardown with N children performs N+1 full collections/trims. Please retain rate limiting for nested closes or designate one outer teardown trim.- The new
context.memory_trimcontrols are not documented.website/docs/user-guide/configuration.md:812-830currently documents onlycontext.engine, while this PR changes no docs file. - The tests do not cover the new
AIAgent.close()or slash-worker lifecycle hooks.
Suggested changes
- Add lifecycle tests for those two hooks, including nested-child cooldown behavior.
- Add configuration documentation for defaults and Linux/glibc-only behavior.
Automated hermes-sweeper review.
| # mark until exit. This helper is a safe no-op on other allocators. | ||
| try: | ||
| from hermes_cli.mem_trim import trim_memory | ||
| trim_memory(force=True, reason="agent close") |
There was a problem hiding this comment.
force=True bypasses the helper cooldown, while close() recursively calls child.close() for every active child. A parent teardown can therefore run a full collection/trim once per child plus once for the parent. Please preserve rate limiting for nested closes or arrange for only the outer teardown to force a trim.
|
Thanks @aider4ryder — this is exactly the comprehensive trim design the earlier per-site attempts needed (it already superseded #63708/#64591, which were closed pointing here). The base had drifted (DEFAULT_CONFIG moved to config_defaults.py, housekeeping/close/reaper anchors all shifted), so I re-anchored it onto current main and salvaged it into #76905 with your authorship preserved via cherry-pick; all 5 lifecycle hooks and every test carried over (505/505 gateway-server tests green). Closing in favor of the salvage. |
…test Simplify-pass follow-up on the NousResearch#66355 salvage: 1. _config_settings runs on EVERY trim attempt (before the cooldown check) and only reads — swap load_config for load_config_readonly. Deep-copying the whole config per attempt generates exactly the allocator garbage this module exists to release. Tests re-seamed. 2. Trim-failure logs demoted warning->debug at all 3 periodic sites (gateway housekeeping, idle reaper, slash worker): sibling failure branches in the same loops log at debug, and a persistent failure (e.g. broken import after a partial update) would otherwise warn every 60s forever. 3. The frame-inspection test now asserts the expected locals exist before reading them — a rename in _run_prompt_submit fails the test loudly instead of vacuously passing on None.
Efficiency-pass follow-up on the NousResearch#66355 salvage: force=True bypassed the cooldown entirely, and AIAgent.close() fires a forced trim for EVERY in-process child subagent close (delegate_tool child.close(), parent close step 5). A delegate batch of N children closing back-to-back in the gateway process stacked N+1 uncooled full gc.collect()+malloc_trim passes (50-500ms each with a large live heap). Forced trims now honor a 5s floor — bursts coalesce, the parent's final close-trim still fires. Guard test mutation-checked (floor zeroed -> test fails).
…test Simplify-pass follow-up on the #66355 salvage: 1. _config_settings runs on EVERY trim attempt (before the cooldown check) and only reads — swap load_config for load_config_readonly. Deep-copying the whole config per attempt generates exactly the allocator garbage this module exists to release. Tests re-seamed. 2. Trim-failure logs demoted warning->debug at all 3 periodic sites (gateway housekeeping, idle reaper, slash worker): sibling failure branches in the same loops log at debug, and a persistent failure (e.g. broken import after a partial update) would otherwise warn every 60s forever. 3. The frame-inspection test now asserts the expected locals exist before reading them — a rename in _run_prompt_submit fails the test loudly instead of vacuously passing on None.
Efficiency-pass follow-up on the #66355 salvage: force=True bypassed the cooldown entirely, and AIAgent.close() fires a forced trim for EVERY in-process child subagent close (delegate_tool child.close(), parent close step 5). A delegate batch of N children closing back-to-back in the gateway process stacked N+1 uncooled full gc.collect()+malloc_trim passes (50-500ms each with a large live heap). Forced trims now honor a 5s floor — bursts coalesce, the parent's final close-trim still fires. Guard test mutation-checked (floor zeroed -> test fails).
…test Simplify-pass follow-up on the NousResearch#66355 salvage: 1. _config_settings runs on EVERY trim attempt (before the cooldown check) and only reads — swap load_config for load_config_readonly. Deep-copying the whole config per attempt generates exactly the allocator garbage this module exists to release. Tests re-seamed. 2. Trim-failure logs demoted warning->debug at all 3 periodic sites (gateway housekeeping, idle reaper, slash worker): sibling failure branches in the same loops log at debug, and a persistent failure (e.g. broken import after a partial update) would otherwise warn every 60s forever. 3. The frame-inspection test now asserts the expected locals exist before reading them — a rename in _run_prompt_submit fails the test loudly instead of vacuously passing on None.
Efficiency-pass follow-up on the NousResearch#66355 salvage: force=True bypassed the cooldown entirely, and AIAgent.close() fires a forced trim for EVERY in-process child subagent close (delegate_tool child.close(), parent close step 5). A delegate batch of N children closing back-to-back in the gateway process stacked N+1 uncooled full gc.collect()+malloc_trim passes (50-500ms each with a large live heap). Forced trims now honor a 5s floor — bursts coalesce, the parent's final close-trim still fires. Guard test mutation-checked (floor zeroed -> test fails).
|
FYI — this already landed upstream. It was salvaged into official main as PR #76905 ( |
Summary
Config-driven glibc
malloc_trimfor long-lived Hermes processes (gateway, TUI gateway, slash_worker).Changes
hermes_cli/mem_trim.py:trim_memory()with configurable cooldown, RSS snapshot telemetry, and forced-trim INFO logginggateway/run.py: periodic trim in gateway housekeeping looptui_gateway/server.py: trim in idle reaper (~every 5 min)tui_gateway/slash_worker.py: trim on turn boundaryrun_agent.py: force trim on agent closehermes_cli/config.py:context.memory_trimconfig sectionConfig
Unsupported platforms (non-Linux/non-glibc) are safe no-ops.
Testing
Supersedes #63708 + #64591.