feat(mem): add config-driven allocator trim - #63708
Conversation
625d924 to
312d7c7
Compare
teknium1
left a comment
There was a problem hiding this comment.
Thanks for re-scoping the earlier allocator-trim work around config.yaml; current main still has no equivalent trim at run_agent.py:3604-3628 or the TUI finalizer at tui_gateway/server.py:9365-9377.
Problems
run_agent.py:3547resolves trim configuration duringAIAgent.close()without a profile context. Currenttui_gateway/server.py:689-713invokesagent.close()during teardown without installing the session'sprofile_home, so a resumed profile can use the default profile'scontext.memory_trimsetting.
Suggested changes
- Resolve the trim decision in a profile-scoped teardown path, or carry profile/config state to the agent, and add a two-profile regression covering a disabled profile.
- Use
load_config_readonly()for the helper's read-only lookup;hermes_cli/config.py:6837-6874documents it specifically for allocation-sensitive hot reads.
This is an 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.
This resolves config at close time, but AIAgent has no profile-home input. tui_gateway/server.py:_teardown_session calls agent.close() after the per-turn override has been reset, so a resumed profile that disables trimming can consult the default profile config. Move the decision into a profile-scoped caller (or carry profile/config state on the agent) and add a profile-isolation regression.
…gateway/slash_worker housekeeper + forced-trim logging (PR NousResearch#63708 + NousResearch#64591 enhanced, CSA tier-4 reviewed)
…gateway/slash_worker housekeeper + forced-trim logging (PR NousResearch#63708 + NousResearch#64591 enhanced, CSA tier-4 reviewed)
…gateway/slash_worker housekeeper + forced-trim logging (PR NousResearch#63708 + NousResearch#64591 enhanced, CSA tier-4 reviewed)
Return freed glibc heap pages after hard agent close and TUI turn completion, with safe no-op behavior on unsupported allocators. Configure enablement and cooldown through context.memory_trim in config.yaml rather than environment variables.
312d7c7 to
b909061
Compare
GottZ
left a comment
There was a problem hiding this comment.
This was generated by AI during triage.
Summary
Three PRs address retained RSS in long-running processes by invoking glibc malloc_trim after releasing stale references. #62961 introduced the allocator helper and TUI/agent cleanup boundaries, #63708 replaced its environment-variable controls with config.yaml settings and profile-aware TUI ordering, and #66355 expanded that design across gateway, TUI, slash-worker, and agent lifecycles with telemetry.
Related pull requests
- #62961 [closed]
duplicate— (+274/-0) — closed baseline, superseded by #63708: it directly targets retained allocator pages with rate-limited trims after agent close and TUI turns, but its behavioral environment variables and missing config plumbing conflict with AGENTS.md:102-106, as cited by the automated close verdict. - #63708
related— (+345/-0) — close as superseded by the #66355 → #76905 salvage path: it preserves #62961's trim mechanism while moving controls into context.memory_trim and keeping the profile override active for the TUI post-turn read. Despite the keep_open review on #63708, which identified unscoped AIAgent.close() configuration and requested load_config_readonly(), the later consolidated implementation carried this core diff and broader lifecycle coverage into #76905. - #66355 [closed]
duplicate— (+644/-0) — closed in favor of #76905, but relevant as the consolidated reference implementation: it incorporates the config-driven helper and TUI cleanup from #63708, then adds gateway housekeeping, idle-reaper, slash-worker, telemetry, and agent-close coverage. Despite the keep_open review on #66355, which flagged nested forced trims, missing documentation, and lifecycle-test gaps, the contributor closure records that its five lifecycle hooks and tests were re-anchored onto current main and salvaged into #76905.
Duplicates
#62961 and #63708 substantially overlap on the allocator helper plus agent-close and TUI turn boundaries; #63708 supersedes #62961's configuration approach. #66355 then consolidates that work with broader lifecycle coverage, and was itself superseded by the salvage in #76905, yielding the chain #62961 → #63708 → #66355 → #76905.
Suggested consolidation
Close #63708 as duplicate of the consolidated salvage #76905, making the full supersession chain explicit: #62961 → #63708 → #66355 → #76905. This follows the existing automated close decision on #62961 and the contributor-recorded closure of #66355 in favor of #76905; no merge recommendation is warranted here.
Complex graph
flowchart LR
classDef open fill:#dbeafe,stroke:#1d4ed8,color:#1e3a8a
classDef merged fill:#dcfce7,stroke:#15803d,color:#14532d
classDef closed fill:#e5e7eb,stroke:#6b7280,color:#1f2937
classDef unverified fill:#f3f4f6,stroke:#9ca3af,color:#374151
classDef best stroke-width:3px,stroke:#b45309
classDef target stroke-width:3px,stroke:#4338ca
subgraph Dup62961 ["PRs duplicating each other"]
P62961["PR #62961 (closed)"]
P63708["PR #63708 (open)"]
P66355["PR #66355 (closed)"]
end
class P62961 closed
class P63708 open
class P66355 closed
class P63708 target
click P62961 "https://github.com/NousResearch/hermes-agent/pull/62961"
click P63708 "https://github.com/NousResearch/hermes-agent/pull/63708"
click P66355 "https://github.com/NousResearch/hermes-agent/pull/66355"
Graph: solid arrow = fixes / best fix, dashed arrow = partial or unverified (see edge label); boxed group = PRs duplicating each other; amber border = best fix; indigo border = target; gray node = closed (state tag in the node label).
Cross-PR triage: Reviewed 3 pull requests and 0 issues in this complex. Each diff was read against this issue; Assessment working set: 55 kB of PR diffs, 7 kB of issue/PR text, 3 kB of discussion (5 comments), 0 verify verdicts. verdicts reflect diff content, not PR titles. Part of an automated triage batch.
…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 #63708 + #64591 with enhanced telemetry and gateway/slash_worker coverage.
…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.
|
FYI — this already landed upstream. Config-driven allocator trim is covered by PR #76905 (commit |
Summary
AIAgent.close()and TUI turn completioncontext.memory_trim.enabledandcontext.memory_trim.cooldown_secondsinconfig.yamlHERMES_HOMEactive until the post-turn trim reads that session's configWhy this replaces #62961
#62961 established the allocator mechanism and cleanup boundaries, but it was closed because its kill switch and cooldown were environment-variable controls and the advertised
config.yamlplumbing was absent. This revision removes those behavioral environment variables, adds the settings toDEFAULT_CONFIG, and tests the config contract and profile-ordering boundary.Tests
A bounded Linux/glibc allocator probe allocated 256 MiB of small blocks and observed RSS fall from 272.7 MiB before trim to 27.5 MiB after
trim_memory(force=True); the helper returnedTrue.Safety
HERMES_HOME, so the correct profile config is usedAI-assisted development disclosure
This revision was implemented and tested with AI assistance. The original allocator design from #62961 was replayed only after a clean apply check; the configuration-policy and profile-ordering adaptations were developed with failing regression tests before implementation.