Skip to content

perf(runtime): reuse source fingerprint per request - #4694

Merged
huangruiteng merged 1 commit into
loopx-project:mainfrom
Duang777:codex/optimize-core-hotpath-9
Sep 18, 2026
Merged

huangruiteng merged 1 commit into
loopx-project:mainfrom
Duang777:codex/optimize-core-hotpath-9

Conversation

@Duang777

Copy link
Copy Markdown
Contributor

Goal And Delivered Outcome

  • Goal/source and gap: Every Effect runtime RPC rebuilt the packaged TypeScript source snapshot before selecting the managed runtime. A 50-Goal status request issued 100 RPCs and walked the same source tree 100 times.
  • Observable before/after, with the validation row that proves it: Bounded control-plane commands and programmatic status collection now pin one lazy runtime revision. The 50-Goal fixture keeps all 100 RPCs but reduces source scans from 100 to 1. An isolated nine-run comparison reduced median CLI time from 749.1 ms to 188.3 ms. A field-by-field normalized payload comparison had no differences.
  • Issue/task and intended base: Self-contained control-plane performance fix against main.

Scope And Continuation

  • Completed scope and remaining work: The Effect runtime owns a context-local revision. Nested scopes join atomically, separate requests remain fresh, and copied contexts cannot retain a revision after the final joined scope exits.
  • Slice boundary / successor: Long-running dashboard and server commands do not get a process-lifetime scope. Each status request that they serve creates a fresh scope through collect_status().

Validation

  • Tested revision: 8153dc22868b72fe32b5f666ee2416e058fccb5a
  • Run state: finished
  • Input classes: synthetic, public_fixture
Check kind Result Public-safe evidence / limitation
static passed Ruff passed on the changed files, excluding five pre-existing compatibility re-export warnings in status.py. Diff and DCO checks passed.
unit passed 123 focused runtime lifecycle, status, CLI, and current-base Todo tests passed.
real_entrypoint passed loopx canary premerge --from-git-diff ran 18 of 18 checks with no warnings, failures, skips, or manual holds.
real_backend passed The production CLI used the managed Node runtime and file-backed synthetic Goal state.
regression_parity passed The production-entrypoint probe kept 100 Effect RPCs, reduced source scans from 100 to 1, and produced an empty normalized payload diff.
static failed The configured targeted mypy command expands to the whole project and reports the same 3,704 existing errors in 427 files on both origin/main and this branch.
  • Coverage and gaps: Tests cover nested scopes, independent concurrent requests, concurrent copied contexts, parent-close races, failed fingerprint resolution, source changes between requests, long-running command isolation, runtime restart, and real status collection.

See validation disclosure guidance.

Frontend / Visual Evidence

  • UI impact: none
  • Before: N/A
  • After: N/A
  • States and viewports shown: N/A
  • Source data: none

Type of Change

  • Bug fix
  • New feature
  • Breaking change
  • Refactoring (no functional changes)
  • Documentation update
  • Test update

LoopX Area

  • Control plane (goals, todos, quota, scheduler, registry, runtime)
  • Benchmark boundary (adapters, runners, verifiers, scoring, evidence)
  • Capability or extension (providers, adapters, skills)
  • Public docs or presentation surface (README, protocols, dashboard)
  • Build, packaging, installer, or CI
  • Host or runtime integration

Technical Direction

  • Direction / acceptance reference, when applicable: Keep runtime identity in the Effect runtime boundary and reuse it only within one bounded request.

Shared-authority RFC fixture impact

N/A. This change does not claim progress against the TypeScript control-plane migration or the shared Goal Authority RFC.

Boundary Checklist

  • Neither the diff nor this PR body/comments/attachments disclose private state, credentials, raw traces or verifier output, internal links, or local machine paths (including .loopx/, .codex/goals/, and live ACTIVE_GOAL_STATE.md).
  • I did not duplicate maintainer-owned benchmark work unless a maintainer split out a public issue for it.
  • I kept the change scoped to the linked issue/task.
  • I completed the visual evidence section for UI changes, or marked UI impact none.
  • Every commit includes a DCO Signed-off-by trailer (git commit -s).

Pin one lazy Effect runtime revision across bounded control-plane commands and programmatic status collection. Preserve fresh direct calls and keep long-running CLI services outside the scope.

Signed-off-by: duanjialing.777 <duanjialing.777@bytedance.com>

@huangruiteng huangruiteng left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

评审 head:8153dc22868b72fe32b5f666ee2416e058fccb5a(base main,作者 Duang777)。

动机

这个 PR 修的是一个纯粹的控制面开销:effect_runtime_request 每次都调用 _runtime_fingerprint(),而它要遍历一次打包好的 TypeScript 源码树。50 个 Goal 的 status 请求会发出 100 次 RPC,于是同一棵源码树被走了 100 次。作者给出的 before/after 是 100 次 → 1 次扫描,九次运行的 CLI 中位数 749.1 ms → 188.3 ms,归一化 payload 无差异。

判定 goal_achieved:问题、机制、可观测结果三者对齐,且这是一个自包含、可回滚的切片。需要点明的是"不能更小"的理由——最小改动(把 fingerprint 做进程级 memoize)会让源码变更永远不可见,属于把性能问题换成正确性问题;本 PR 用的是"每个逻辑请求固定一个 revision、请求结束即丢弃"的写法。

改动思路

权威仍在 effect_runtime:revision 的选择规则没有搬走,只是新增了一个请求作用域,把同一次请求内重复的源码遍历折叠成一次。作用域用 ContextVar 承载、带引用计数(嵌套作用域加入外层而不是各自解析)、带 closed 标志(借出去的引用在父作用域退出后不能再用已退休的 revision,会重新解析)。

接入面只有两个真实入口:cli_runtime.dispatch_common_command(一次 CLI 命令 = 一次请求)和 status.collect_status(一次程序化 status = 一次请求)。这两处正是"请求"这个概念存在的边界。反过来,操作者可感知的路径——restart_effect_runtime、_serving_runtime_identity、collect_effect_runtime_readiness——刻意继续走未加作用域的 _runtime_fingerprint(),这样 doctor/重启看到的一定是当下真实的 revision,而不是被缓存的假象。

具体改动

5 个文件、+438/-22:生产代码约 130 行(作用域类型、解析器、两个包装点),其余 329 行是测试。

关键代码讲解

  1. loopx/control_plane/effect_runtime.py 的 effect_runtime_request_scope(第 279 行):外层调用创建 _RequestRuntimeRevision 并设置 ContextVar;嵌套调用走 try_join() 只加引用计数。finally 里 leave() 把计数减到 0 时置 _closed 并清空 revision,再 reset(token),因此下一个请求一定重新取指纹,不存在跨请求陈旧。
  2. 同文件 _runtime_fingerprint_for_request(第 302 行):有作用域就用作用域内的 revision,没有就调用原来的 _runtime_fingerprint();resolve() 若发现状态已 close,则返回一次新解析而不是复用。未加作用域的调用者行为与改动前逐字相同。
  3. loopx/cli_runtime.py 的 dispatch_common_command(第 326 行):原来的函数体改名为 _dispatch_common_command,新函数只在外面包一层作用域,签名与返回值不变;作用域生命周期与命令完全一致,所以长时间运行的 CLI 不会把 revision 钉住。

对主干的风险

最强的回归场景是作用域泄漏:某个调用方把 revision 带过请求边界,后续命令就会按已不匹配的源码选中运行时的 info 路径。缓解点就是上面的 _closed 检查与引用计数,且这一点有测试兜底。

我用变异验证了测试是否真的守住了这个契约:把 _runtime_fingerprint_for_request 改成"模块级缓存一次、永不过期"(也就是作者明确拒绝的那种更小实现),新套件 11 个测试里挂了 10 个,包括 test_long_running_full_cli_does_not_pin_a_revision、test_collect_status_defines_one_programmatic_request、两个 copied-context 用例和 dispatch 用例;变异文件已还原。head 上原样运行 tests/control_plane/test_effect_runtime_request_scope.py 与 tests/control_plane/test_status_rollout_event_snapshot.py 为 18 passed。

需要显式记录的有意语义变化只有一条:在一次逻辑请求内部,如果源码在请求中途发生变化,改动前每一次 RPC 都会重新取指纹、因而可能切换到新 revision,改动后会在本次请求内固定旧 revision。这正是本优化的目的,PR 说明里也写明了;跨请求的新鲜度不变,且有专门测试(test_source_changes_become_visible_on_the_next_request)。

