From c2eda92fd047665aa8752e68f2625059c409c073 Mon Sep 17 00:00:00 2001 From: teknium1 <127238744+teknium1@users.noreply.github.com> Date: Wed, 29 Jul 2026 09:32:13 -0700 Subject: [PATCH] perf(config): stop deepcopying config on per-turn read-only paths MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Four hot-path consumers paid a full config deepcopy per read: - telemetry gate relay_shared_metrics.enabled() — runs 2-3x per agent turn (2x per API call from lifecycle hooks + 1x per tool call) and called read_raw_config(), which deepcopies the whole raw config every call. New read_raw_config_readonly() serves the cached dict directly: 248 us -> 4.6 us per call (54x) on Teknium's real 77-key config. - interruptible_streaming_api_call local-endpoint stale-timeout branch called load_config() once per API call for every local-model user. - gateway get_inbound_media_max_bytes() + _get_ephemeral_system_ttl_default() called load_config() on per-message paths. All three switched to load_config_readonly() (345 us -> 12 us; PR #28866 lineage). Together these account for ~90% of the ~1,900 deepcopy primitives per turn measured in the 26-call stubbed-LLM profile. read_raw_config_readonly() keeps the (mtime_ns, size) freshness key so config edits are picked up next call, and preserves the identity invariant (cache-miss returns the same object later hits serve) — regression-tested with 'is', per the PR #28866 identity-bug lesson. The mutable read_raw_config() is unchanged for save-path callers. 581 targeted tests green (config, relay metrics x2, ephemeral reply, platform base, new readonly suite). --- agent/chat_completion_helpers.py | 4 +- gateway/platforms/base.py | 8 +- hermes_cli/config.py | 45 +++++++++ .../observability/relay_shared_metrics.py | 8 +- .../test_read_raw_config_readonly.py | 92 +++++++++++++++++++ 5 files changed, 148 insertions(+), 9 deletions(-) create mode 100644 tests/hermes_cli/test_read_raw_config_readonly.py diff --git a/agent/chat_completion_helpers.py b/agent/chat_completion_helpers.py index 33d8452e75c..1c49a59e2a3 100644 --- a/agent/chat_completion_helpers.py +++ b/agent/chat_completion_helpers.py @@ -4028,9 +4028,9 @@ def interruptible_streaming_api_call(agent, api_kwargs: dict, *, on_first_delta= # env var ``HERMES_LOCAL_STREAM_STALE_TIMEOUT`` overrides for escape-hatch. _local_default = 900.0 try: - from hermes_cli.config import load_config + from hermes_cli.config import load_config_readonly - _cfg = load_config() + _cfg = load_config_readonly() # read-only consumer — no deepcopy _agent_cfg = _cfg.get("agent") if isinstance(_cfg, dict) else None if isinstance(_agent_cfg, dict): _v = _agent_cfg.get("local_stream_stale_timeout") diff --git a/gateway/platforms/base.py b/gateway/platforms/base.py index a2e00343d3f..f87c3820d6b 100644 --- a/gateway/platforms/base.py +++ b/gateway/platforms/base.py @@ -731,8 +731,8 @@ def get_inbound_media_max_bytes() -> int: unreadable — falls back to the default. """ try: - from hermes_cli.config import load_config as _load_config - cfg = _load_config() + from hermes_cli.config import load_config_readonly as _load_config + cfg = _load_config() # read-only: .get() only, never mutated except Exception: return DEFAULT_INBOUND_MEDIA_MAX_BYTES gw = cfg.get("gateway", {}) if isinstance(cfg, dict) else {} @@ -3572,11 +3572,11 @@ class BasePlatformAdapter(ABC): auto-deletion. Non-fatal if config is unreadable. """ try: - from hermes_cli.config import load_config as _load_config + from hermes_cli.config import load_config_readonly as _load_config except Exception: return 0 try: - cfg = _load_config() + cfg = _load_config() # read-only: .get() only, never mutated except Exception: return 0 display = cfg.get("display", {}) if isinstance(cfg, dict) else {} diff --git a/hermes_cli/config.py b/hermes_cli/config.py index eda819929ac..663c89a38fe 100644 --- a/hermes_cli/config.py +++ b/hermes_cli/config.py @@ -2937,6 +2937,51 @@ def read_user_config_raw(config_path: Optional[Path] = None) -> Dict[str, Any]: return data if isinstance(data, dict) else {} +def read_raw_config_readonly() -> Dict[str, Any]: + """Fast-path variant of ``read_raw_config()`` for callers that ONLY READ. + + Returns the cached raw-config dict directly, skipping the per-call + ``copy.deepcopy`` that ``read_raw_config()`` performs (needed there + because some callers mutate the result before ``save_config``). + + **Mutating the returned dict corrupts the in-process cache for every + subsequent caller.** Only use on read-only paths — e.g. per-turn policy + checks like the shared-metrics gate, which runs 2-3x per agent turn and + was paying a full config deepcopy each time. + + Same (mtime_ns, size) freshness key as ``read_raw_config()`` — an edited + config.yaml is picked up on the next call. + """ + with _CONFIG_LOCK: + try: + config_path = get_config_path() + st = config_path.stat() + cache_key = (st.st_mtime_ns, st.st_size) + except (FileNotFoundError, OSError): + return {} + + path_key = str(config_path) + cached = _RAW_CONFIG_CACHE.get(path_key) + if cached is not None and cached[:2] == cache_key: + return cached[2] + + try: + with open(config_path, encoding="utf-8") as f: + data = fast_safe_load(f) or {} + except Exception as e: + _warn_config_parse_failure(config_path, e) + return {} + + if not isinstance(data, dict): + data = {} + # Store and return THE SAME object (identity invariant): the first + # caller must see the exact dict later cache hits return, so a test + # asserting ``ro1 is ro2`` holds from the very first call. + cached_copy = copy.deepcopy(data) + _RAW_CONFIG_CACHE[path_key] = (cache_key[0], cache_key[1], cached_copy) + return cached_copy + + def require_readable_config_before_write(config_path: Optional[Path] = None) -> None: """Refuse to replace an existing config.yaml that cannot be read.""" if config_path is None: diff --git a/hermes_cli/observability/relay_shared_metrics.py b/hermes_cli/observability/relay_shared_metrics.py index c35be04d697..a7af6b879e4 100644 --- a/hermes_cli/observability/relay_shared_metrics.py +++ b/hermes_cli/observability/relay_shared_metrics.py @@ -634,12 +634,14 @@ def enabled() -> bool: """Return the shared-metrics policy for the active Hermes profile.""" profile_key = relay_runtime.current_profile_key() try: - from hermes_cli.config import read_raw_config + from hermes_cli.config import read_raw_config_readonly # Collection consent is profile-owned. Managed config overlays may # control runtime policy, but cannot opt a profile into or out of - # shared metrics. - config = read_raw_config() or {} + # shared metrics. Read-only fast path: this gate runs 2-3x per agent + # turn, and the mutable read_raw_config() paid a full config deepcopy + # on every call. + config = read_raw_config_readonly() or {} except Exception: logger.debug("Unable to read Hermes shared-metrics policy", exc_info=True) value = False diff --git a/tests/hermes_cli/test_read_raw_config_readonly.py b/tests/hermes_cli/test_read_raw_config_readonly.py new file mode 100644 index 00000000000..7f00379a84f --- /dev/null +++ b/tests/hermes_cli/test_read_raw_config_readonly.py @@ -0,0 +1,92 @@ +"""Tests for read_raw_config_readonly() — the no-deepcopy raw config read. + +The readonly variant exists for per-turn policy checks (e.g. the +shared-metrics gate) that were paying a full config deepcopy on every call. +Contract under test: + +1. identity invariant — repeat calls return the SAME cached object, + including across the very first (cache-miss) call; +2. freshness — an edited config.yaml (mtime/size change) is picked up; +3. parity — content equals read_raw_config()'s result; +4. missing/broken config degrades to {} exactly like read_raw_config(). +""" + +import os +import time + +import pytest +import yaml + + +@pytest.fixture() +def isolated_hermes_home(): + """Per-test HERMES_HOME dir (already redirected by the autouse conftest + fixture) as a Path, with the raw-config cache cleared around the test.""" + from pathlib import Path + + import hermes_cli.config as config_mod + + home = Path(os.environ["HERMES_HOME"]) + home.mkdir(parents=True, exist_ok=True) + config_mod._RAW_CONFIG_CACHE.clear() + yield home + config_mod._RAW_CONFIG_CACHE.clear() + + +def _write_config(home, data): + cfg = home / "config.yaml" + cfg.write_text(yaml.safe_dump(data), encoding="utf-8") + return cfg + + +def test_identity_invariant_from_first_call(isolated_hermes_home): + from hermes_cli.config import read_raw_config_readonly + + _write_config(isolated_hermes_home, {"telemetry": {"shared_metrics": {"enabled": True}}}) + ro1 = read_raw_config_readonly() + ro2 = read_raw_config_readonly() + assert ro1 is ro2, "cache-miss return must be the same object later hits serve" + assert ro1["telemetry"]["shared_metrics"]["enabled"] is True + + +def test_parity_with_mutable_read(isolated_hermes_home): + from hermes_cli.config import read_raw_config, read_raw_config_readonly + + _write_config(isolated_hermes_home, {"gateway": {"max_inbound_media_bytes": 123}}) + assert read_raw_config_readonly() == read_raw_config() + + +def test_freshness_after_config_edit(isolated_hermes_home): + from hermes_cli.config import read_raw_config_readonly + + cfg = _write_config(isolated_hermes_home, {"display": {"ephemeral_system_ttl": 1}}) + first = read_raw_config_readonly() + assert first["display"]["ephemeral_system_ttl"] == 1 + + _write_config(isolated_hermes_home, {"display": {"ephemeral_system_ttl": 7}}) + # Force a distinct mtime_ns even on coarse-timestamp filesystems. + st = cfg.stat() + os.utime(cfg, ns=(st.st_atime_ns, st.st_mtime_ns + 1_000_000)) + + second = read_raw_config_readonly() + assert second["display"]["ephemeral_system_ttl"] == 7 + + +def test_missing_config_returns_empty(isolated_hermes_home): + from hermes_cli.config import read_raw_config_readonly + + cfg = isolated_hermes_home / "config.yaml" + if cfg.exists(): + cfg.unlink() + assert read_raw_config_readonly() == {} + + +def test_mutable_variant_still_isolated(isolated_hermes_home): + """read_raw_config() callers may mutate; that must not corrupt the + readonly cache entry.""" + from hermes_cli.config import read_raw_config, read_raw_config_readonly + + _write_config(isolated_hermes_home, {"a": {"b": 1}}) + mutable = read_raw_config() + mutable["a"]["b"] = 999 + assert read_raw_config_readonly()["a"]["b"] == 1