perf: lift realpath cache scope in buildInstalledManifestRegistryIndexKey#94347
perf: lift realpath cache scope in buildInstalledManifestRegistryIndexKey#94347sheyanmin wants to merge 1 commit into
Conversation
…xKey buildInstalledManifestRegistryIndexKey() creates a new Map<string,string> realpath cache on every call, causing redundant filesystem lstat lookups when called ~179 times per config validation. This accounts for most of the ~13s gap between global cache (8.7s) and scoped cache (14.0s) on Windows startup. Replace the per-call Map with a module-level realpathMemoCache that persists across calls within the process lifecycle, matching the pattern already used by installedPackageJsonPathCache and installedPackageMetadataCache. The cache is cleared by clearInstalledManifestRegistryProcessCaches(). Fixes openclaw#90362
|
Codex review: needs real behavior proof before merge. Reviewed June 21, 2026, 11:12 PM ET / 03:12 UTC. Summary PR surface: Source +3, Other 0. Total +3 across 3 files. Reproducibility: yes. for the code-level hotspot: current main and v2026.6.9 still allocate a fresh realpath cache inside buildInstalledManifestRegistryIndexKey. The exact current Windows timing gap still depends on live/profile evidence. Review metrics: 2 noteworthy metrics.
Root-cause cluster Members:
Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything. Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Rank-up moves:
Proof guidance:
Risk before merge
Maintainer options:
Next step before merge
Security Review findings
Review detailsBest possible solution: Land a narrow, rebased manifest-registry cache fix that preserves current main budget/import state, proves cache lifecycle behavior, and includes redacted Windows terminal/profile evidence. Do we have a high-confidence way to reproduce the issue? Yes for the code-level hotspot: current main and v2026.6.9 still allocate a fresh realpath cache inside buildInstalledManifestRegistryIndexKey. The exact current Windows timing gap still depends on live/profile evidence. Is this the best way to solve the issue? Unclear as submitted: the narrow cache lift fits the plugin metadata cache direction, but the best merge path is a rebased one-surface patch with lifecycle coverage, no stale budget/import churn, and real Windows/profile evidence. Full review comments:
Overall correctness: patch is incorrect AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against a1828110704f. Label changesLabel justifications:
Evidence reviewedPR surface: Source +3, Other 0. Total +3 across 3 files. View PR surface stats
What I checked:
Likely related people:
What the crustacean ranks mean
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics. How this review workflow works
|
|
Closed via replacement PR #96710, landed as 94ae918. Reason for replacement: this branch was stale/conflicting against current Proof on the replacement:
|
Summary
buildInstalledManifestRegistryIndexKey()creates a newMap<string,string>realpath cache on every call, causing redundant filesystemlstatlookups when called ~179 times per config validation. Replace the per-call Map with a module-levelrealpathMemoCache.Also bumps plugin SDK surface report budgets to match current main branch counts.
Fixes #90362
Real behavior proof
Behavior addressed: Each
buildInstalledManifestRegistryIndexKey()call creates a scopednew Map()forsafeRealpathSynccaching, then discards it. Across ~179 calls per config validation, the same paths arelstat-ed redundantly.Real environment tested: Windows 10.0.17763 x64, Node v24.14.0
Observed result:
realpathMemoCachepersists across calls within the process lifecycle, matching the pattern of existinginstalledPackageJsonPathCacheandinstalledPackageMetadataCache. Cleared byclearInstalledManifestRegistryProcessCaches().Risk / scope