perf: share a frozen team-summary snapshot instead of cloning on every listTeams
listTeams() deep-cloned ALL team summaries via structuredClone on every call -- even cache hits and concurrent in-flight awaiters. A heap allocation sample of a launch put this (listTeams -> cloneTeamSummaries -> structuredClone) as the single largest memory allocator, driving heap churn + GC pressure during launch (this stand has ~158 teams, and listTeams is called constantly: startup, notification init, task projection, IPC polls, provisioning). Build ONE deep-frozen, independent snapshot per uncached load and hand the same reference to the cache entry, in-flight awaiters, and every later reader. The single cloneTeamSummaries keeps it independent of any cached config the loader returns; freezing lets all readers share it safely. Audited every listTeams consumer -- all iterate / map / filter / serialize, none mutate -- and the freeze turns any stray future mutation into a loud error rather than silent cross-caller corruption. TeamConfigReader 26/26 (added a frozen + same-reference regression test), and the listTeams consumers (TeamDataService 116, CrossTeamService 26) all pass under frozen summaries.
This commit is contained in:
parent
58dfac8377
commit
0a750a9fa8
2 changed files with 59 additions and 6 deletions
|
|
@ -222,6 +222,26 @@ function cloneTeamSummaries(teams: readonly TeamSummary[]): TeamSummary[] {
|
||||||
return structuredClone([...teams]);
|
return structuredClone([...teams]);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Deep-freeze a team-summary snapshot so it can be shared by every listTeams() reader
|
||||||
|
// (and concurrent in-flight awaiters) instead of deep-cloning all summaries on every
|
||||||
|
// call -- that per-read structuredClone was the single largest memory allocator during
|
||||||
|
// launch. Consumers treat the result as read-only (audited: all iterate / map / filter
|
||||||
|
// / serialize, none mutate), and freezing turns any stray future mutation into a loud
|
||||||
|
// error instead of silent cross-caller corruption.
|
||||||
|
function freezeTeamSummariesDeep(teams: TeamSummary[]): TeamSummary[] {
|
||||||
|
const freeze = (value: unknown): void => {
|
||||||
|
if (!value || typeof value !== 'object' || Object.isFrozen(value)) {
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
Object.freeze(value);
|
||||||
|
for (const nested of Object.values(value as Record<string, unknown>)) {
|
||||||
|
freeze(nested);
|
||||||
|
}
|
||||||
|
};
|
||||||
|
freeze(teams);
|
||||||
|
return teams;
|
||||||
|
}
|
||||||
|
|
||||||
function classifyConfigReadTiming(timing: {
|
function classifyConfigReadTiming(timing: {
|
||||||
statMs: number | null;
|
statMs: number | null;
|
||||||
readMs: number | null;
|
readMs: number | null;
|
||||||
|
|
@ -353,15 +373,23 @@ export class TeamConfigReader {
|
||||||
const teamsBasePath = getTeamsBasePath();
|
const teamsBasePath = getTeamsBasePath();
|
||||||
const cached = TeamConfigReader.listTeamsCacheByBasePath.get(teamsBasePath);
|
const cached = TeamConfigReader.listTeamsCacheByBasePath.get(teamsBasePath);
|
||||||
if (cached && cached.expiresAt > Date.now()) {
|
if (cached && cached.expiresAt > Date.now()) {
|
||||||
return cloneTeamSummaries(cached.value);
|
// Frozen, independent snapshot -> safe to hand out directly. The per-read
|
||||||
|
// structuredClone that used to be here was the top memory allocator on launch.
|
||||||
|
return cached.value;
|
||||||
}
|
}
|
||||||
|
|
||||||
const existingRequest = TeamConfigReader.listTeamsInFlightByBasePath.get(teamsBasePath);
|
const existingRequest = TeamConfigReader.listTeamsInFlightByBasePath.get(teamsBasePath);
|
||||||
if (existingRequest?.generationAtStart === TeamConfigReader.listTeamsGeneration) {
|
if (existingRequest?.generationAtStart === TeamConfigReader.listTeamsGeneration) {
|
||||||
return cloneTeamSummaries(await existingRequest.promise);
|
return existingRequest.promise;
|
||||||
}
|
}
|
||||||
|
|
||||||
const request = this.listTeamsUncached(teamsBasePath);
|
// Build ONE frozen, independent snapshot shared by this load's cache entry, its
|
||||||
|
// in-flight awaiters, and every later reader. cloneTeamSummaries() makes the copy
|
||||||
|
// independent of any cached config the (worker or fallback) loader may return;
|
||||||
|
// freezing then lets all readers share it without per-call deep clones.
|
||||||
|
const request = this.listTeamsUncached(teamsBasePath).then((teams) =>
|
||||||
|
freezeTeamSummariesDeep(cloneTeamSummaries(teams))
|
||||||
|
);
|
||||||
const generationAtStart = TeamConfigReader.listTeamsGeneration;
|
const generationAtStart = TeamConfigReader.listTeamsGeneration;
|
||||||
TeamConfigReader.listTeamsInFlightByBasePath.set(teamsBasePath, {
|
TeamConfigReader.listTeamsInFlightByBasePath.set(teamsBasePath, {
|
||||||
promise: request,
|
promise: request,
|
||||||
|
|
@ -369,14 +397,14 @@ export class TeamConfigReader {
|
||||||
});
|
});
|
||||||
|
|
||||||
try {
|
try {
|
||||||
const teams = await request;
|
const frozenTeams = await request;
|
||||||
if (TeamConfigReader.listTeamsGeneration === generationAtStart) {
|
if (TeamConfigReader.listTeamsGeneration === generationAtStart) {
|
||||||
TeamConfigReader.listTeamsCacheByBasePath.set(teamsBasePath, {
|
TeamConfigReader.listTeamsCacheByBasePath.set(teamsBasePath, {
|
||||||
value: cloneTeamSummaries(teams),
|
value: frozenTeams,
|
||||||
expiresAt: Date.now() + LIST_TEAMS_CACHE_TTL_MS,
|
expiresAt: Date.now() + LIST_TEAMS_CACHE_TTL_MS,
|
||||||
});
|
});
|
||||||
}
|
}
|
||||||
return cloneTeamSummaries(teams);
|
return frozenTeams;
|
||||||
} finally {
|
} finally {
|
||||||
if (TeamConfigReader.listTeamsInFlightByBasePath.get(teamsBasePath)?.promise === request) {
|
if (TeamConfigReader.listTeamsInFlightByBasePath.get(teamsBasePath)?.promise === request) {
|
||||||
TeamConfigReader.listTeamsInFlightByBasePath.delete(teamsBasePath);
|
TeamConfigReader.listTeamsInFlightByBasePath.delete(teamsBasePath);
|
||||||
|
|
|
||||||
|
|
@ -327,6 +327,31 @@ describe('TeamConfigReader', () => {
|
||||||
expect(readdirSpy.mock.calls.length).toBeGreaterThan(readdirAfterFirstBatch);
|
expect(readdirSpy.mock.calls.length).toBeGreaterThan(readdirAfterFirstBatch);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it('returns a frozen team-summary snapshot shared across cached reads (no per-read clone)', async () => {
|
||||||
|
const teamName = 'frozen-list-team';
|
||||||
|
const teamDir = path.join(tempDir, teamName);
|
||||||
|
await fs.mkdir(teamDir, { recursive: true });
|
||||||
|
await fs.writeFile(
|
||||||
|
path.join(teamDir, 'config.json'),
|
||||||
|
JSON.stringify({
|
||||||
|
name: 'Frozen Team',
|
||||||
|
members: [{ name: 'team-lead', agentType: 'team-lead' }],
|
||||||
|
}),
|
||||||
|
'utf8'
|
||||||
|
);
|
||||||
|
|
||||||
|
const reader = new TeamConfigReader();
|
||||||
|
const first = await reader.listTeams();
|
||||||
|
const second = await reader.listTeams(); // served from cache
|
||||||
|
|
||||||
|
expect(first.length).toBeGreaterThan(0);
|
||||||
|
// Deep-frozen so the shared snapshot cannot be mutated by any reader.
|
||||||
|
expect(Object.isFrozen(first)).toBe(true);
|
||||||
|
expect(Object.isFrozen(first[0])).toBe(true);
|
||||||
|
// Same reference across reads -> the per-read structuredClone is gone.
|
||||||
|
expect(second).toBe(first);
|
||||||
|
});
|
||||||
|
|
||||||
it('does not reuse a stale in-flight listTeams scan after invalidation', async () => {
|
it('does not reuse a stale in-flight listTeams scan after invalidation', async () => {
|
||||||
const teamName = 'inflight-invalidated-list-team';
|
const teamName = 'inflight-invalidated-list-team';
|
||||||
const teamDir = path.join(tempDir, teamName);
|
const teamDir = path.join(tempDir, teamName);
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue