perf(acp): reduce hermes acp cold-start connect time by ~4× - #56601
perf(acp): reduce hermes acp cold-start connect time by ~4×#56601alanjds wants to merge 4 commits into
Conversation
Three changes that together cut the time from process spawn to the first
ACP initialize response from ~66s down to ~13–17s (cold pydantic cache).
hermes_cli/main.py — fast-path for `hermes acp`
Detect the `acp` subcommand before the heavy module-level imports in
hermes_cli.main (rich, prompt_toolkit, argparse, all subcommands) and
jump directly to acp_adapter.entry.main. This alone saves ~7s of
import time on every `hermes acp` invocation. Profile flag (-p/--profile)
is honoured before the jump. Pattern mirrors the existing Termux fast-path.
acp_adapter/entry.py — async startup with deferred heavy imports
Replace the synchronous, pre-loop startup sequence with an async _run()
coroutine that:
1. Wires sys.stdin to an asyncio StreamReader immediately on event-loop
start, so the client's initialize bytes are buffered in the kernel pipe
while imports are in flight — they are never dropped.
2. Runs `import acp` + `from acp_adapter.server import HermesACPAgent` in
asyncio.to_thread(), keeping the event loop alive and responsive during
the ~8s pydantic/opentelemetry import cost.
3. Schedules MCP tool discovery (discover_mcp_tools) as a separate
asyncio.create_task() background thread, so it does not delay the
initialize response.
Previously, discover_mcp_tools() blocked the main thread synchronously
before asyncio.run() was even called, adding another ~11s.
tests/acp/test_entry.py — update test_main_enables_unstable_protocol
The test now stubs connect_read_pipe / connect_write_pipe on
asyncio.BaseEventLoop so the test does not require real stdio, and patches
remain effective since Python's import cache ensures the same acp module
object is used inside _heavy_imports.
alt-glitch
added
type/perf
teknium1
left a comment
teknium1
left a comment
There was a problem hiding this comment.
Thanks for targeting a verified ACP cold-start bottleneck: current acp_adapter/entry.py:246-262 imports the ACP server and runs configured MCP discovery before starting the event loop.
Problems
hermes_cli/main.py:298forwards ACP arguments straight toacp_adapter.entry. The normal ACP parser adds--accept-hooks(hermes_cli/subcommands/acp.py:21,_shared.py:21), whileentry._parse_args()does not. Thushermes acp --accept-hooksregresses to an argparse error.acp_adapter/entry.py:287backgrounds configured MCP discovery without coordinating with the first session. ACP constructsAIAgentinacp_adapter/session.py:645; its tool list is snapshotted inagent/agent_init.py:1195-1205.tools/mcp_tool.py:5444-5476documents the required refresh when discovery finishes after that snapshot. The PR does not provide a bounded wait or ACP late-refresh path.
Suggested changes
- Preserve ACP parser semantics in the fast path, especially
--accept-hooks, rather than duplicating partial argument/profile handling. - Keep initialize non-blocking but synchronize discovery with the first agent snapshot or refresh it at a safe turn boundary; add a blocked-discovery regression test.
- Add an ACP initialize transport test; the changed unit test only mocks pipes and checks the protocol flag.
Automated hermes-sweeper review.
| _profiles_root = _Path.home() / ".hermes" / "profiles" | ||
| _os.environ["HERMES_HOME"] = str(_profiles_root / _profile) | ||
| from acp_adapter.entry import main as _acp_main # noqa: PLC0415 | ||
| _acp_main(argv[i + 1:]) |
There was a problem hiding this comment.
This bypasses the normal ACP parser, which accepts --accept-hooks through build_acp_parser(); acp_adapter.entry._parse_args() does not accept it. Preserve that flag’s behavior (and avoid duplicating the profile pre-parser) before forwarding arguments.
| except Exception: | ||
| logger.debug("MCP tool discovery failed at ACP startup", exc_info=True) | ||
|
|
||
| _asyncio.create_task(_discover_mcp_bg()) |
There was a problem hiding this comment.
Configured MCP discovery can still be running when the client creates its first session. That session snapshots AIAgent.tools before the registry is populated, and this path has no bounded join or ACP late-refresh. Coordinate the task with the first tool snapshot or refresh at a safe turn boundary.
teknium1
added
sweeper:risk-compatibility
|
Thanks for this — the analysis was right when filed, but the codebase has since absorbed each of the three optimizations through other merges, and the timings no longer reproduce on current main:
For IDE integrations wanting zero CLI overhead, the Closing as superseded on current main rather than for any defect in the work — the direction was sound, and the MCP-discovery half of it is exactly what ended up shipping via #75985's design. |
4 participants
Issue
ACP process startup is slow (~66s from spawn to initialize response), blocking IDE integrations and making the
hermes acpCLI command feel unresponsive.Improvements
The savings are based on:
hermes_cli/main.pydetects theacpsubcommand before loading rich, prompt_toolkit, argparse, and all subcommands (~7s saved)asyncio.create_task()) instead of blocking the main thread pre-loop (~11s saved)import acpand pydantic/opentelemetry imports run inasyncio.to_thread(), keeping the event loop responsive and able to buffer incoming initialize bytesResult
~4× speedup: Process now responds to initialize within 13–17s (cold pydantic cache), down from ~66s.
Changes
hermes acp(mirrors existing Termux fast-path pattern)