Skip to content

perf(plugins): memoize metadata snapshots per process#81570

Closed
Kaspre wants to merge 4 commits into
openclaw:mainfrom
Kaspre:perf/plugin-metadata-snapshot-process-memo-slim
Closed

perf(plugins): memoize metadata snapshots per process#81570
Kaspre wants to merge 4 commits into
openclaw:mainfrom
Kaspre:perf/plugin-metadata-snapshot-process-memo-slim

Conversation

@Kaspre

@Kaspre Kaspre commented May 13, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Add a small process-local memo around loadPluginMetadataSnapshot so repeated same-process CLI startup callers reuse metadata snapshot work.
  • Keep the memo key intentionally narrow: caller-visible config/control-plane inputs plus persisted registry and managed npm root fingerprints.
  • Cache only persisted/provided snapshots and policy-stale derived snapshots; broader discovery freshness hardening remains in fix(plugins): keep plugin metadata memo fresh #81064 / follow-up.
  • Ignore vanished recovered install records when their install/source paths are gone, so stale local install-record residue does not force beta6 back to full derived discovery.

AI-assisted: yes, with Codex.

Root cause

Beta6 still calls loadPluginMetadataSnapshot hundreds of times during CLI startup. The lower-level manifest scan cache helps, but callers still repeatedly rebuild registry snapshot state, manifest registry state, owner maps, id normalizers, and metrics.

The earlier slim branch avoided some broad caching risk, but it skipped policy-stale derived snapshots. On this beta6 install, many validation callers use config: {}, which produces persisted-registry-stale-policy; skipping those snapshots left the hot path intact.

Scope notes

This is the release-sized hotfix slice. It intentionally does not attempt the full freshness matrix from #81064. In particular, arbitrary direct plugin file edits in the same process are not exhaustively detected unless the persisted registry / managed npm fingerprint changes or the process restarts. That broader P2 hardening should stay with #81064 or a follow-up.

GitHub file API after the final recut reports 4 files changed, 468 additions, and 31 deletions (size: M).

Real behavior proof

Behavior or issue addressed: repeated in-process plugin metadata snapshot construction dominates beta6 CLI startup on installs with many plugins.

Real environment tested: local production OpenClaw install on Linux/WSL2, upgraded to OpenClaw 2026.5.12-beta.6 (c4292e0). The local CLI exit-fix patch was not applied for the final proof. The after-fix timing proof used #81570 production head fb8e1ce931def59793d3218ffef7c0393f8deedd applied to beta6 commit c4292e053d7c86361700b3acb59cb51e4a310f2d, built from a fresh beta6 proof worktree, and temporarily overlaid as a complete dist/ onto the same install. The current PR head is 9de96674aff0415862d1ef88be3f5722a53e61a0; commits after the timing proof are limited to a memo-key Nix-mode input simplification and test fixture/size trims. The install was restored to beta6 afterward.

Exact steps or command run after this patch: pnpm -C <slim-pr-worktree> format:check src/plugins/plugin-metadata-snapshot.ts src/plugins/plugin-metadata-snapshot.memo.test.ts src/plugins/plugin-registry-snapshot.ts src/plugins/plugin-registry-snapshot.test.ts; pnpm -C <slim-pr-worktree> test src/plugins/plugin-metadata-snapshot.memo.test.ts src/plugins/plugin-registry-snapshot.test.ts src/plugins/plugin-registry.test.ts; timeout 90s pnpm tsgo:core; pnpm -C <beta6-proof-worktree> format:check src/plugins/plugin-metadata-snapshot.ts src/plugins/plugin-registry-snapshot.ts src/plugins/plugin-metadata-snapshot.memo.test.ts src/plugins/plugin-registry-snapshot.test.ts; pnpm -C <beta6-proof-worktree> test src/plugins/plugin-metadata-snapshot.memo.test.ts src/plugins/plugin-registry-snapshot.test.ts src/plugins/plugin-registry.test.ts; timeout 240s pnpm build; /usr/bin/time -p timeout 240s openclaw plugins list; /usr/bin/time -p timeout 240s openclaw gateway status; OPENCLAW_DIAGNOSTICS=timeline OPENCLAW_DIAGNOSTICS_TIMELINE_PATH=/tmp/openclaw-beta6-81570-fb8-timeline.jsonl timeout 120s openclaw plugins list --json.

Evidence after fix: unpatched beta6 baseline in the same install was openclaw plugins list real 80.63 and openclaw gateway status real 64.15. With #81570 head fb8e1ce overlaid, openclaw plugins list completed with Plugins (18/104 enabled) in real 14.69, user 14.25, sys 1.53; openclaw gateway status completed in real 12.69, user 12.28, sys 1.69. The timeline run recorded 906 plugins.metadata.scan spans with 896 cache hits, total metadata span time 9411.2ms, max 291.8ms, and no stderr.

Observed result after fix: beta6 CLI metadata startup work collapses to process-local cache hits after the first few distinct contexts. plugins list improves from 80.63s to 14.69s, and gateway status improves from 64.15s to 12.69s on the same install.

What was not tested: current head 9de96674aff0415862d1ef88be3f5722a53e61a0 was not retimed in the local beta6 install after the post-proof memo-key Nix-mode input simplification and test-only trim commits; hosted non-draft CI is green on that exact head. The comprehensive plugin discovery freshness matrix from #81064 is intentionally out of scope for this release-sized PR; this PR only includes compact coverage for the vanished install-record case needed to keep beta6 on the persisted fast path.

Verification

  • pnpm -C <slim-pr-worktree> format:check src/plugins/plugin-metadata-snapshot.ts src/plugins/plugin-metadata-snapshot.memo.test.ts src/plugins/plugin-registry-snapshot.ts src/plugins/plugin-registry-snapshot.test.ts
  • pnpm -C <slim-pr-worktree> test src/plugins/plugin-metadata-snapshot.memo.test.ts src/plugins/plugin-registry-snapshot.test.ts src/plugins/plugin-registry.test.ts
  • timeout 90s pnpm tsgo:core
  • pnpm -C <beta6-proof-worktree> format:check src/plugins/plugin-metadata-snapshot.ts src/plugins/plugin-registry-snapshot.ts src/plugins/plugin-metadata-snapshot.memo.test.ts src/plugins/plugin-registry-snapshot.test.ts
  • pnpm -C <beta6-proof-worktree> test src/plugins/plugin-metadata-snapshot.memo.test.ts src/plugins/plugin-registry-snapshot.test.ts src/plugins/plugin-registry.test.ts
  • timeout 240s pnpm build from the beta6 proof worktree
  • git diff --check
  • Hosted non-draft CI on 9de96674aff0415862d1ef88be3f5722a53e61a0: Real behavior proof, check-lint, check-test-types, check-prod-types, check, build-artifacts, build-smoke, checks-node-core, checks-node-core-fast, checks-fast-protocol, Critical Quality (plugin-boundary), Critical Quality (network-runtime-boundary), selected Security High shards, and the remaining required PR checks.

@openclaw-barnacle openclaw-barnacle Bot added proof: supplied External PR includes structured after-fix real behavior proof. size: M labels May 13, 2026
@clawsweeper

clawsweeper Bot commented May 14, 2026

Copy link
Copy Markdown
Contributor

Codex review: needs maintainer review before merge.

Summary
The PR adds a single-entry process-local memo for plugin metadata snapshots, keyed by config/env/registry inputs, plus tests and a persisted-registry fast-path fix for vanished recovered install records.

Reproducibility: yes. at source/proof level: current main rebuilds plugin metadata snapshots for each load, and the PR body supplies beta6 terminal timing plus diagnostics cache-hit proof. I did not run local tests because this was a read-only review.

Real behavior proof
Sufficient (terminal): The PR body includes terminal proof from a real beta6 Linux/WSL2 install with baseline timings, after-fix timings, exact commands, and diagnostics cache-hit output.

Next step before merge
No repair lane is needed; there are no actionable review findings for an automated fix PR, so the remaining path is normal maintainer review/landing.

Security
Cleared: The diff stays within plugin metadata/registry code and tests, with no dependency, workflow, install-script, package-resolution, or secret-handling changes.

Review details

Best possible solution:

Land this scoped hotfix after maintainer review, and keep the broader freshness matrix in #81064 or a narrower follow-up.

Do we have a high-confidence way to reproduce the issue?

Yes, at source/proof level: current main rebuilds plugin metadata snapshots for each load, and the PR body supplies beta6 terminal timing plus diagnostics cache-hit proof. I did not run local tests because this was a read-only review.

Is this the best way to solve the issue?

Yes for the release-sized hotfix: the single-entry process memo is narrowly keyed, clone-isolates returned snapshots, and stays within the documented snapshot boundary. The wider freshness hardening is correctly left to the related broader PR or a follow-up.

What I checked:

  • Current main behavior: Current main still wraps every loadPluginMetadataSnapshot call around loadPluginMetadataSnapshotImpl with no process-local memo path, so the central performance change is not already present. (src/plugins/plugin-metadata-snapshot.ts:176, 462a05621014)
  • PR memo implementation: The PR head computes a memo key from control-plane/env/path/persisted-registry inputs, returns cloned snapshots on cache hits, and stores only allowed snapshot sources. (src/plugins/plugin-metadata-snapshot.ts:336, 9de96674aff0)
  • Regression coverage: The added memo tests cover persisted snapshot reuse, mutation isolation, policy-stale derived snapshot memoization, non-memoized unsafe derived cases, and persisted registry file invalidation. (src/plugins/plugin-metadata-snapshot.memo.test.ts:113, 9de96674aff0)
  • Vanished recovered record handling: The registry snapshot change now ignores recovered install records missing from the persisted index when their install/source paths no longer exist, with a focused persisted fast-path test. (src/plugins/plugin-registry-snapshot.ts:197, 9de96674aff0)
  • Architecture boundary: The plugin architecture docs allow carrying an explicit PluginMetadataSnapshot for a current flow and emphasize snapshot replacement rather than unbounded hidden metadata caches; this PR uses a single-entry process memo with explicit invalidation inputs. Public docs: docs/plugins/architecture.md. (docs/plugins/architecture.md:150, 462a05621014)
  • Prior review blockers repaired: The author’s follow-up identifies commits 1a4b611, d684bfd, and 9de9667 as repairing the previous mock, fixture, and lint findings; the current raw files reflect those repairs. (9de96674aff0)

Likely related people:

  • steipete: Recent main history shows repeated work on plugin metadata snapshot reuse, cache invalidation, startup metadata planning, and registry snapshot behavior. (role: feature-history contributor; confidence: high; commits: 48ff3909531f, 97a34e0f5076, 440fc73448ac; files: src/plugins/plugin-metadata-snapshot.ts, src/plugins/plugin-registry-snapshot.ts, src/plugins/current-plugin-metadata-snapshot.ts)
  • shakkernerd: Recent main history includes diagnostics timeline propagation and multiple plugin metadata reuse/fingerprint fixes adjacent to this hot path. (role: recent area contributor; confidence: high; commits: c795a1a8ef19, a7cc9e8a56fe, 3dffef651b87; files: src/plugins/plugin-metadata-snapshot.ts, src/plugins/current-plugin-metadata-snapshot.ts)
  • vincentkoc: Current blame and recent commits show nearby plugin registry/snapshot maintenance, and the PR timeline shows vincentkoc assigned to this review path. (role: recent adjacent contributor; confidence: medium; commits: c635f0087ec8, 2e593d07f706, 8e1755928c6a; files: src/plugins/plugin-registry-snapshot.ts, src/plugins/plugin-metadata-snapshot.ts)

Remaining risk / open question:

  • The real timing proof was collected on fb8e1ce; current head 9de9667 only has the stated memo-key simplification and test trims plus green hosted checks, but was not retimed in the same beta6 install.
  • The PR intentionally leaves broader same-process arbitrary plugin-file edit freshness to fix(plugins): keep plugin metadata memo fresh #81064 or a follow-up.

Codex review notes: model gpt-5.5, reasoning high; reviewed against 462a05621014.

