perf(cli): fast-path global version startup - #37704
Conversation
89538e5 to
f700527
Compare
teknium1
left a comment
There was a problem hiding this comment.
Thanks for the focused startup optimization. The non-Termux fast-path premise is still valid on current main, but this June patch needs target-aware salvage before it is safe.
Problems
- Current
hermes_cli/main.py:248readsPROJECT_ROOT, butPROJECT_ROOTis initialized athermes_cli/main.py:342, after the early exit. The changed early call athermes_cli/main.py:320in this PR would therefore need a current-main-safe version printer; the PR diff does not update that printer. Commiteb4040242introduced the latePROJECT_ROOTreference after this PR was authored. - Current normal version output includes
Install methodathermes_cli/main.py:4389-4395; the fast printer in this PR does not emit it, so the new exact-flag path would lose a current output field.
Suggested changes
- Rework the pre-import helpers against current main so the printer derives its own root (or uses a dedicated stdlib-only helper) and preserves the current version-output fields.
- Add direct-subprocess coverage for exact
--versionon both Termux and non-Termux paths after that rework.
Automated hermes-sweeper review.
| elif _container_mode_may_be_active_fast(): | ||
| return False | ||
|
|
||
| _print_fast_version_info() |
There was a problem hiding this comment.
When salvaging this early call onto current main, _print_fast_version_info() cannot use the target's PROJECT_ROOT: current main.py:248 reads it before its initialization at main.py:342. Please make the pre-import printer derive the root independently (and preserve current version output) before enabling this exit globally.
|
Thanks @yyzquwu — this fast-path idea was a solid startup win. It has been salvaged into #62096, which moves the same logic into the canonical |
What does this PR do?
Speeds up global
hermes --versionandhermes -Vby answering them before the heavier CLI parser/config/logging imports run.The fast path keeps normal startup semantics for cases that may need routing or profile setup:
hermes versionstill uses the regular command path, Termux keeps its existingversionbehavior and disable flag, and host-side container/profile mode skips the fast path.Related Issue
N/A - small CLI startup performance improvement found while comparing agent startup speed.
Type of Change
Changes Made
hermes_cli/main.py: adds an early global version fast path for exact--version/-Vhermes_cli/main.py: keeps project-root path precedence for direct source-checkout invocationhermes_cli/main.py: skips the fast path when container mode or non-default active-profile routing may applytests/hermes_cli/test_tui_resume_flow.py: covers the fast path, direct script invocation, container/profile guards, preservedhermes version, and Termux disable scopingHow to Test
Checklist
Code
fix(scope):,feat(scope):, etc.)pytest tests/ -qand all tests pass (not run locally; focused tests below passed)Documentation & Housekeeping
docs/, docstrings) - or N/Acli-config.yaml.exampleif I added/changed config keys - or N/ACONTRIBUTING.mdorAGENTS.mdif I changed architecture or workflows - or N/AFor New Skills
N/A - this PR does not add a skill.
Screenshots / Logs
Focused startup/version test run:
Focused startup/plugin regression run:
Lint/compile/diff checks:
Local timing samples:
Local review: