From a6b9597d5fb92969d605a858d5f14536e805553a Mon Sep 17 00:00:00 2001 From: kshitijk4poor <82637225+kshitijk4poor@users.noreply.github.com> Date: Fri, 3 Jul 2026 19:59:16 +0530 Subject: [PATCH] perf(console): cache CLI-surface summaries + bound console worker pool MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses two non-blocking review notes on the Hermes Console PR: - console_engine: the four _*_summaries helpers import a subcommand module and build a throwaway argparse tree purely to extract help summaries. The dashboard opens a fresh HermesConsoleEngine per /api/console connection, so every reconnect re-imported + re-parsed the whole CLI surface. The surface is process-static, so memoize with functools.lru_cache — callers only read the returned map. - web_server: console commands run in a worker thread via asyncio.to_thread. On a 60s timeout asyncio.wait_for cancels the awaitable, but Python threads aren't preemptible, so a stuck worker keeps running and would leak into the shared default thread pool. Route console execution through a small dedicated bounded ThreadPoolExecutor (max_workers=4) so a leaked worker is capped and concurrent console execution is bounded regardless of reconnects. Follow-up on top of @shannonsands' NS-574 Hermes Console. --- hermes_cli/console_engine.py | 11 ++++++++ hermes_cli/web_server.py | 49 +++++++++++++++++++++++++++++++----- 2 files changed, 54 insertions(+), 6 deletions(-) diff --git a/hermes_cli/console_engine.py b/hermes_cli/console_engine.py index 6d8409d1c38..7bfa13fbf60 100644 --- a/hermes_cli/console_engine.py +++ b/hermes_cli/console_engine.py @@ -10,6 +10,7 @@ from __future__ import annotations import argparse import contextlib import difflib +import functools import importlib import io import json @@ -291,6 +292,13 @@ def _noop_console_command(_args: argparse.Namespace) -> None: return None +# The CLI surface these helpers reflect is process-static: they import a +# subcommand module and build a throwaway argparse tree purely to extract help +# summaries. Nothing about the result changes across engine instances, but the +# dashboard opens a fresh HermesConsoleEngine per /api/console connection, so +# without memoization every reconnect re-imports + re-parses the whole surface. +# Cache by args (all hashable strings); callers only read the returned map. +@functools.lru_cache(maxsize=None) def _extracted_summaries( module_name: str, builder_name: str, @@ -306,6 +314,7 @@ def _extracted_summaries( return {} +@functools.lru_cache(maxsize=None) def _registered_summaries( root: str, module_name: str, @@ -322,6 +331,7 @@ def _registered_summaries( return {} +@functools.lru_cache(maxsize=None) def _builder_summaries( module_name: str, builder_name: str, @@ -335,6 +345,7 @@ def _builder_summaries( return {} +@functools.lru_cache(maxsize=None) def _adder_summaries(module_name: str, add_name: str) -> dict[tuple[str, ...], str]: try: parser, subparsers = _parser_root() diff --git a/hermes_cli/web_server.py b/hermes_cli/web_server.py index 6c19c1868a9..4231c8395d2 100644 --- a/hermes_cli/web_server.py +++ b/hermes_cli/web_server.py @@ -12,8 +12,11 @@ Usage: from contextlib import asynccontextmanager, contextmanager import asyncio +import atexit import base64 import binascii +import concurrent.futures +import functools from dataclasses import dataclass from datetime import datetime, timezone import hmac @@ -12640,6 +12643,36 @@ _CONSOLE_PROMPT = "hermes> " _CONSOLE_COMMAND_TIMEOUT_SECONDS = 60.0 _CONSOLE_OUTPUT_LIMIT = 50000 +# Console commands run in a worker thread. On a timeout, asyncio.wait_for cancels +# the *awaitable*, but Python threads aren't preemptible, so a genuinely stuck +# worker keeps running to completion. To keep that from exhausting the shared +# default thread pool (asyncio.to_thread), we run console commands on a small +# dedicated, bounded pool: a leaked worker is capped, and concurrent console +# execution is bounded to a fixed number of threads regardless of reconnects. +_CONSOLE_EXECUTOR_MAX_WORKERS = 4 +_console_executor: Optional[concurrent.futures.ThreadPoolExecutor] = None +_console_executor_lock = threading.Lock() + + +def _get_console_executor() -> concurrent.futures.ThreadPoolExecutor: + """Lazily create the bounded console worker pool (once per process).""" + global _console_executor + if _console_executor is None: + with _console_executor_lock: + if _console_executor is None: + _console_executor = concurrent.futures.ThreadPoolExecutor( + max_workers=_CONSOLE_EXECUTOR_MAX_WORKERS, + thread_name_prefix="hermes-console", + ) + # Ensure the pool is torn down on interpreter exit. Don't wait on + # in-flight workers: a stuck 60s console command must not block + # shutdown (cancel_futures drops anything not yet started). + atexit.register( + lambda: _console_executor + and _console_executor.shutdown(wait=False, cancel_futures=True) + ) + return _console_executor + def _dashboard_console_context() -> str: """Choose local vs hosted command policy for the dashboard console.""" @@ -12916,13 +12949,17 @@ async def console_ws(ws: WebSocket) -> None: async def run_command(line: str, *, confirmed: bool, command_id: int) -> None: nonlocal active_task, pending_confirmation, command_generation try: + loop = asyncio.get_running_loop() result = await asyncio.wait_for( - asyncio.to_thread( - _execute_console_line, - engine, - line, - confirmed=confirmed, - profile=profile, + loop.run_in_executor( + _get_console_executor(), + functools.partial( + _execute_console_line, + engine, + line, + confirmed=confirmed, + profile=profile, + ), ), timeout=_CONSOLE_COMMAND_TIMEOUT_SECONDS, )