@Kaspre Kaspre force-pushed the perf/plugin-metadata-snapshot-process-memo-slim branch from 07786f8 to 61bf711 Compare May 14, 2026 00:03
@Kaspre Kaspre force-pushed the perf/plugin-metadata-snapshot-process-memo-slim branch from 95fa019 to 3d05063 Compare May 14, 2026 05:21
@clawsweeper clawsweeper Bot added the proof: sufficient ClawSweeper judged the real behavior proof convincing. label May 14, 2026
@Kaspre Kaspre force-pushed the perf/plugin-metadata-snapshot-process-memo-slim branch from 3d05063 to f592933 Compare May 14, 2026 05:36
@clawsweeper clawsweeper Bot removed the proof: sufficient ClawSweeper judged the real behavior proof convincing. label May 14, 2026
@Kaspre Kaspre force-pushed the perf/plugin-metadata-snapshot-process-memo-slim branch 4 times, most recently from fa3cddf to ba05b66 Compare May 14, 2026 05:55
@Kaspre Kaspre force-pushed the perf/plugin-metadata-snapshot-process-memo-slim branch from ba05b66 to 676e3df Compare May 14, 2026 06:07
@Kaspre Kaspre force-pushed the perf/plugin-metadata-snapshot-process-memo-slim branch 2 times, most recently from a076fb3 to 81edbf3 Compare May 14, 2026 06:28
@clawsweeper clawsweeper Bot added the proof: sufficient ClawSweeper judged the real behavior proof convincing. label May 14, 2026
@Kaspre Kaspre force-pushed the perf/plugin-metadata-snapshot-process-memo-slim branch from 81edbf3 to 9169cf3 Compare May 14, 2026 06:52
@openclaw-barnacle openclaw-barnacle Bot removed the proof: sufficient ClawSweeper judged the real behavior proof convincing. label May 14, 2026
@clawsweeper clawsweeper Bot added the proof: sufficient ClawSweeper judged the real behavior proof convincing. label May 14, 2026
@Kaspre Kaspre force-pushed the perf/plugin-metadata-snapshot-process-memo-slim branch from 9169cf3 to be937da Compare May 14, 2026 07:05
@Kaspre Kaspre force-pushed the perf/plugin-metadata-snapshot-process-memo-slim branch from be937da to fb8e1ce Compare May 14, 2026 07:08
@openclaw-barnacle openclaw-barnacle Bot added size: M and removed size: L proof: sufficient ClawSweeper judged the real behavior proof convincing. labels May 14, 2026
@vincentkoc vincentkoc self-assigned this May 14, 2026
@clawsweeper clawsweeper Bot added the proof: sufficient ClawSweeper judged the real behavior proof convincing. label May 14, 2026
@Kaspre Kaspre marked this pull request as ready for review May 14, 2026 07:38
@openclaw-barnacle openclaw-barnacle Bot removed the proof: sufficient ClawSweeper judged the real behavior proof convincing. label May 14, 2026

Kaspre commented May 14, 2026

Copy link
Copy Markdown
Contributor Author

Follow-up on the earlier clawsweeper P2s:

  • Config-path mock breakage is addressed in 1a4b611d22 by removing the new resolveIsNixMode import from the memo-key path.
  • The InstalledPluginIndex fixture is completed in d684bfd59e.
  • The lint-only non-null assertion failures are removed in 9de96674af.

Current head 9de96674aff0415862d1ef88be3f5722a53e61a0 is non-draft, size: M, mergeable, and hosted checks are green after the PR body refresh, including Real behavior proof, check-lint, check-test-types, check-prod-types, build-artifacts/build-smoke, checks-node-core, checks-node-core-fast, Critical Quality (plugin-boundary), Critical Quality (network-runtime-boundary), and the selected Security High shards.

@clawsweeper clawsweeper Bot added the proof: sufficient ClawSweeper judged the real behavior proof convincing. label May 14, 2026
@vincentkoc

Copy link
Copy Markdown
Member

Thanks @Kaspre. I landed the cleaned version directly on main in f71df80 with your git authorship preserved and changelog credit included.

Proof on the landed commit:

  • node scripts/run-vitest.mjs run src/plugins/plugin-metadata-snapshot.memo.test.ts src/plugins/plugin-registry-snapshot.test.ts src/plugins/plugin-registry.test.ts --maxWorkers=1 --reporter=dot passed locally: 3 files, 37 tests
  • Testbox tbx_01krjrqyzsx903ktfwg1gz6bhw, Actions run https://github.com/openclaw/openclaw/actions/runs/25849493523, pnpm check:changed passed
  • dependency pin guard checked 429 direct dependency specs with 0 violations

This will be included in the next release. Closing this PR as superseded by the landed main commit.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

proof: sufficient ClawSweeper judged the real behavior proof convincing. proof: supplied External PR includes structured after-fix real behavior proof. size: M

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants