Skip to content

perf(tools): malloc_trim(0) after each tool dispatch - #23675

Closed
NikolayGusev-astra wants to merge 1 commit into
NousResearch:mainfrom
NikolayGusev-astra:feat/malloc-trim
Closed

perf(tools): malloc_trim(0) after each tool dispatch#23675
NikolayGusev-astra wants to merge 1 commit into
NousResearch:mainfrom
NikolayGusev-astra:feat/malloc-trim

Conversation

@NikolayGusev-astra

Copy link
Copy Markdown

Summary

Call malloc_trim(0) after each tool dispatch in handle_function_call() to return freed heap pages to the OS.

Why

On long CLI sessions (100+ turns), Python heap can grow to 400+ MB RSS. malloc_trim(0) tells glibc to release freed pages.

Implementation

  • _maybe_trim_memory() via ctypes
  • Called after every registry.dispatch()
  • Linux-only, best-effort, silent failure

Tested

  • Syntax-validated in container
  • Does not break tool dispatch
  • 13 lines total

@teknium1 teknium1 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for targeting long-lived-session memory pressure. The current main branch still has no allocator-trimming implementation, but this patch needs rework before it can be evaluated safely.

Problems

  • model_tools.py:41 loads libc.so.6 and calls malloc_trim(0) without a platform gate, cached capability check, throttle, or measured trigger; PR line 803 invokes it after every shared tool execution.
  • Current main no longer has the direct-dispatch structure this patch targets: model_tools.py:1281-1291 now routes through execution middleware, whose chain can return without calling the terminal registry dispatch (hermes_cli/middleware.py:192-210, 240-300). A direct salvage would not preserve the stated "after each registry dispatch" semantics.
  • No tests establish invocation behavior, unsupported-platform behavior, or an RSS benefit.

Suggested changes

  • Reproduce and measure the RSS issue on current main first, then use a cached Linux/glibc capability check and a throttled cleanup boundary rather than unconditional per-tool trimming.
  • Rework against the middleware-aware path and add focused coverage for the chosen trigger policy.

Automated hermes-sweeper review.

Comment thread model_tools.py
"""Call malloc_trim(0) on Linux to reduce RSS."""
try:
import ctypes
libc = ctypes.CDLL("libc.so.6")

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This shared hot path attempts to load libc.so.6 on every call and has no platform gate, cached capability check, or throttle. Please establish a measured trigger and make the Linux/glibc capability detection cached before adding allocator work here.

Comment thread model_tools.py
)
duration_ms = int((time.monotonic() - _dispatch_start) * 1000)

# Autolycus: return freed heap pages to OS

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Current main now wraps the terminal registry callback in run_tool_execution_middleware() (model_tools.py:1281-1291). A salvage must decide whether trimming follows an actual registry dispatch, since middleware can return a result without invoking its downstream callback.

@teknium1 teknium1 added sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:risk-platform-windows Sweeper risk: may break or behave differently on native Windows sweeper:blast-broad Sweeper blast radius: broad — a core path most sessions hit labels Jul 13, 2026
@kshitijk4poor

Copy link
Copy Markdown
Collaborator

Thanks @NikolayGusev-astra for identifying the RSS growth issue and proposing malloc_trim — the underlying idea is sound and the problem is real. We're consolidating this work into #66355, which implements the same allocator trim with a configurable cooldown, telemetry, allocator/platform guards, and coverage across multiple lifecycle points, avoiding the latency cost of trimming after every tool dispatch. Closing this in favor of #66355; if you spot a lifecycle hook it misses, feedback there would be very welcome.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/tools Tool registry, model_tools, toolsets P3 Low — cosmetic, nice to have sweeper:blast-broad Sweeper blast radius: broad — a core path most sessions hit sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:risk-platform-windows Sweeper risk: may break or behave differently on native Windows type/perf Performance improvement or optimization

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants