From 9b8b054c2d0638eeaf9c09b062f5e77fec39249a Mon Sep 17 00:00:00 2001 From: Brooklyn Nicholson Date: Fri, 17 Jul 2026 15:01:54 -0400 Subject: [PATCH] =?UTF-8?q?perf:=20fast=20model=20picker=20+=20dialogs=20?= =?UTF-8?q?=E2=80=94=20config-load=20hot=20path,=20model.options=20off=20t?= =?UTF-8?q?he=20reader=20thread,=20off-screen=20turns=20skip=20rendering?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Third profiling round (after #66033 / #66347), targeting the composer model picker and dialog opens (worktree dialog etc.), measured over CDP on real 1000+-message sessions. Backend — model.options took 4.8s cold / 1.8s warm per call, and the desktop model pill/picker blocks on it every open: - agent/credential_pool: _load_config_safe uses load_config_readonly(). Every consumer only reads, and the per-call deepcopy was the dominant cost — list_authenticated_providers calls load_pool() per provider row, and each load_pool loaded (and deep-copied) the full config again via get_pool_strategy. - hermes_cli/config: memoize ensure_hermes_home() per home path. It runs inside the config lock on EVERY load_config(), paying ~14 mkdir/chmod syscalls per call. The fast path still re-checks that the home dir exists, so a deleted home is recreated as before; profile switches hit the new path and re-run. Tests cover both. - tui_gateway/server: add model.options to _LONG_HANDLERS. It measured seconds inline on the WS reader thread — while it ran, prompt.submit and session.interrupt sat unread (same class as #21123). Together: model.options RPC 4825/1842ms → 426/230ms (measured on the live desktop backend); build_models_payload in isolation 6.2s → 0.97s cold, 0.27s warm. Desktop — every Radix dialog/popover open forced a whole-document style recalc (Presence reads getComputedStyle on mount), which on a 1300-message transcript cost ~650-730ms per open (CPU profile: getAnimationName 483ms self). The worktree dialog (⌘⇧B) paid it on every single open: - thread/list: content-visibility:auto + contain-intrinsic-size on the per-turn group wrappers. Off-screen turns now skip style recalc, layout, and paint entirely; never-rendered turns hold a placeholder height (auto: remembered real size once rendered) so scrollbar and anchoring stay stable. Verified over CDP: worktree dialog open 656- 730ms → ~200ms on the same session; stick-to-bottom pin, scroll-to- top rendering, and sticky human bubbles all intact. Also: profile-session-switch harness accepts CDP_HTTP (Chrome tends to squat on 9222). Verification: - scripts/run_tests.sh: config, credential-pool, inventory, model-switch routing, tui_gateway protocol, profiles suites green (test_profiles has one pre-existing failure on main, unrelated); new tests for the ensure_hermes_home memo. - apps/desktop: tsc clean, eslint/prettier clean, thread + session suites green (326 tests). - E2E over CDP on the live app: numbers above, plus scroll/pin sanity. --- agent/credential_pool.py | 16 ++++++-- .../scripts/profile-session-switch.mjs | 2 +- .../components/assistant-ui/thread/list.tsx | 12 +++++- hermes_cli/config.py | 20 ++++++++++ .../test_ensure_hermes_home_memo.py | 40 +++++++++++++++++++ tui_gateway/server.py | 6 +++ 6 files changed, 90 insertions(+), 6 deletions(-) create mode 100644 tests/hermes_cli/test_ensure_hermes_home_memo.py diff --git a/agent/credential_pool.py b/agent/credential_pool.py index d4db081e5c4b..a968b3ef9682 100644 --- a/agent/credential_pool.py +++ b/agent/credential_pool.py @@ -43,11 +43,19 @@ logger = logging.getLogger(__name__) def _load_config_safe() -> Optional[dict]: - """Load config.yaml, returning None on any error.""" - try: - from hermes_cli.config import load_config + """Load config.yaml read-only, returning None on any error. - return load_config() + Uses ``load_config_readonly()``: every consumer in this module only reads + (``get_pool_strategy``, ``_iter_custom_providers``, the model-config seed), + and the deepcopy that ``load_config()`` pays per call is what made + credential-pool checks the dominant cost of ``model.options`` — the picker + calls ``load_pool()`` once per provider row, each of which loaded (and + deep-copied) the full config again. + """ + try: + from hermes_cli.config import load_config_readonly + + return load_config_readonly() except Exception: return None diff --git a/apps/desktop/scripts/profile-session-switch.mjs b/apps/desktop/scripts/profile-session-switch.mjs index 9445f6a3bec4..e706cbbbc2df 100644 --- a/apps/desktop/scripts/profile-session-switch.mjs +++ b/apps/desktop/scripts/profile-session-switch.mjs @@ -10,7 +10,7 @@ import { writeFileSync } from 'node:fs' -const CDP_HTTP = 'http://127.0.0.1:9222' +const CDP_HTTP = process.env.CDP_HTTP || 'http://127.0.0.1:9222' const A = process.argv[2] const B = process.argv[3] const ROUNDS = Number(process.argv[4] || 2) diff --git a/apps/desktop/src/components/assistant-ui/thread/list.tsx b/apps/desktop/src/components/assistant-ui/thread/list.tsx index 28c41b727f96..d549229660a6 100644 --- a/apps/desktop/src/components/assistant-ui/thread/list.tsx +++ b/apps/desktop/src/components/assistant-ui/thread/list.tsx @@ -360,8 +360,18 @@ const ThreadMessageListInner: FC = ({ )} {visibleGroups.map(group => ( + // content-visibility:auto — off-screen turns skip style recalc, + // layout, and paint. On a long transcript this is what keeps + // UNRELATED UI fast: any dialog/popover mount (Radix Presence + // reads getComputedStyle) forces a whole-document style recalc, + // measured ~650-730ms per open on a 1300-message session and + // ~100-200ms with this on. contain-intrinsic-size keeps a + // placeholder height for never-rendered turns (auto: remembered + // real size once rendered), so scrollbar/anchoring stay stable. + // Sticky human bubbles are unaffected — their turn is rendered + // whenever any part of it intersects the viewport.
diff --git a/hermes_cli/config.py b/hermes_cli/config.py index ac3b0aaf82fc..2a7c1cd49e51 100644 --- a/hermes_cli/config.py +++ b/hermes_cli/config.py @@ -913,14 +913,32 @@ def _ensure_default_soul_md(home: Path) -> None: _secure_file(soul_path) +# Home paths whose directory skeleton has been created this process — see +# ensure_hermes_home(). Only successful passes are recorded, so a raised +# managed-mode/missing-profile error keeps re-checking on later loads. +_HERMES_HOME_ENSURED: set = set() + + def ensure_hermes_home(): """Ensure ~/.hermes directory structure exists with secure permissions. In managed mode (NixOS), dirs are created by the activation script with setgid + group-writable (2770). We skip mkdir and set umask(0o007) so any files created (e.g. SOUL.md) are group-writable (0660). + + Memoized per home path: this runs on EVERY ``load_config()`` (inside the + config lock), and the ~14 mkdir/chmod syscalls per call made repeated + config loads the dominant cost of hot read paths like ``model.options``. + After the first successful pass for a given ``HERMES_HOME`` we only re-run + the full walk if the home directory itself has vanished (a deleted home is + recreated on the next load, as before). Profile switches change + ``get_hermes_home()`` and therefore re-run for the new path. """ home = get_hermes_home() + key = str(home) + + if key in _HERMES_HOME_ENSURED and home.is_dir(): + return # Named profiles must be created explicitly (e.g. ``hermes profile create``). # If a stale process keeps running after the profile was renamed/deleted, # silently mkdir-ing the old HERMES_HOME would resurrect an empty skeleton @@ -948,6 +966,8 @@ def ensure_hermes_home(): _secure_dir(d) _ensure_default_soul_md(home) + _HERMES_HOME_ENSURED.add(key) + def _ensure_hermes_home_managed(home: Path): """Managed-mode variant: verify dirs exist (activation creates them), seed SOUL.md.""" diff --git a/tests/hermes_cli/test_ensure_hermes_home_memo.py b/tests/hermes_cli/test_ensure_hermes_home_memo.py new file mode 100644 index 000000000000..7a07cb837ea6 --- /dev/null +++ b/tests/hermes_cli/test_ensure_hermes_home_memo.py @@ -0,0 +1,40 @@ +"""ensure_hermes_home is memoized per home path (perf: it runs on every +load_config), but a deleted home must still be recreated on the next call.""" + +import shutil + +from hermes_cli import config as cfg + + +def test_repeat_calls_are_memoized_but_deleted_home_is_recreated(tmp_path, monkeypatch): + home = tmp_path / ".hermes" + monkeypatch.setenv("HERMES_HOME", str(home)) + + cfg.ensure_hermes_home() + assert (home / "sessions").is_dir() + + # Memoized: a second call must not recreate a removed SUBDIR (the fast + # path only re-checks the home root)… + shutil.rmtree(home / "sessions") + cfg.ensure_hermes_home() + assert not (home / "sessions").exists() + + # …but a vanished HOME re-runs the full walk and restores the skeleton. + shutil.rmtree(home) + cfg.ensure_hermes_home() + assert (home / "sessions").is_dir() + + +def test_distinct_home_paths_each_get_the_skeleton(tmp_path, monkeypatch): + first = tmp_path / "a" / ".hermes" + second = tmp_path / "b" / ".hermes" + + monkeypatch.setenv("HERMES_HOME", str(first)) + cfg.ensure_hermes_home() + + # Profile switch: HERMES_HOME moves → the new path is ensured too. + monkeypatch.setenv("HERMES_HOME", str(second)) + cfg.ensure_hermes_home() + + assert (first / "logs").is_dir() + assert (second / "logs").is_dir() diff --git a/tui_gateway/server.py b/tui_gateway/server.py index b3d6df0af038..5c142328c1c8 100644 --- a/tui_gateway/server.py +++ b/tui_gateway/server.py @@ -191,6 +191,12 @@ _LONG_HANDLERS = frozenset( "complete.path", "complete.slash", "llm.oneshot", + # model.options builds the full picker payload — per-provider credential + # pool checks, pricing fetch, Nous tier check, optional custom-provider + # probe — measured seconds inline. While it runs on the reader thread, + # prompt.submit / session.interrupt sit unread (same class as #21123), + # and the Desktop model pill / picker block on it every open. + "model.options", # Pet RPCs hit the network (manifest fetch / spritesheet download) or do # per-frame PNG decode/encode (pet.cells): inline they serialize on the # reader thread, so picker previews trickle in one at a time and the