From 5b6d03e3e2f145ffd7ffd1420ede003388ddd257 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Mon, 25 May 2026 22:40:46 +0100 Subject: [PATCH] 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 --- src/config/sessions.cache.test.ts | 29 +++++++- src/config/sessions/store-cache.ts | 19 +++-- src/config/sessions/store-load.ts | 1 + .../manifest-model-id-normalization.test.ts | 73 ++++++++++++++++--- .../manifest-model-id-normalization.ts | 43 +++++++---- 5 files changed, 134 insertions(+), 31 deletions(-) diff --git a/src/config/sessions.cache.test.ts b/src/config/sessions.cache.test.ts index 2c0fc459c2a..06bbb2c87be 100644 --- a/src/config/sessions.cache.test.ts +++ b/src/config/sessions.cache.test.ts @@ -173,6 +173,16 @@ describe("Session Store Cache", () => { parseSpy.mockRestore(); }); + it("keeps disk-loaded clone:false cache hits by reference", () => { + const testStore = createSingleSessionStore(); + fs.writeFileSync(storePath, JSON.stringify(testStore), "utf8"); + + const loaded1 = loadSessionStore(storePath, { clone: false }); + const loaded2 = loadSessionStore(storePath, { clone: false }); + + expect(loaded2["session:1"]).toBe(loaded1["session:1"]); + }); + it("does not cache pre-migration or pre-normalization disk JSON", () => { fs.writeFileSync( storePath, @@ -235,7 +245,7 @@ describe("Session Store Cache", () => { structuredCloneSpy.mockRestore(); }); - it("does not parse serialized stores when writing the cache", () => { + it("does not parse serialized stores when writing or reading object-cache hits", () => { const testStore = createSingleSessionStore( createSessionEntry({ origin: { provider: "openai" }, @@ -252,11 +262,26 @@ describe("Session Store Cache", () => { const cached = readSessionStoreCache({ storePath }); expect(cached?.["session:1"].origin?.provider).toBe("openai"); - expect(parseSpy).toHaveBeenCalledTimes(1); + expect(parseSpy).not.toHaveBeenCalled(); parseSpy.mockRestore(); }); + it("clones cached session records without invoking prototype setters", () => { + const testStore = JSON.parse( + `{"session:1":{"sessionId":"id-1","updatedAt":${Date.now()},"displayName":"Test Session 1","__proto__":{"polluted":true}}}`, + ) as Record; + + writeSessionStoreCache({ storePath, store: testStore }); + const cached = readSessionStoreCache({ storePath }); + const entry = cached?.["session:1"] as (SessionEntry & { polluted?: boolean }) | undefined; + + expect(entry).toBeDefined(); + expect(entry?.polluted).toBeUndefined(); + expect(Object.prototype.hasOwnProperty.call(entry, "__proto__")).toBe(true); + expect(Object.prototype).not.toHaveProperty("polluted"); + }); + it("clones disk-loaded stores from the raw serialized JSON", () => { const testStore = createSingleSessionStore( createSessionEntry({ diff --git a/src/config/sessions/store-cache.ts b/src/config/sessions/store-cache.ts index 1bf43199fa1..f148fb356a2 100644 --- a/src/config/sessions/store-cache.ts +++ b/src/config/sessions/store-cache.ts @@ -179,7 +179,10 @@ export function cloneSessionStoreRecord( store: Record, serialized?: string, ): Record { - const cloned = JSON.parse(serialized ?? JSON.stringify(store)) as Record; + const cloned = + serialized === undefined + ? cloneJsonLikeValue(store) + : (JSON.parse(serialized) as Record); internSessionStoreLargeStrings(cloned); return cloned; } @@ -193,7 +196,12 @@ function cloneJsonLikeValue(value: T): T { } const cloned: Record = {}; for (const [key, child] of Object.entries(value as Record)) { - cloned[key] = cloneJsonLikeValue(child); + Object.defineProperty(cloned, key, { + value: cloneJsonLikeValue(child), + enumerable: true, + configurable: true, + writable: true, + }); } return cloned as T; } @@ -342,7 +350,7 @@ export function readSessionStoreCache(params: { if (params.clone === false) { return cached.store; } - return cloneSessionStoreRecord(cached.store, cached.serialized); + return cloneSessionStoreRecord(cached.store); } export function takeMutableSessionStoreCache(params: { @@ -368,10 +376,11 @@ export function writeSessionStoreCache(params: { mtimeMs?: number; sizeBytes?: number; serialized?: string; + takeOwnership?: boolean; }): void { const store = - params.serialized === undefined ? cloneSessionStoreRecord(params.store) : params.store; - if (params.serialized !== undefined) { + params.takeOwnership === true ? params.store : cloneSessionStoreRecord(params.store); + if (params.takeOwnership === true) { internSessionStoreLargeStrings(store); } SESSION_STORE_CACHE.set(params.storePath, { diff --git a/src/config/sessions/store-load.ts b/src/config/sessions/store-load.ts index 4dfc3ee0cc3..617a5648b8d 100644 --- a/src/config/sessions/store-load.ts +++ b/src/config/sessions/store-load.ts @@ -448,6 +448,7 @@ export function loadSessionStore( mtimeMs, sizeBytes: fileStat?.sizeBytes, serialized: serializedFromDisk, + takeOwnership: serializedFromDisk !== undefined, }); } diff --git a/src/plugins/manifest-model-id-normalization.test.ts b/src/plugins/manifest-model-id-normalization.test.ts index 7941f67b330..7b4f19a263d 100644 --- a/src/plugins/manifest-model-id-normalization.test.ts +++ b/src/plugins/manifest-model-id-normalization.test.ts @@ -2,6 +2,7 @@ import fs from "node:fs"; import os from "node:os"; import path from "node:path"; import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import type { OpenClawConfig } from "../config/types.openclaw.js"; import { clearCurrentPluginMetadataSnapshot, resolvePluginMetadataControlPlaneFingerprint, @@ -11,6 +12,7 @@ import { resolveInstalledPluginIndexPolicyHash } from "./installed-plugin-index- import type { InstalledPluginIndex } from "./installed-plugin-index.js"; import { listOpenClawPluginManifestMetadata } from "./manifest-metadata-scan.js"; import { normalizeProviderModelIdWithManifest } from "./manifest-model-id-normalization.js"; +import { clearPluginMetadataLifecycleCaches } from "./plugin-metadata-lifecycle.js"; import type { PluginMetadataSnapshot } from "./plugin-metadata-snapshot.js"; import { createEmptyPluginRegistry } from "./registry-empty.js"; import { resetPluginRuntimeStateForTest, setActivePluginRegistry } from "./runtime.js"; @@ -87,8 +89,10 @@ function createCurrentSnapshot(params: { manifestHash: string; prefix: string; workspaceDir?: string; + config?: OpenClawConfig; }): PluginMetadataSnapshot { - const policyHash = resolveInstalledPluginIndexPolicyHash({}); + const config = params.config ?? {}; + const policyHash = resolveInstalledPluginIndexPolicyHash(config); const index: InstalledPluginIndex = { version: 1, hostContractVersion: "test-host", @@ -119,15 +123,12 @@ function createCurrentSnapshot(params: { }; return { policyHash, - configFingerprint: resolvePluginMetadataControlPlaneFingerprint( - {}, - { - env: process.env, - index, - policyHash, - workspaceDir: params.workspaceDir, - }, - ), + configFingerprint: resolvePluginMetadataControlPlaneFingerprint(config, { + env: process.env, + index, + policyHash, + workspaceDir: params.workspaceDir, + }), workspaceDir: params.workspaceDir, index, registryDiagnostics: [], @@ -153,6 +154,17 @@ function normalizeDemoModel(modelId = "demo-model"): string | undefined { }); } +function normalizeDemoModelWithEnv( + env: NodeJS.ProcessEnv, + modelId = "demo-model", +): string | undefined { + return normalizeProviderModelIdWithManifest({ + provider: "demo", + env, + context: { provider: "demo", modelId }, + }); +} + describe("manifest model id normalization", () => { beforeEach(() => { resetPluginRuntimeStateForTest(); @@ -234,6 +246,45 @@ describe("manifest model id normalization", () => { expect(normalizeDemoModel()).toBe("alpha/demo-model"); }); + it("reuses current metadata when callers omit config", () => { + const config: OpenClawConfig = { plugins: { allow: ["normalizer"] } }; + setCurrentPluginMetadataSnapshot( + createCurrentSnapshot({ + manifestHash: "alpha", + prefix: "alpha", + config, + }), + { config, env: process.env }, + ); + + expect(normalizeDemoModel()).toBe("alpha/demo-model"); + }); + + it("validates explicit env before reusing current metadata", () => { + setCurrentPluginMetadataSnapshot( + createCurrentSnapshot({ + manifestHash: "alpha", + prefix: "alpha", + }), + { config: {}, env: process.env }, + ); + + const stateDir = makeTempDir(); + const pluginDir = path.join(stateDir, "extensions", "normalizer"); + writeInstallIndex({ stateDir, pluginDir }); + writeNormalizerManifest({ pluginDir, prefix: "bravo" }); + + const env = { + ...process.env, + OPENCLAW_STATE_DIR: stateDir, + OPENCLAW_HOME: undefined, + OPENCLAW_DISABLE_BUNDLED_PLUGINS: "1", + OPENCLAW_BUNDLED_PLUGINS_DIR: undefined, + }; + + expect(normalizeDemoModelWithEnv(env)).toBe("bravo/demo-model"); + }); + it("reflects manifest edits and state-dir changes on the next lookup", () => { const stateDirA = makeTempDir(); const pluginDirA = path.join(stateDirA, "extensions", "normalizer"); @@ -248,6 +299,7 @@ describe("manifest model id normalization", () => { expect(normalizeDemoModel()).toBe("alpha/demo-model"); writeNormalizerManifest({ pluginDir: pluginDirA, prefix: "bravo-local" }); + clearPluginMetadataLifecycleCaches(); expect(normalizeDemoModel()).toBe("bravo-local/demo-model"); const stateDirB = makeTempDir(); @@ -256,6 +308,7 @@ describe("manifest model id normalization", () => { writeNormalizerManifest({ pluginDir: pluginDirB, prefix: "charlie" }); process.env.OPENCLAW_STATE_DIR = stateDirB; + clearPluginMetadataLifecycleCaches(); expect(normalizeDemoModel()).toBe("charlie/demo-model"); }); diff --git a/src/plugins/manifest-model-id-normalization.ts b/src/plugins/manifest-model-id-normalization.ts index 6049bdc7607..62ac6173444 100644 --- a/src/plugins/manifest-model-id-normalization.ts +++ b/src/plugins/manifest-model-id-normalization.ts @@ -1,11 +1,9 @@ import type { OpenClawConfig } from "../config/types.openclaw.js"; import { normalizeLowercaseStringOrEmpty } from "../shared/string-coerce.js"; +import { getCurrentPluginMetadataSnapshot } from "./current-plugin-metadata-snapshot.js"; import type { PluginManifestRecord } from "./manifest-registry.js"; import type { PluginManifestModelIdNormalizationProvider } from "./manifest.js"; -import { - resolvePluginMetadataSnapshot, - type PluginMetadataSnapshot, -} from "./plugin-metadata-snapshot.js"; +import { resolvePluginMetadataSnapshot } from "./plugin-metadata-snapshot.js"; import { getActivePluginRegistryWorkspaceDirFromState } from "./runtime-state.js"; type ManifestModelIdNormalizationLookupParams = { @@ -37,19 +35,37 @@ let cachedPolicies: ManifestModelIdNormalizationPolicyCache | undefined; function resolveMetadataSnapshotForPolicies( params: ManifestModelIdNormalizationLookupParams = {}, ): { - snapshot: PluginMetadataSnapshot; + plugins: readonly Pick[]; + configFingerprint?: string; cacheable: boolean; } { const env = params.env ?? process.env; const workspaceDir = params.workspaceDir ?? getActivePluginRegistryWorkspaceDirFromState(); - return { - snapshot: resolvePluginMetadataSnapshot({ - config: params.config ?? {}, + if (params.config === undefined) { + const currentSnapshot = getCurrentPluginMetadataSnapshot({ env, workspaceDir, - allowWorkspaceScopedCurrent: true, - }), - cacheable: true, + allowWorkspaceScopedSnapshot: true, + requireDefaultDiscoveryContext: params.env !== undefined, + }); + if (currentSnapshot) { + return { + plugins: currentSnapshot.plugins, + configFingerprint: currentSnapshot.configFingerprint, + cacheable: true, + }; + } + } + const snapshot = resolvePluginMetadataSnapshot({ + config: params.config ?? {}, + env, + workspaceDir, + allowWorkspaceScopedCurrent: true, + }); + return { + plugins: snapshot.plugins, + configFingerprint: snapshot.configFingerprint, + cacheable: false, }; } @@ -59,12 +75,11 @@ function loadManifestModelIdNormalizationPolicies( if (params.plugins) { return collectManifestModelIdNormalizationPolicies(params.plugins); } - const { snapshot, cacheable } = resolveMetadataSnapshotForPolicies(params); - const configFingerprint = snapshot.configFingerprint; + const { plugins, configFingerprint, cacheable } = resolveMetadataSnapshotForPolicies(params); if (cacheable && configFingerprint && cachedPolicies?.configFingerprint === configFingerprint) { return cachedPolicies.policies; } - const policies = collectManifestModelIdNormalizationPolicies(snapshot.plugins); + const policies = collectManifestModelIdNormalizationPolicies(plugins); if (cacheable && configFingerprint) { cachedPolicies = { configFingerprint, policies }; }