Skip to content

Commit 5b6d03e

Browse files
steipetesteipete
authored
perf: reduce runtime cache churn
Reduce hot-path cache churn by reusing the active plugin metadata snapshot for manifest model-id normalization when safe, and by avoiding repeated JSON reparses for cached session stores while preserving clone semantics. Verification: - pnpm exec oxfmt --check src/plugins/manifest-model-id-normalization.ts src/plugins/manifest-model-id-normalization.test.ts src/config/sessions/store-cache.ts src/config/sessions.cache.test.ts - node scripts/run-vitest.mjs src/config/sessions.cache.test.ts src/plugins/manifest-model-id-normalization.test.ts src/gateway/session-utils.subagent.test.ts - pnpm tsgo:core - autoreview clean - PR CI green
1 parent 0d4575a commit 5b6d03e

5 files changed

Lines changed: 134 additions & 31 deletions

File tree

‎src/config/sessions.cache.test.ts‎

Lines changed: 27 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -173,6 +173,16 @@ describe("Session Store Cache", () => {
173173
parseSpy.mockRestore();
174174
});
175175

176+
it("keeps disk-loaded clone:false cache hits by reference", () => {
177+
const testStore = createSingleSessionStore();
178+
fs.writeFileSync(storePath, JSON.stringify(testStore), "utf8");
179+
180+
const loaded1 = loadSessionStore(storePath, { clone: false });
181+
const loaded2 = loadSessionStore(storePath, { clone: false });
182+
183+
expect(loaded2["session:1"]).toBe(loaded1["session:1"]);
184+
});
185+
176186
it("does not cache pre-migration or pre-normalization disk JSON", () => {
177187
fs.writeFileSync(
178188
storePath,
@@ -235,7 +245,7 @@ describe("Session Store Cache", () => {
235245
structuredCloneSpy.mockRestore();
236246
});
237247

238-
it("does not parse serialized stores when writing the cache", () => {
248+
it("does not parse serialized stores when writing or reading object-cache hits", () => {
239249
const testStore = createSingleSessionStore(
240250
createSessionEntry({
241251
origin: { provider: "openai" },
@@ -252,11 +262,26 @@ describe("Session Store Cache", () => {
252262
const cached = readSessionStoreCache({ storePath });
253263

254264
expect(cached?.["session:1"].origin?.provider).toBe("openai");
255-
expect(parseSpy).toHaveBeenCalledTimes(1);
265+
expect(parseSpy).not.toHaveBeenCalled();
256266

257267
parseSpy.mockRestore();
258268
});
259269

270+
it("clones cached session records without invoking prototype setters", () => {
271+
const testStore = JSON.parse(
272+
`{"session:1":{"sessionId":"id-1","updatedAt":${Date.now()},"displayName":"Test Session 1","__proto__":{"polluted":true}}}`,
273+
) as Record<string, SessionEntry>;
274+
275+
writeSessionStoreCache({ storePath, store: testStore });
276+
const cached = readSessionStoreCache({ storePath });
277+
const entry = cached?.["session:1"] as (SessionEntry & { polluted?: boolean }) | undefined;
278+
279+
expect(entry).toBeDefined();
280+
expect(entry?.polluted).toBeUndefined();
281+
expect(Object.prototype.hasOwnProperty.call(entry, "__proto__")).toBe(true);
282+
expect(Object.prototype).not.toHaveProperty("polluted");
283+
});
284+
260285
it("clones disk-loaded stores from the raw serialized JSON", () => {
261286
const testStore = createSingleSessionStore(
262287
createSessionEntry({

‎src/config/sessions/store-cache.ts‎

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -179,7 +179,10 @@ export function cloneSessionStoreRecord(
179179
store: Record<string, SessionEntry>,
180180
serialized?: string,
181181
): Record<string, SessionEntry> {
182-
const cloned = JSON.parse(serialized ?? JSON.stringify(store)) as Record<string, SessionEntry>;
182+
const cloned =
183+
serialized === undefined
184+
? cloneJsonLikeValue(store)
185+
: (JSON.parse(serialized) as Record<string, SessionEntry>);
183186
internSessionStoreLargeStrings(cloned);
184187
return cloned;
185188
}
@@ -193,7 +196,12 @@ function cloneJsonLikeValue<T>(value: T): T {
193196
}
194197
const cloned: Record<string, unknown> = {};
195198
for (const [key, child] of Object.entries(value as Record<string, unknown>)) {
196-
cloned[key] = cloneJsonLikeValue(child);
199+
Object.defineProperty(cloned, key, {
200+
value: cloneJsonLikeValue(child),
201+
enumerable: true,
202+
configurable: true,
203+
writable: true,
204+
});
197205
}
198206
return cloned as T;
199207
}
@@ -342,7 +350,7 @@ export function readSessionStoreCache(params: {
342350
if (params.clone === false) {
343351
return cached.store;
344352
}
345-
return cloneSessionStoreRecord(cached.store, cached.serialized);
353+
return cloneSessionStoreRecord(cached.store);
346354
}
347355

348356
export function takeMutableSessionStoreCache(params: {
@@ -368,10 +376,11 @@ export function writeSessionStoreCache(params: {
368376
mtimeMs?: number;
369377
sizeBytes?: number;
370378
serialized?: string;
379+
takeOwnership?: boolean;
371380
}): void {
372381
const store =
373-
params.serialized === undefined ? cloneSessionStoreRecord(params.store) : params.store;
374-
if (params.serialized !== undefined) {
382+
params.takeOwnership === true ? params.store : cloneSessionStoreRecord(params.store);
383+
if (params.takeOwnership === true) {
375384
internSessionStoreLargeStrings(store);
376385
}
377386
SESSION_STORE_CACHE.set(params.storePath, {

‎src/config/sessions/store-load.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -448,6 +448,7 @@ export function loadSessionStore(
448448
mtimeMs,
449449
sizeBytes: fileStat?.sizeBytes,
450450
serialized: serializedFromDisk,
451+
takeOwnership: serializedFromDisk !== undefined,
451452
});
452453
}
453454

‎src/plugins/manifest-model-id-normalization.test.ts‎

Lines changed: 63 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import fs from "node:fs";
22
import os from "node:os";
33
import path from "node:path";
44
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
5+
import type { OpenClawConfig } from "../config/types.openclaw.js";
56
import {
67
clearCurrentPluginMetadataSnapshot,
78
resolvePluginMetadataControlPlaneFingerprint,
@@ -11,6 +12,7 @@ import { resolveInstalledPluginIndexPolicyHash } from "./installed-plugin-index-
1112
import type { InstalledPluginIndex } from "./installed-plugin-index.js";
1213
import { listOpenClawPluginManifestMetadata } from "./manifest-metadata-scan.js";
1314
import { normalizeProviderModelIdWithManifest } from "./manifest-model-id-normalization.js";
15+
import { clearPluginMetadataLifecycleCaches } from "./plugin-metadata-lifecycle.js";
1416
import type { PluginMetadataSnapshot } from "./plugin-metadata-snapshot.js";
1517
import { createEmptyPluginRegistry } from "./registry-empty.js";
1618
import { resetPluginRuntimeStateForTest, setActivePluginRegistry } from "./runtime.js";
@@ -87,8 +89,10 @@ function createCurrentSnapshot(params: {
8789
manifestHash: string;
8890
prefix: string;
8991
workspaceDir?: string;
92+
config?: OpenClawConfig;
9093
}): PluginMetadataSnapshot {
91-
const policyHash = resolveInstalledPluginIndexPolicyHash({});
94+
const config = params.config ?? {};
95+
const policyHash = resolveInstalledPluginIndexPolicyHash(config);
9296
const index: InstalledPluginIndex = {
9397
version: 1,
9498
hostContractVersion: "test-host",
@@ -119,15 +123,12 @@ function createCurrentSnapshot(params: {
119123
};
120124
return {
121125
policyHash,
122-
configFingerprint: resolvePluginMetadataControlPlaneFingerprint(
123-
{},
124-
{
125-
env: process.env,
126-
index,
127-
policyHash,
128-
workspaceDir: params.workspaceDir,
129-
},
130-
),
126+
configFingerprint: resolvePluginMetadataControlPlaneFingerprint(config, {
127+
env: process.env,
128+
index,
129+
policyHash,
130+
workspaceDir: params.workspaceDir,
131+
}),
131132
workspaceDir: params.workspaceDir,
132133
index,
133134
registryDiagnostics: [],
@@ -153,6 +154,17 @@ function normalizeDemoModel(modelId = "demo-model"): string | undefined {
153154
});
154155
}
155156

157+
function normalizeDemoModelWithEnv(
158+
env: NodeJS.ProcessEnv,
159+
modelId = "demo-model",
160+
): string | undefined {
161+
return normalizeProviderModelIdWithManifest({
162+
provider: "demo",
163+
env,
164+
context: { provider: "demo", modelId },
165+
});
166+
}
167+
156168
describe("manifest model id normalization", () => {
157169
beforeEach(() => {
158170
resetPluginRuntimeStateForTest();
@@ -234,6 +246,45 @@ describe("manifest model id normalization", () => {
234246
expect(normalizeDemoModel()).toBe("alpha/demo-model");
235247
});
236248

249+
it("reuses current metadata when callers omit config", () => {
250+
const config: OpenClawConfig = { plugins: { allow: ["normalizer"] } };
251+
setCurrentPluginMetadataSnapshot(
252+
createCurrentSnapshot({
253+
manifestHash: "alpha",
254+
prefix: "alpha",
255+
config,
256+
}),
257+
{ config, env: process.env },
258+
);
259+
260+
expect(normalizeDemoModel()).toBe("alpha/demo-model");
261+
});
262+
263+
it("validates explicit env before reusing current metadata", () => {
264+
setCurrentPluginMetadataSnapshot(
265+
createCurrentSnapshot({
266+
manifestHash: "alpha",
267+
prefix: "alpha",
268+
}),
269+
{ config: {}, env: process.env },
270+
);
271+
272+
const stateDir = makeTempDir();
273+
const pluginDir = path.join(stateDir, "extensions", "normalizer");
274+
writeInstallIndex({ stateDir, pluginDir });
275+
writeNormalizerManifest({ pluginDir, prefix: "bravo" });
276+
277+
const env = {
278+
...process.env,
279+
OPENCLAW_STATE_DIR: stateDir,
280+
OPENCLAW_HOME: undefined,
281+
OPENCLAW_DISABLE_BUNDLED_PLUGINS: "1",
282+
OPENCLAW_BUNDLED_PLUGINS_DIR: undefined,
283+
};
284+
285+
expect(normalizeDemoModelWithEnv(env)).toBe("bravo/demo-model");
286+
});
287+
237288
it("reflects manifest edits and state-dir changes on the next lookup", () => {
238289
const stateDirA = makeTempDir();
239290
const pluginDirA = path.join(stateDirA, "extensions", "normalizer");
@@ -248,6 +299,7 @@ describe("manifest model id normalization", () => {
248299
expect(normalizeDemoModel()).toBe("alpha/demo-model");
249300

250301
writeNormalizerManifest({ pluginDir: pluginDirA, prefix: "bravo-local" });
302+
clearPluginMetadataLifecycleCaches();
251303
expect(normalizeDemoModel()).toBe("bravo-local/demo-model");
252304

253305
const stateDirB = makeTempDir();
@@ -256,6 +308,7 @@ describe("manifest model id normalization", () => {
256308
writeNormalizerManifest({ pluginDir: pluginDirB, prefix: "charlie" });
257309

258310
process.env.OPENCLAW_STATE_DIR = stateDirB;
311+
clearPluginMetadataLifecycleCaches();
259312
expect(normalizeDemoModel()).toBe("charlie/demo-model");
260313
});
261314

‎src/plugins/manifest-model-id-normalization.ts‎

Lines changed: 29 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,9 @@
11
import type { OpenClawConfig } from "../config/types.openclaw.js";
22
import { normalizeLowercaseStringOrEmpty } from "../shared/string-coerce.js";
3+
import { getCurrentPluginMetadataSnapshot } from "./current-plugin-metadata-snapshot.js";
34
import type { PluginManifestRecord } from "./manifest-registry.js";
45
import type { PluginManifestModelIdNormalizationProvider } from "./manifest.js";
5-
import {
6-
resolvePluginMetadataSnapshot,
7-
type PluginMetadataSnapshot,
8-
} from "./plugin-metadata-snapshot.js";
6+
import { resolvePluginMetadataSnapshot } from "./plugin-metadata-snapshot.js";
97
import { getActivePluginRegistryWorkspaceDirFromState } from "./runtime-state.js";
108

119
type ManifestModelIdNormalizationLookupParams = {
@@ -37,19 +35,37 @@ let cachedPolicies: ManifestModelIdNormalizationPolicyCache | undefined;
3735
function resolveMetadataSnapshotForPolicies(
3836
params: ManifestModelIdNormalizationLookupParams = {},
3937
): {
40-
snapshot: PluginMetadataSnapshot;
38+
plugins: readonly Pick<PluginManifestRecord, "modelIdNormalization">[];
39+
configFingerprint?: string;
4140
cacheable: boolean;
4241
} {
4342
const env = params.env ?? process.env;
4443
const workspaceDir = params.workspaceDir ?? getActivePluginRegistryWorkspaceDirFromState();
45-
return {
46-
snapshot: resolvePluginMetadataSnapshot({
47-
config: params.config ?? {},
44+
if (params.config === undefined) {
45+
const currentSnapshot = getCurrentPluginMetadataSnapshot({
4846
env,
4947
workspaceDir,
50-
allowWorkspaceScopedCurrent: true,
51-
}),
52-
cacheable: true,
48+
allowWorkspaceScopedSnapshot: true,
49+
requireDefaultDiscoveryContext: params.env !== undefined,
50+
});
51+
if (currentSnapshot) {
52+
return {
53+
plugins: currentSnapshot.plugins,
54+
configFingerprint: currentSnapshot.configFingerprint,
55+
cacheable: true,
56+
};
57+
}
58+
}
59+
const snapshot = resolvePluginMetadataSnapshot({
60+
config: params.config ?? {},
61+
env,
62+
workspaceDir,
63+
allowWorkspaceScopedCurrent: true,
64+
});
65+
return {
66+
plugins: snapshot.plugins,
67+
configFingerprint: snapshot.configFingerprint,
68+
cacheable: false,
5369
};
5470
}
5571

@@ -59,12 +75,11 @@ function loadManifestModelIdNormalizationPolicies(
5975
if (params.plugins) {
6076
return collectManifestModelIdNormalizationPolicies(params.plugins);
6177
}
62-
const { snapshot, cacheable } = resolveMetadataSnapshotForPolicies(params);
63-
const configFingerprint = snapshot.configFingerprint;
78+
const { plugins, configFingerprint, cacheable } = resolveMetadataSnapshotForPolicies(params);
6479
if (cacheable && configFingerprint && cachedPolicies?.configFingerprint === configFingerprint) {
6580
return cachedPolicies.policies;
6681
}
67-
const policies = collectManifestModelIdNormalizationPolicies(snapshot.plugins);
82+
const policies = collectManifestModelIdNormalizationPolicies(plugins);
6883
if (cacheable && configFingerprint) {
6984
cachedPolicies = { configFingerprint, policies };
7085
}

0 commit comments

Comments
 (0)