关于 CI:本 head 的红色检查不是这个 PR 造成的,我逐项归因过——

  • kernel-static-checks、node-forward-compatibility、windows-powershell 三个 job 都挂在同一处:tests/control_plane_ts/local_authority_provider.test.ts 的 "selected provider … has accurate failure … and no fallback" 断言。我在 origin/main 657902286 上直接复现(27 pass / 8 fail),也就是 main 侧既有破损。
  • stage2c (mutants 0) 在 examples/shared-goal-authority-e2e/mutants.py 抛 ValueError: mutation locator drift,在任何 PR 代码运行之前就中止,同样是 main 侧漂移。
  • pytest / checks / merge-gate 是上述 job 的级联门。
  • 本 PR 自己的车道全绿:test-shard 1–4、postgresql-authority(真实服务器)、stage2c(e2e 1/2、installed 0)、dashboard-acceptance、dependency-review、build、Sign-off。

因此代码层面没有需要作者修的阻塞项;但合并门仍不可过——必须等 main 侧修复后重跑一次、并让 pr-review --check-merge-readiness 4694@8153dc22… 对不变 head 返回 ready。

我的整体评价

approve。这是那种"把重复劳动从热路径上拿掉、同时不牺牲语义"的正向改动:机制落在拥有 revision 选择的模块里,接入面只有两个真实请求入口,操作者可感知的读取刻意绕开缓存,未加作用域的调用者行为完全不变。测试覆盖了嵌套、复制上下文、父作用域退出、并发首次解析、指纹失败重试、跨请求新鲜度和两个入口的边界,且对"退化成进程级缓存"的变异是敏感的。

残余风险:作者那次九次计时对比与完整 50-Goal CLI 探针我没有重跑,我验证的是扫描次数契约、隔离/新鲜度测试与变异敏感性;另外合并前置条件(main 侧破损修复后重跑 CI)不在作者控制范围内,需在 main 恢复后重新取得绿色。变更本身小、局部、可回滚。

English verdict: APPROVE - 4694@8153dc22868b72fe32b5f666ee2416e058fccb5a; the request-scoped runtime revision removes 99 of 100 source scans while keeping cross-request freshness (18 local tests pass; a process-lifetime-cache mutant kills 10 of 11), the unpinned operator/readiness paths and unscoped callers are unchanged, and the one deliberate delta (mid-request pinning) is disclosed and covered. The head's red required checks are pre-existing main breakage (local_authority_provider.test.ts provider-failure assertions reproduced at origin/main 6579022, plus shared-goal-authority mutant locator drift) and cascades, so merge waits for a re-run after main is repaired and a ready check-merge-readiness on this unchanged head.

@huangruiteng

Copy link
Copy Markdown
Collaborator

Merge-readiness re-read for the unchanged approved head 8153dc22868b72fe32b5f666ee2416e058fccb5a (no re-review of the diff; the existing approval still stands):

loopx pr-review --check-merge-readiness 4694@8153dc22868b72fe32b5f666ee2416e058fccb5a is not ready:

  • status_checks_failed (8) / status_checks_incomplete: kernel-static-checks, node-forward-compatibility, windows-powershell, stage2c (mutants 0), plus the aggregate gates checks / pytest / stage2c-correctness-e2e / merge-gate.
  • merge_state_requires_update: the branch is 26 commits behind main (merge base 657902286, current main f5c95ff0).

The failing diagnostics are the pre-repair, repo-wide ones, not this change. kernel-static-checks fails on the provider matrix — e.g. selected provider database_missing has accurate failure across runtime entrypoints and no fallback (tests/control_plane_ts/local_authority_provider.test.ts). On current main that file is green: node --experimental-strip-types --test tests/control_plane_ts/local_authority_provider.test.ts → 35/35 pass at f5c95ff0.

Action: update the branch onto latest main and let the checks re-run, then re-read merge readiness on the new head. The same applies to the other two approved heads in this group (#4695c0869ef and #46914fec73e — both share merge base 657902286).

No code change is requested here and no merge action was taken.

@huangruiteng
huangruiteng merged commit 211ebfa into loopx-project:main Sep 18, 2026
18 of 26 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants