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:
@@ -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.
|
# env var ``HERMES_LOCAL_STREAM_STALE_TIMEOUT`` overrides for escape-hatch.
|
||||||
_local_default = 900.0
|
_local_default = 900.0
|
||||||
try:
|
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
|
_agent_cfg = _cfg.get("agent") if isinstance(_cfg, dict) else None
|
||||||
if isinstance(_agent_cfg, dict):
|
if isinstance(_agent_cfg, dict):
|
||||||
_v = _agent_cfg.get("local_stream_stale_timeout")
|
_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.
|
unreadable — falls back to the default.
|
||||||
"""
|
"""
|
||||||
try:
|
try:
|
||||||
from hermes_cli.config import load_config as _load_config
|
from hermes_cli.config import load_config_readonly as _load_config
|
||||||
cfg = _load_config()
|
cfg = _load_config() # read-only: .get() only, never mutated
|
||||||
except Exception:
|
except Exception:
|
||||||
return DEFAULT_INBOUND_MEDIA_MAX_BYTES
|
return DEFAULT_INBOUND_MEDIA_MAX_BYTES
|
||||||
gw = cfg.get("gateway", {}) if isinstance(cfg, dict) else {}
|
gw = cfg.get("gateway", {}) if isinstance(cfg, dict) else {}
|
||||||
@@ -3572,11 +3572,11 @@ class BasePlatformAdapter(ABC):
|
|||||||
auto-deletion. Non-fatal if config is unreadable.
|
auto-deletion. Non-fatal if config is unreadable.
|
||||||
"""
|
"""
|
||||||
try:
|
try:
|
||||||
from hermes_cli.config import load_config as _load_config
|
from hermes_cli.config import load_config_readonly as _load_config
|
||||||
except Exception:
|
except Exception:
|
||||||
return 0
|
return 0
|
||||||
try:
|
try:
|
||||||
cfg = _load_config()
|
cfg = _load_config() # read-only: .get() only, never mutated
|
||||||
except Exception:
|
except Exception:
|
||||||
return 0
|
return 0
|
||||||
display = cfg.get("display", {}) if isinstance(cfg, dict) else {}
|
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 {}
|
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:
|
def require_readable_config_before_write(config_path: Optional[Path] = None) -> None:
|
||||||
"""Refuse to replace an existing config.yaml that cannot be read."""
|
"""Refuse to replace an existing config.yaml that cannot be read."""
|
||||||
if config_path is None:
|
if config_path is None:
|
||||||
|
|||||||
@@ -634,12 +634,14 @@ def enabled() -> bool:
|
|||||||
"""Return the shared-metrics policy for the active Hermes profile."""
|
"""Return the shared-metrics policy for the active Hermes profile."""
|
||||||
profile_key = relay_runtime.current_profile_key()
|
profile_key = relay_runtime.current_profile_key()
|
||||||
try:
|
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
|
# Collection consent is profile-owned. Managed config overlays may
|
||||||
# control runtime policy, but cannot opt a profile into or out of
|
# control runtime policy, but cannot opt a profile into or out of
|
||||||
# shared metrics.
|
# shared metrics. Read-only fast path: this gate runs 2-3x per agent
|
||||||
config = read_raw_config() or {}
|
# turn, and the mutable read_raw_config() paid a full config deepcopy
|
||||||
|
# on every call.
|
||||||
|
config = read_raw_config_readonly() or {}
|
||||||
except Exception:
|
except Exception:
|
||||||
logger.debug("Unable to read Hermes shared-metrics policy", exc_info=True)
|
logger.debug("Unable to read Hermes shared-metrics policy", exc_info=True)
|
||||||
value = False
|
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
|
||||||
Reference in New Issue
Block a user