perf(runtime): reuse source fingerprint per request - #4694
Conversation
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
left a comment
There was a problem hiding this comment.
评审 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 行是测试。
关键代码讲解
loopx/control_plane/effect_runtime.py的effect_runtime_request_scope(第 279 行):外层调用创建_RequestRuntimeRevision并设置ContextVar;嵌套调用走try_join()只加引用计数。finally里leave()把计数减到 0 时置_closed并清空 revision,再reset(token),因此下一个请求一定重新取指纹,不存在跨请求陈旧。- 同文件
_runtime_fingerprint_for_request(第 302 行):有作用域就用作用域内的 revision,没有就调用原来的_runtime_fingerprint();resolve()若发现状态已 close,则返回一次新解析而不是复用。未加作用域的调用者行为与改动前逐字相同。 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.
|
Merge-readiness re-read for the unchanged approved head
The failing diagnostics are the pre-repair, repo-wide ones, not this change. Action: update the branch onto latest No code change is requested here and no merge action was taken. |
Goal And Delivered Outcome
main.Scope And Continuation
collect_status().Validation
8153dc22868b72fe32b5f666ee2416e058fccb5afinishedsynthetic,public_fixturestaticpassedstatus.py. Diff and DCO checks passed.unitpassedreal_entrypointpassedloopx canary premerge --from-git-diffran 18 of 18 checks with no warnings, failures, skips, or manual holds.real_backendpassedregression_paritypassedstaticfailedorigin/mainand this branch.See validation disclosure guidance.
Frontend / Visual Evidence
nonenoneType of Change
LoopX Area
Technical Direction
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
.loopx/,.codex/goals/, and liveACTIVE_GOAL_STATE.md).none.Signed-off-bytrailer (git commit -s).