perf: cache resourceLoader across embedded runs to eliminate 5-9s reload overhead#82523
perf: cache resourceLoader across embedded runs to eliminate 5-9s reload overhead#82523sunvember wants to merge 1 commit into
Conversation
…oad overhead DefaultResourceLoader construction triggers packageManager.resolve() which scans skills directories and blocks the event loop for 5-9 seconds on every embedded run. This is pure overhead when the same workspace/agent is reused across consecutive runs (normal session flow). Solution: - Add in-memory cache keyed by (cwd, agentDir) in resource-loader.ts - Cache TTL defaults to 60s, configurable via OPENCLAW_RESOURCE_LOADER_CACHE_TTL_MS - add markResourceLoaderReloaded() to track reload completion - Add pruneResourceLoaderCache() for periodic cleanup - Add invalidateResourceLoaderCache() for targeted or full cache invalidation - Call markResourceLoaderReloaded in attempt.ts and compact.ts after reload() Files changed: - resource-loader.ts: cache logic - resource-loader.test.ts: 10 new test cases - attempt.ts: call markResourceLoaderReloaded after reload - compact.ts: call markResourceLoaderReloaded after reload AI-assisted development
openclaw-barnacle
Bot
added
agents
|
Codex review: needs real behavior proof before merge. Summary Reproducibility: no. high-confidence real benchmark reproduction was provided. Source inspection is high-confidence that the PR still calls the upstream Real behavior proof Next step before merge Security Review findings
Review detailsBest possible solution: Keep the current fresh-loader behavior unless a benchmarked optimization caches the actual expensive work while preserving per-run settings, extension factory inputs, invalidation, and real timing proof. Do we have a high-confidence way to reproduce the issue? No high-confidence real benchmark reproduction was provided. Source inspection is high-confidence that the PR still calls the upstream Is this the best way to solve the issue? No. This caches the wrong level of state, still calls the expensive reload path, and adds a TTL env knob without proof; a safer solution would benchmark the hotspot and cache only immutable resolved resources or reuse loaders with a complete input key and explicit invalidation. Full review comments:
Overall correctness: patch is incorrect Acceptance criteria:
What I checked:
Likely related people:
Remaining risk / open question:
Codex review notes: model gpt-5.5, reasoning high; reviewed against 9c5acb7ea363. |
|
Closing: after deeper analysis, the caching approach was fundamentally flawed. DefaultResourceLoader constructor is cheap — the expensive work (packageManager.resolve()) happens in reload(), which is called every time. Caching the loader instance doesn't avoid the bottleneck. See detailed analysis in the thread. Will redesign and submit a new PR. |
1 participant
Problem
DefaultResourceLoader construction triggers packageManager.resolve() which scans skills directories and blocks the event loop for 5-9 seconds on every embedded run. This is pure overhead when the same workspace/agent is reused across consecutive runs (normal session flow).
Solution
Add in-memory cache keyed by (cwd, agentDir) in resource-loader.ts:
Files Changed
Checklist