mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-30 19:09:28 +00:00
perf(config): stop deepcopying config on per-turn read-only paths
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).
This commit is contained in:
parent
6d292fd5eb
commit
c2eda92fd0
5 changed files with 148 additions and 9 deletions
|
|
@ -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")
|
||||
|
|
|
|||
|
|
@ -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 {}
|
||||
|
|
|
|||
|
|
@ -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:
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
92
tests/hermes_cli/test_read_raw_config_readonly.py
Normal file
92
tests/hermes_cli/test_read_raw_config_readonly.py
Normal file
|
|
@ -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
|
||||
Loading…
Add table
Add a link
Reference in a new issue