Skip to content

Commit 00816c0

Browse files
sunvembersunvember
committed
perf: skip packageManager.resolve() by loading extensionFactories directly
## Problem The previous PR (#82523) attempted to cache DefaultResourceLoader instances, but this approach was fundamentally flawed because: 1. The bottleneck is NOT in the constructor (which only assigns properties) 2. The bottleneck is in `reload()` → `packageManager.resolve()` 3. `resolve()` scans many directories (~/.pi/*, ~/.agents/skills, ancestor .agents/skills up to git repo root) 4. Caching the loader doesn't help because callers always call `await loader.reload()` 5. The cached loader's extensionFactories closure captured stale values ## Root Cause Analysis When all `no*` flags are true: - `resolve()` still executes all filesystem scans (expensive) - But the results are discarded (empty arrays for skills/extensions/prompts/themes) - Only `loadExtensionFactories()` produces useful output The `DefaultResourceLoader` constructor initializes: - `extensionsResult = { extensions: [], errors: [], runtime: createExtensionRuntime() }` - `skills = [], prompts = [], themes = [], agentsFiles = []` - `systemPrompt = undefined` These match `reload()` output when `no*` flags are true, except `systemPrompt`. OpenClaw overrides `systemPrompt` via `applySystemPromptOverrideToSession()`, so the undefined value is harmless. ## Solution Instead of caching loaders, we: 1. Create a new loader with `no*` flags true 2. Call `loadExtensionFactories()` directly (public method) 3. Skip `reload()` entirely This eliminates the 5-9 second `resolve()` overhead while still loading inline extensions needed for compaction safeguards and tool middleware. ## Changes - `resource-loader.ts`: Changed to async function, loads extensionFactories directly without calling reload() - `attempt.ts`: Use `await createEmbeddedPiResourceLoader()` + remove reload() - `compact.ts`: Same pattern - `resource-loader.test.ts`: Rewritten to match new behavior ## Performance Impact Before: 5-9 seconds overhead per embedded run After: ~milliseconds (only extension factory loading, no filesystem scans) Files changed: - src/agents/pi-embedded-runner/resource-loader.ts - src/agents/pi-embedded-runner/resource-loader.test.ts - src/agents/pi-embedded-runner/compact.ts - src/agents/pi-embedded-runner/run/attempt.ts
1 parent 559095a commit 00816c0

4 files changed

Lines changed: 142 additions & 313 deletions

File tree

‎src/agents/pi-embedded-runner/compact.ts‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1012,13 +1012,15 @@ async function compactEmbeddedPiSessionDirectOnce(
10121012
modelId,
10131013
model,
10141014
});
1015-
const resourceLoader = createEmbeddedPiResourceLoader({
1015+
// Create resource loader with fast-path: skip packageManager.resolve()
1016+
const resourceLoader = await createEmbeddedPiResourceLoader({
10161017
cwd: resolvedWorkspace,
10171018
agentDir,
10181019
settingsManager,
10191020
extensionFactories,
10201021
});
1021-
await resourceLoader.reload();
1022+
// No reload() needed - constructor state matches reload() output for no* flags,
1023+
// and OpenClaw overrides systemPrompt via applySystemPromptOverrideToSession.
10221024
markResourceLoaderReloaded(resolvedWorkspace, agentDir);
10231025
// DefaultResourceLoader.reload() rehydrates settings from disk and can drop OpenClaw
10241026
// compaction overrides applied in createPreparedEmbeddedPiSettingsManager — same

‎src/agents/pi-embedded-runner/resource-loader.test.ts‎

Lines changed: 83 additions & 175 deletions
Original file line numberDiff line numberDiff line change
@@ -2,42 +2,44 @@ import { DefaultResourceLoader } from "@earendil-works/pi-coding-agent";
22
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
33
import {
44
createEmbeddedPiResourceLoader,
5+
createEmbeddedPiResourceLoaderSync,
56
EMBEDDED_PI_RESOURCE_LOADER_DISCOVERY_OPTIONS,
6-
getResourceLoaderCacheSize,
7-
invalidateResourceLoaderCache,
87
markResourceLoaderReloaded,
9-
pruneResourceLoaderCache,
108
} from "./resource-loader.js";
119

10+
// Mock DefaultResourceLoader to track construction and method calls
11+
const mockLoadExtensionFactories = vi.fn(async (runtime: unknown) => ({
12+
extensions: [],
13+
errors: [],
14+
}));
15+
1216
vi.mock("@earendil-works/pi-coding-agent", () => ({
1317
DefaultResourceLoader: vi.fn(function DefaultResourceLoader(
1418
this: Record<string, unknown>,
1519
options: unknown,
1620
) {
1721
Object.assign(this, {
1822
options,
19-
reload: vi.fn(async () => undefined),
23+
extensionsResult: { extensions: [], errors: [], runtime: {} },
24+
loadExtensionFactories: mockLoadExtensionFactories,
2025
});
2126
}),
2227
}));
2328

2429
describe("createEmbeddedPiResourceLoader", () => {
2530
beforeEach(() => {
26-
// Clear cache before each test
27-
invalidateResourceLoaderCache();
28-
// Reset mock call count
2931
vi.clearAllMocks();
3032
});
3133

3234
afterEach(() => {
33-
invalidateResourceLoaderCache();
35+
vi.clearAllMocks();
3436
});
3537

36-
it("keeps inline extensions but disables Pi filesystem discovery", () => {
38+
it("passes correct options to DefaultResourceLoader including discovery flags", async () => {
3739
const settingsManager = {};
3840
const extensionFactories = [vi.fn()];
3941

40-
createEmbeddedPiResourceLoader({
42+
await createEmbeddedPiResourceLoader({
4143
cwd: "/workspace",
4244
agentDir: "/agent",
4345
settingsManager: settingsManager as never,
@@ -53,230 +55,136 @@ describe("createEmbeddedPiResourceLoader", () => {
5355
});
5456
});
5557

56-
it("caches resource loader for repeated calls with same cwd/agentDir", () => {
58+
it("calls loadExtensionFactories directly without reload()", async () => {
5759
const settingsManager = {};
5860

59-
// First call creates new loader
60-
const loader1 = createEmbeddedPiResourceLoader({
61-
cwd: "/workspace",
62-
agentDir: "/agent",
63-
settingsManager: settingsManager as never,
64-
extensionFactories: [],
65-
});
66-
67-
// Second call should return cached loader (no new DefaultResourceLoader call)
68-
const loader2 = createEmbeddedPiResourceLoader({
61+
await createEmbeddedPiResourceLoader({
6962
cwd: "/workspace",
7063
agentDir: "/agent",
7164
settingsManager: settingsManager as never,
7265
extensionFactories: [],
7366
});
7467

75-
// Should be same instance
76-
expect(loader1).toBe(loader2);
77-
// Should only create one DefaultResourceLoader
78-
expect(DefaultResourceLoader).toHaveBeenCalledTimes(1);
68+
// Should call loadExtensionFactories directly
69+
expect(mockLoadExtensionFactories).toHaveBeenCalledTimes(1);
70+
// Should not have a reload method called (we skip it entirely)
71+
// The mock doesn't have reload, so if it was called it would throw
7972
});
8073

81-
it("creates separate loaders for different workspaces", () => {
82-
const loader1 = createEmbeddedPiResourceLoader({
83-
cwd: "/workspace1",
84-
agentDir: "/agent",
85-
settingsManager: {} as never,
86-
extensionFactories: [],
74+
it("loads inline extensionFactories into extensionsResult", async () => {
75+
const extensionFactories = [vi.fn(), vi.fn()];
76+
const mockExtensions = [{ name: "test-extension" }];
77+
78+
mockLoadExtensionFactories.mockResolvedValueOnce({
79+
extensions: mockExtensions,
80+
errors: [],
8781
});
8882

89-
const loader2 = createEmbeddedPiResourceLoader({
90-
cwd: "/workspace2",
83+
const loader = await createEmbeddedPiResourceLoader({
84+
cwd: "/workspace",
9185
agentDir: "/agent",
9286
settingsManager: {} as never,
93-
extensionFactories: [],
87+
extensionFactories: extensionFactories as never,
9488
});
9589

96-
// Should be different instances
97-
expect(loader1).not.toBe(loader2);
98-
// Should create two DefaultResourceLoader
99-
expect(DefaultResourceLoader).toHaveBeenCalledTimes(2);
90+
// Extensions should be loaded into extensionsResult
91+
expect((loader as any).extensionsResult.extensions).toEqual(mockExtensions);
10092
});
10193

102-
it("creates separate loaders for different agentDirs", () => {
103-
const loader1 = createEmbeddedPiResourceLoader({
104-
cwd: "/workspace",
105-
agentDir: "/agent1",
106-
settingsManager: {} as never,
107-
extensionFactories: [],
94+
it("accumulates errors from loadExtensionFactories", async () => {
95+
const mockErrors = [{ path: "<inline:1>", error: "Failed to load" }];
96+
97+
mockLoadExtensionFactories.mockResolvedValueOnce({
98+
extensions: [],
99+
errors: mockErrors,
108100
});
109101

110-
const loader2 = createEmbeddedPiResourceLoader({
102+
const loader = await createEmbeddedPiResourceLoader({
111103
cwd: "/workspace",
112-
agentDir: "/agent2",
113-
settingsManager: {} as never,
114-
extensionFactories: [],
115-
});
116-
117-
expect(loader1).not.toBe(loader2);
118-
expect(DefaultResourceLoader).toHaveBeenCalledTimes(2);
119-
});
120-
121-
it("cache size tracks number of cached loaders", () => {
122-
expect(getResourceLoaderCacheSize()).toBe(0);
123-
124-
createEmbeddedPiResourceLoader({
125-
cwd: "/workspace1",
126104
agentDir: "/agent",
127105
settingsManager: {} as never,
128-
extensionFactories: [],
106+
extensionFactories: [vi.fn()] as never,
129107
});
130-
expect(getResourceLoaderCacheSize()).toBe(1);
131108

132-
createEmbeddedPiResourceLoader({
133-
cwd: "/workspace2",
134-
agentDir: "/agent",
135-
settingsManager: {} as never,
136-
extensionFactories: [],
137-
});
138-
expect(getResourceLoaderCacheSize()).toBe(2);
139-
140-
// Same workspace should not increase cache size
141-
createEmbeddedPiResourceLoader({
142-
cwd: "/workspace1",
143-
agentDir: "/agent",
144-
settingsManager: {} as never,
145-
extensionFactories: [],
146-
});
147-
expect(getResourceLoaderCacheSize()).toBe(2);
109+
expect((loader as any).extensionsResult.errors).toEqual(mockErrors);
148110
});
149111

150-
it("invalidateResourceLoaderCache clears all entries", () => {
151-
createEmbeddedPiResourceLoader({
152-
cwd: "/workspace1",
153-
agentDir: "/agent",
154-
settingsManager: {} as never,
155-
extensionFactories: [],
156-
});
157-
createEmbeddedPiResourceLoader({
158-
cwd: "/workspace2",
112+
it("is async to allow extension factory loading", async () => {
113+
// createEmbeddedPiResourceLoader returns a Promise
114+
const result = createEmbeddedPiResourceLoader({
115+
cwd: "/workspace",
159116
agentDir: "/agent",
160117
settingsManager: {} as never,
161118
extensionFactories: [],
162119
});
163-
expect(getResourceLoaderCacheSize()).toBe(2);
164-
165-
invalidateResourceLoaderCache();
166-
expect(getResourceLoaderCacheSize()).toBe(0);
167120

168-
// After invalidate, should create new loader
169-
createEmbeddedPiResourceLoader({
170-
cwd: "/workspace1",
171-
agentDir: "/agent",
172-
settingsManager: {} as never,
173-
extensionFactories: [],
174-
});
175-
expect(DefaultResourceLoader).toHaveBeenCalledTimes(3);
121+
expect(result).toBeInstanceOf(Promise);
122+
await result; // Should resolve without error
176123
});
124+
});
177125

178-
it("invalidateResourceLoaderCache with specific cwd/agentDir only removes that entry", () => {
179-
createEmbeddedPiResourceLoader({
180-
cwd: "/workspace1",
181-
agentDir: "/agent1",
182-
settingsManager: {} as never,
183-
extensionFactories: [],
184-
});
185-
createEmbeddedPiResourceLoader({
186-
cwd: "/workspace2",
187-
agentDir: "/agent2",
188-
settingsManager: {} as never,
189-
extensionFactories: [],
190-
});
191-
expect(getResourceLoaderCacheSize()).toBe(2);
192-
193-
invalidateResourceLoaderCache("/workspace1", "/agent1");
194-
expect(getResourceLoaderCacheSize()).toBe(1);
195-
196-
// workspace1 should create new loader
197-
createEmbeddedPiResourceLoader({
198-
cwd: "/workspace1",
199-
agentDir: "/agent1",
200-
settingsManager: {} as never,
201-
extensionFactories: [],
202-
});
203-
expect(DefaultResourceLoader).toHaveBeenCalledTimes(3);
204-
205-
// workspace2 should still use cached loader
206-
createEmbeddedPiResourceLoader({
207-
cwd: "/workspace2",
208-
agentDir: "/agent2",
209-
settingsManager: {} as never,
210-
extensionFactories: [],
211-
});
212-
expect(DefaultResourceLoader).toHaveBeenCalledTimes(3); // No new call
126+
describe("createEmbeddedPiResourceLoaderSync", () => {
127+
beforeEach(() => {
128+
vi.clearAllMocks();
213129
});
214130

215-
it("markResourceLoaderReloaded updates lastReloadAt timestamp", () => {
216-
createEmbeddedPiResourceLoader({
131+
it("creates loader synchronously without extensionFactories", () => {
132+
const loader = createEmbeddedPiResourceLoaderSync({
217133
cwd: "/workspace",
218134
agentDir: "/agent",
219135
settingsManager: {} as never,
220-
extensionFactories: [],
221136
});
222137

223-
// Should not throw
224-
markResourceLoaderReloaded("/workspace", "/agent");
138+
expect(loader).toBeDefined();
139+
expect(DefaultResourceLoader).toHaveBeenCalledTimes(1);
225140
});
226141

227-
it("pruneResourceLoaderCache removes expired entries when TTL env is set", async () => {
228-
// Set very short TTL for test
229-
process.env.OPENCLAW_RESOURCE_LOADER_CACHE_TTL_MS = "100";
230-
231-
createEmbeddedPiResourceLoader({
142+
it("does not call loadExtensionFactories for sync version", () => {
143+
createEmbeddedPiResourceLoaderSync({
232144
cwd: "/workspace",
233145
agentDir: "/agent",
234146
settingsManager: {} as never,
235-
extensionFactories: [],
236147
});
237-
expect(getResourceLoaderCacheSize()).toBe(1);
238-
239-
// Wait for TTL to expire
240-
await new Promise((resolve) => setTimeout(resolve, 150));
241-
242-
pruneResourceLoaderCache();
243-
expect(getResourceLoaderCacheSize()).toBe(0);
244148

245-
// After prune, should create new loader
246-
createEmbeddedPiResourceLoader({
247-
cwd: "/workspace",
248-
agentDir: "/agent",
249-
settingsManager: {} as never,
250-
extensionFactories: [],
251-
});
252-
expect(DefaultResourceLoader).toHaveBeenCalledTimes(2);
253-
254-
// Clean up env
255-
delete process.env.OPENCLAW_RESOURCE_LOADER_CACHE_TTL_MS;
149+
// Sync version skips extension loading
150+
expect(mockLoadExtensionFactories).not.toHaveBeenCalled();
256151
});
257152

258-
it("respects OPENCLAW_RESOURCE_LOADER_CACHE_TTL_MS env var", () => {
259-
// Set TTL to 10 seconds (minimum)
260-
process.env.OPENCLAW_RESOURCE_LOADER_CACHE_TTL_MS = "10000";
261-
262-
createEmbeddedPiResourceLoader({
153+
it("passes empty extensionFactories array", () => {
154+
createEmbeddedPiResourceLoaderSync({
263155
cwd: "/workspace",
264156
agentDir: "/agent",
265157
settingsManager: {} as never,
266-
extensionFactories: [],
267158
});
268-
expect(DefaultResourceLoader).toHaveBeenCalledTimes(1);
269159

270-
// Immediate second call should use cache
271-
createEmbeddedPiResourceLoader({
160+
expect(DefaultResourceLoader).toHaveBeenCalledWith({
272161
cwd: "/workspace",
273162
agentDir: "/agent",
274-
settingsManager: {} as never,
163+
settingsManager: {},
275164
extensionFactories: [],
165+
...EMBEDDED_PI_RESOURCE_LOADER_DISCOVERY_OPTIONS,
276166
});
277-
expect(DefaultResourceLoader).toHaveBeenCalledTimes(1);
167+
});
168+
});
278169

279-
// Clean up env
280-
delete process.env.OPENCLAW_RESOURCE_LOADER_CACHE_TTL_MS;
170+
describe("EMBEDDED_PI_RESOURCE_LOADER_DISCOVERY_OPTIONS", () => {
171+
it("disables all filesystem discovery", () => {
172+
expect(EMBEDDED_PI_RESOURCE_LOADER_DISCOVERY_OPTIONS).toEqual({
173+
noExtensions: true,
174+
noSkills: true,
175+
noPromptTemplates: true,
176+
noThemes: true,
177+
noContextFiles: true,
178+
});
281179
});
282180
});
181+
182+
describe("markResourceLoaderReloaded", () => {
183+
it("is a no-op that does not throw", () => {
184+
// Should not throw for any arguments
185+
markResourceLoaderReloaded("/workspace", "/agent");
186+
markResourceLoaderReloaded("", "");
187+
markResourceLoaderReloaded("any", "path");
188+
expect(true).toBe(true); // Explicit pass
189+
});
190+
});

0 commit comments

Comments
 (0)