perf(cli): cache LoadedSettings per workspace with stat-based invalidation - #6310
Conversation
…ation
The ACP child under `qwen serve` is long-lived and re-runs a full
loadSettings() on the shared event loop for every session/new,
session/load and session/resume: four settings files read, parsed,
migration-checked and structuredClone'd, the .env tree walked, home
.env re-read, ${VAR} references re-resolved, and all scopes merged.
Same-cwd repeat sessions (the typical serve workload) pay full price
every time.
Add a process-level cache keyed by resolved workspace dir (LRU 64).
Freshness is checked deterministically on every access via a
fingerprint of every filesystem input: stat signatures
(mtimeMs:size:ino) of the four settings files, the re-discovered .env
file list with signatures, IDE trust, realpath(cwd) and
realpath(homedir). Any change -> full reload; fingerprint errors fail
open to a reload; loadSettings() throws propagate uncached.
Only the three hot ACP session handlers switch to loadSettingsCached();
all other loadSettings() callers (ext-methods write paths etc.) keep
their direct read semantics.
Known accepted differences (documented in the module doc): direct
process.env mutation without any file change does not re-bake ${VAR}
references on a hit; a .env edit racing the miss-path load itself is
the usual mtime-cache TOCTOU microsecond window; an in-place overwrite
preserving mtime+size+ino is invisible (self-writes go through
temp+rename, which changes the inode).
Part of the qwen serve multi-session performance work (#6263).
🤖 Generated with [Qwen Code](https://github.com/QwenLM/qwen-code)
There was a problem hiding this comment.
Pull request overview
Introduces a process-level cache for LoadedSettings in the CLI to speed up ACP session creation in long-lived qwen serve children, using stat-based fingerprinting to deterministically invalidate cached entries when relevant inputs change.
Changes:
- Added
loadSettingsCached()with an LRU-bounded workspace cache and stat-based fingerprint invalidation. - Switched ACP session entry points (
newSession,loadSession,unstable_resumeSession) to use the cached loader. - Exported
findEnvFiles()so the cache can reuse the exact.envdiscovery semantics, and added a focused unit test suite for the cache.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/cli/src/config/settings-cache.ts | New LoadedSettings cache keyed by workspace directory with fingerprint-based invalidation and LRU eviction. |
| packages/cli/src/config/settings-cache.test.ts | New unit tests covering cache hits, invalidation triggers, non-caching of failures, and LRU eviction. |
| packages/cli/src/config/environment.ts | Exports findEnvFiles() for reuse by the cache’s fingerprint validation. |
| packages/cli/src/acp-integration/acpAgent.ts | Routes ACP session creation paths through loadSettingsCached() instead of loadSettings(). |
| packages/cli/src/acp-integration/acpAgent.test.ts | Mocks settings-cache as passthrough to preserve existing per-call loadSettings mocks. |
| packages/cli/src/acp-integration/acpAgent.worktree.test.ts | Same passthrough mock for worktree ACP tests to avoid caching interfering with mock setups. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Thanks for the PR! Template looks good ✓ Problem: Real, observed performance bottleneck — Direction: Aligned. Settings caching on the ACP session-creation hot path is the right first step before heavier extension/snapshot caching follow-ups. The honest e2e assessment (HTTP-level timing can't resolve 0.5ms against 70-80ms total) is appreciated and shows intellectual honesty about what's measurable. Approach: Scope is tight — one new module (241 lines), one export visibility change, three call-site swaps. The stat-based invalidation with documented gaps is the right tradeoff. Note: core-path line count (~561 including the 315-line test file) crosses the 500-line Tier 1 threshold, but this is a breadth-concentrated change where the bulk is collocated tests (project convention). Non-test core changes are ~246 lines. Flagging for maintainer awareness, not blocking. Moving on to code review. 🔍 中文说明感谢贡献! 模板完整 ✓ 问题:真实的性能瓶颈—— 方向:对齐。ACP session 创建热路径上的 settings 缓存是更重的 extension/snapshot 缓存之前应该先落地的低风险优化。对 e2e 结果的诚实评估值得肯定。 方案:范围精准——一个新模块(241 行)、一个导出可见性变更、三个调用点替换。核心路径行数(含 315 行测试文件约 561 行)超过 500 行阈值,但大部分是按项目惯例 collocated 的测试,非测试核心改动约 246 行。标记 maintainer 知悉,不阻塞。 进入代码审查 🔍 — Qwen Code · qwen3.7-max |
Code ReviewIndependent proposal before reading diff: I'd add a process-level Map keyed by resolved workspace dir, wrapping The PR's approach matches this exactly and exceeds it — the fingerprint is more thorough than I'd initially considered (includes Fingerprint completeness — signs all four settings file paths, discovered Fail-open discipline — every error path (fingerprint collection, cache storage, load failure) falls through to a full LRU via Map insertion order — delete + re-set on hit refreshes position; oldest key evicted when size exceeds 64. Simple and correct. Shared instance safety — repo-wide grep confirms no caller mutates Reuse — No critical blockers found. No AGENTS.md violations. Unit TestsThe test suite covers identity (same instance = no reload), every invalidation trigger (user/workspace settings edit, workspace settings appearing + deletion, Real-Scenario Testing (qwen serve)Built and bundled the PR code, started All 13 sessions returned 200 with zero errors. First session (cold, cache miss) took 33ms server-side; subsequent sessions (cache hits on — Qwen Code · qwen3.7-max |
ReflectionThis is a well-executed performance PR that solves a real bottleneck on the shared ACP event loop. The author correctly identified the synchronous What makes this maintainable: the module doc is the load-bearing artifact. The three documented invalidation gaps (process.env mutation, microsecond TOCTOU, mtime-preserving overwrite) are the kind of thing that saves a future debugger hours of confusion. The fail-open discipline on every error path means this cache can never become a new failure source. The IMPORTANT comment on The scope is minimal and every line serves the stated goal. My independent proposal was a simpler fingerprint (just settings files + .env), and the PR exceeds it by also signing Real-scenario testing confirms: 13 consecutive sessions on the same workspace, all 200, first (miss) at 33ms, subsequent (hits) at 9-14ms. The Approving. ✅ 中文说明这是一个执行良好的性能优化 PR,解决了共享 ACP event loop 上的真实瓶颈。作者正确识别了同步 可维护性:模块文档是承重构件。三个已记录的失效缺口能帮未来的调试者节省数小时的困惑。所有错误路径的 fail-open 纪律确保缓存永远不会成为新的失败来源。 范围精准,每一行都服务于既定目标。我的独立方案是更简单的指纹(仅 settings 文件 + .env),PR 通过额外签名 真实场景测试确认:同一 workspace 上 13 个连续 session,全部 200,首次(miss)33ms,后续(hit)9-14ms。 批准 ✅ — Qwen Code · qwen3.7-max |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
Code Coverage Summary
CLI Package - Full Text ReportCore Package - Full Text ReportFor detailed HTML reports, please see the 'coverage-reports-22.x-ubuntu-latest' artifact from the main CI run. |
|
Part of the session-creation-path performance tracking issue #6312 (this PR is Part A: settings). |
Covers the ideTrust fingerprint component, which is the only trust input that can change within a live process (trustedFolders.json is a permanent singleton, folder-trust toggles live in the settings files). Addresses a Copilot review suggestion to guard against stale-cache trust regressions. 🤖 Generated with [Qwen Code](https://github.com/QwenLM/qwen-code)
Review round 1 — Copilot
Both threads resolved. |
…rage
Addresses three review suggestions:
- Warn comment on settingsFileSigs that the 4-scope path list must stay in
sync with loadSettings() (unlike envFileSigs, it is enumerated separately).
- Add a createDebugLogger('SETTINGS_CACHE'), matching the SETTINGS /
SETTINGS_WATCHER / CONFIG convention in neighbouring config modules, and
log hit/miss, each fail-open catch (with the swallowed error), and eviction.
- Add a fault-injection test asserting the cache reloads (never throws) when
the fingerprint check fails, then recovers once the fault clears.
🤖 Generated with [Qwen Code](https://github.com/QwenLM/qwen-code)
Review round 2 — qwen-code-ci-bot (/review)All three suggestions taken in
Also: the Node 22.x CI failure was unrelated ( All three threads resolved. |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
No review findings. Downgraded from Approve to Comment: CI still running.
The implementation is clean — well-designed stat-based fingerprinting with proper fail-open semantics, LRU bounding, and thorough test coverage across all invalidation triggers. Deterministic analysis (tsc + eslint) and all 197 tests pass. LGTM once CI goes green. ✅
— qwen3.7-max via Qwen Code /review
|
@qwen-code /triage |
Maintainer verification — built & driven locally ✅I built this PR head ( 1) Real-binary E2E — the cache's own hit/miss log on the ACP pathDrove real Trail: 2) Unit / integration / static / mutation / benchmark
The mutation pass is the part I care about most for a cache: I neutered each fingerprint component one at a time and confirmed a specific test flips red for each (homeDir→1, settings sigs→4, Notes for the record (honest scoping)
中文说明(完整对应)维护者本地构建并真实驱动验证 ✅我把本 PR 的 head( 1)真实二进制 E2E —— 缓存自身在 ACP 路径上的 hit/miss 日志对构建好的二进制真实发起 轨迹: 2)单测 / 集成 / 静态 / 变异 / 基准
对缓存我最看重变异这一层:逐一"打断"每个指纹分量,确认各自都有特定测试翻红(homeDir→1、settings 签名→4、 备注(如实界定范围)
|
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅


What this PR does
Adds a process-level cache for
LoadedSettingskeyed by the resolved workspace directory (packages/cli/src/config/settings-cache.ts), and switches the three ACP session-creation handlers (newSession,loadSession,unstable_resumeSession) to use it. A cache hit revalidates a fingerprint of everythingloadSettingsreads — the four settings file paths and their stat signatures (mtimeMs:size:ino), the discovered.envfile list and signatures, IDE trust state,realpath(cwd), andrealpath(os.homedir())— and returns the cached instance only when all of it is unchanged. Any settings-file edit (including self-writes viasetValue), a.envfile appearing/disappearing/changing at any level of the upward walk, aQWEN_HOME/HOMEswitch, or an IDE trust flip invalidates the entry and takes the fullloadSettingspath again. Fingerprint collection is fail-open (any unexpected fs error falls back to a full load, so the cache can never become a new failure source), load failures are never cached, and the cache is LRU-bounded to 64 workspaces.It also exports
findEnvFilesfromenvironment.ts(one line) so the cache reuses the exact same.envdiscovery semantics (trust filtering,QWEN_HOMEredirection, home candidates) instead of duplicating them.Why it's needed
The
qwen serveACP child is a long-lived process that serves many sessions, and everysession/new/session/load/ resume currently re-runs a fullloadSettings(cwd)synchronously on the shared event loop:existsSync+readFileSync+JSON.parseover four settings scopes, migration checks, fourstructuredClonecalls,${VAR}re-interpolation, a home.envre-read, and an upward.envdirectory walk. For serve's typical workload — repeated sessions on the same cwd — this is all repeated work, and it blocks the event loop that all live sessions share. This is part A of the P0-1 "session creation path caching" item from the perf roadmap in #6263 (settings now; extension loading and command/skill snapshots follow as separate PRs).Reviewer Test Plan
How to verify
The new unit suite covers: hit identity (same instance ⇒ no reload ran), each invalidation trigger (user/workspace settings edit, workspace settings appearing and being deleted, a
.envappearing closer to cwd, home.envcontent change,QWEN_HOMEswitch,os.homedir()switch,setValueself-write), load-failure non-caching + recovery after the file is fixed, per-cwd entry independence, and LRU eviction.Functional smoke on the real path: build, start
node packages/cli/dist/index.js serve --port <port>in a sandbox workspace, then POST/sessionwith{"cwd": <ws>, "sessionScope": "thread"}repeatedly against the same cwd — every request after the first multiplexes onto the live ACP child viaconnection.newSession()and goes throughloadSettingsCached. Ran 4 rounds of 40+ consecutive creations on the cached binary with zero failures.Evidence (Before & After)
Microbenchmark (
loadSettings(ws)every call vsloadSettingsCached(ws)hit; N=200 per variant after 20 warmup iterations; sandboxed tmpdir fixture with user + workspace settings files, an 8-key home.env, cwd 3 levels deep; node v24.12.0, macOS with corporate EDR — an environment where syscalls are expensive, i.e. where this matters most).Typical config (~1.6 KB user settings, 6 MCP servers):
loadSettings(every call)loadSettingsCached(hit)→ 5.1x at p50 (across 3 runs the p50 speedup ranged 4.9–5.7x), i.e. ~0.5ms of synchronous event-loop time saved per session-create, and the p99 tail drops from ~3.7ms to ~0.2ms.
Large config (~13 KB user settings, 24 MCP servers, 120-entry
tools.exclude):loadSettings(every call)loadSettingsCached(hit)→ 6.4x at p50 (8.7x on a second run). The hit cost stays flat regardless of settings size (it is stat-bound: ~4
statSync+ the.envupward walk + 1–2realpathSync), while the full-load cost scales with settings size, so the per-call saving grows to ~1ms for heavyweight configs.End-to-end (
qwen serve+ 3×40 POST/sessionthread-scope on the same cwd, before vs after): the full-path p50 is ~70–80ms and is dominated byConfigconstruction, auth and session setup; round-to-round p50 drift on the same binary (±20ms of machine-state noise on this EDR host) is far larger than the ~0.5ms settings segment, so HTTP-level timing cannot resolve this change in either direction — reporting that honestly rather than cherry-picking a favorable round. The e2e runs double as the functional smoke above. The remaining large chunks of the session-create path are exactly what the follow-up PRs (extensions, command/skill snapshot) target.Tested on
Environment (optional)
node v24.12.0 on macOS; unit tests via vitest plus a local
qwen servedaemon (node packages/cli/dist/index.js serve --port <port>) for the end-to-end smoke.Risk & Scope
settings-cache.ts): (1) mutatingprocess.envdirectly does not re-bake${VAR}interpolations on a hit — no file changes are involved, and there is currently no in-repo trigger on the ACP path; (2) a microsecond-scale TOCTOU if a.envfile changes during a miss's load and never changes again afterwards — inherent to all mtime-based caches (same class as therequirecache or tsc incremental); (3) an in-place overwrite that preserves mtime, size and inode is invisible — our own settings writes go through temp+rename (new inode) and are immune, and on filesystems that report inode 0 (some Windows cases) detection degrades to mtime+size. Separately, sessions sharing a cwd now share oneLoadedSettingsinstance: this is safe (repo-wide grep shows no in-place writes to.merged;setValuepersists via temp+rename and recomputes merged state) and it actually improves consistency — areloadScopeFromDisknow benefits every session on that cwd instead of only one.loadSettingscaller (settings ext-methods, permission loads, memory get/set, persistence callbacks) keeps its direct load-mutate-adopt semantics. Extension loading (part B) and command/skill snapshot caching (part C) are separate follow-ups per the roadmap.Linked Issues
Follow-up to the perf roadmap in #6263 (P0-1 "session creation path caching", part A). Related: #6292 (settings concurrency fix that shaped the current call-site structure).
中文说明
本 PR 做了什么
为
LoadedSettings增加以解析后的 workspace 目录为 key 的进程级缓存(packages/cli/src/config/settings-cache.ts),并把 ACP 的三个 session 创建入口(newSession、loadSession、unstable_resumeSession)切换过去。缓存命中时会重新校验一份指纹——四个 settings 文件路径及各自的 stat 签名(mtimeMs:size:ino)、.env发现列表及签名、IDE 信任状态、realpath(cwd)、realpath(os.homedir())——全部一致才返回缓存实例。任何 settings 文件变更(含setValue自写)、任意层级.env的出现/删除/修改、QWEN_HOME/HOME切换、IDE 信任翻转都会使条目失效并重新走完整loadSettings。指纹采集 fail-open(任何意外的 fs 错误都退回全量加载,缓存层绝不成为新的失败来源),加载失败绝不缓存,缓存按 LRU 上限 64 个 workspace。同时从
environment.ts导出findEnvFiles(一行改动),让缓存复用完全相同的.env发现语义(信任过滤、QWEN_HOME重定向、home 候选顺序),而不是复制一份实现。为什么需要
qwen serve的 ACP child 是长驻进程、承载多个 session,而目前每次session/new/session/load/ resume 都在共享的 event loop 上同步重跑一遍完整loadSettings(cwd):四个 settings 作用域的existsSync+readFileSync+JSON.parse、migration 检查、四次structuredClone、${VAR}重新插值、home.env重读、.env目录向上走查。对 serve 的典型负载(同一 cwd 反复建 session)而言这些全是重复劳动,并且阻塞所有活跃 session 共享的 event loop。本 PR 是 #6263 性能路线图中 P0-1「session 创建路径缓存化」的 A 部分(本次只做 settings;extension 加载与命令/skill 快照作为后续独立 PR)。验证方式
新增单测覆盖:命中身份(同一实例 ⇒ 证明未重载)、每一种失效触发(user/workspace settings 编辑、workspace settings 新建与删除、更靠近 cwd 的
.env出现、home.env内容变化、QWEN_HOME切换、os.homedir()切换、setValue自写)、加载失败不缓存 + 文件修复后恢复、不同 cwd 条目相互独立、LRU 淘汰。命令见英文节(settings-cache.test.ts11/11,acpAgent 两个测试文件 184/184——缓存在其中被 mock 成 passthrough,保持按次mockReturnValue的既有用例语义;typecheck/lint通过)。真实链路 smoke:构建后启动
node packages/cli/dist/index.js serve --port <port>,对同一 cwd 反复 POST/session(sessionScope: "thread")——首个请求之后都会经connection.newSession()复用同一个 ACP child,走loadSettingsCached。在缓存版二进制上跑了 4 轮、每轮 40+ 次连续创建,零失败。证据(Before & After)
微基准(每次全量
loadSettings(ws)vs 缓存命中loadSettingsCached(ws);每变体 N=200、预热 20 次;tmpdir 沙箱 fixture:user + workspace settings 文件、8 个 key 的 home.env、cwd 深 3 层;node v24.12.0,macOS + 企业 EDR——syscall 昂贵的环境,正是本优化最有价值的场景)。数据见英文节两张表:典型配置(~1.6 KB user settings、6 个 MCP server)p50 5.1x(3 轮范围 4.9–5.7x,每次省 ~0.5ms 同步 event-loop 时间,p99 尾部 ~3.7ms → ~0.2ms);大配置(~13 KB、24 个 MCP server、120 项tools.exclude)p50 6.4x(另一轮 8.7x)——命中成本与 settings 体积无关(stat 决定:~4 次statSync+.env向上走查 + 1–2 次realpathSync),全量加载成本随体积增长,重量级配置下每次节省 ~1ms。端到端(
qwen serve+ 每侧 3×40 次同 cwd thread-scope POST/session):全链路 p50 约 70–80ms,由Config构建、auth 与 session 装配主导;同一二进制的轮间 p50 漂移(本机 EDR 噪音 ±20ms)远大于 settings 段的 ~0.5ms,HTTP 级计时无法分辨本改动的方向——如实呈现而非挑选有利轮次。e2e 同时充当上述功能 smoke。session 创建路径上剩余的大块开销正是后续 PR(extensions、命令/skill 快照)的目标。风险与范围
settings-cache.ts模块注释中记录的缺口:(1) 直接改process.env不会在命中时重新烘焙${VAR}插值——不涉及任何文件变化,且 ACP 路径上目前没有仓内触发场景;(2) 若.env恰在一次 miss 的加载过程中被改且此后不再变化,存在微秒级 TOCTOU——所有 mtime 缓存(require缓存、tsc incremental)的共同窗口;(3) 保持 mtime、size、inode 全不变的原地覆写不可见——自身写盘走 temp+rename(换 inode)天然免疫,inode 恒为 0 的文件系统(部分 Windows 情形)退化为 mtime+size 检测。另外,同 cwd 的 session 现在共享同一个LoadedSettings实例:这是安全的(全仓 grep 无对.merged的原地写;setValue走 temp+rename 落盘并重算 merged),且反而改善一致性——reloadScopeFromDisk现在对该 cwd 的所有 session 生效而非只有一个。loadSettings调用方(settings ext-methods、权限加载、memory 读写、持久化回调)保持直读的 load-mutate-adopt 语义。extension 加载(B 部分)与命令/skill 快照缓存(C 部分)按路线图作为独立后续 PR。关联
跟进 #6263 中的性能路线图(P0-1「session 创建路径缓存化」A 部分)。相关:#6292(settings 并发修复,塑造了当前调用点结构)。
🤖 Generated with Qwen Code