perf: reuse caller stat in fileBelongsToTeam, drop duplicate fs.stat
On the live resolution path collectRootJsonlSessionIds already stat()s each root jsonl for its mtime-window filter, then fileBelongsToTeam stat()ed the very same file again for its cache validation -- two fs.stat syscalls (plus two Stats allocations) per file, every poll. fileBelongsToTeam now takes an optional precomputed stat and the mtime-filter caller passes the stat it already has, so the file is statted once. Measured 20 files -> 20 stat calls on the mtime path (was ~40). Using a single stat snapshot is also slightly more consistent than two reads that could straddle a concurrent write. The other call site (subagent scan) passes no stat and is unchanged (fileBelongsToTeam stats it itself). Adds a regression test that a caller-supplied stat is the one recorded in the affinity cache.
This commit is contained in:
parent
635321dd9a
commit
776298b0e3
2 changed files with 54 additions and 7 deletions
|
|
@ -966,17 +966,19 @@ export class TeamTranscriptProjectResolver {
|
||||||
while (nextIndex < rootJsonlEntries.length) {
|
while (nextIndex < rootJsonlEntries.length) {
|
||||||
const entry = rootJsonlEntries[nextIndex++];
|
const entry = rootJsonlEntries[nextIndex++];
|
||||||
const filePath = path.join(projectDir, entry.name);
|
const filePath = path.join(projectDir, entry.name);
|
||||||
|
let precomputedStat: { mtimeMs: number; size: number; isFile: () => boolean } | undefined;
|
||||||
if (mtimeSinceMs != null) {
|
if (mtimeSinceMs != null) {
|
||||||
try {
|
try {
|
||||||
const stat = await fs.stat(filePath);
|
const stat = await fs.stat(filePath);
|
||||||
if (!stat.isFile() || stat.mtimeMs < mtimeSinceMs) {
|
if (!stat.isFile() || stat.mtimeMs < mtimeSinceMs) {
|
||||||
continue;
|
continue;
|
||||||
}
|
}
|
||||||
|
precomputedStat = stat;
|
||||||
} catch {
|
} catch {
|
||||||
continue;
|
continue;
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
if (!(await this.fileBelongsToTeam(filePath, teamName))) {
|
if (!(await this.fileBelongsToTeam(filePath, teamName, precomputedStat))) {
|
||||||
continue;
|
continue;
|
||||||
}
|
}
|
||||||
discovered.add(entry.name.slice(0, -'.jsonl'.length));
|
discovered.add(entry.name.slice(0, -'.jsonl'.length));
|
||||||
|
|
@ -1008,17 +1010,29 @@ export class TeamTranscriptProjectResolver {
|
||||||
return this.collectRootJsonlSessionIds(rootJsonlEntries, projectDir, teamName, mtimeSinceMs);
|
return this.collectRootJsonlSessionIds(rootJsonlEntries, projectDir, teamName, mtimeSinceMs);
|
||||||
}
|
}
|
||||||
|
|
||||||
private async fileBelongsToTeam(filePath: string, teamName: string): Promise<boolean> {
|
private async fileBelongsToTeam(
|
||||||
|
filePath: string,
|
||||||
|
teamName: string,
|
||||||
|
precomputedStat?: { mtimeMs: number; size: number; isFile: () => boolean }
|
||||||
|
): Promise<boolean> {
|
||||||
const normalizedTeam = teamName.trim().toLowerCase();
|
const normalizedTeam = teamName.trim().toLowerCase();
|
||||||
if (!normalizedTeam) {
|
if (!normalizedTeam) {
|
||||||
return false;
|
return false;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Reuse the caller's stat when it already statted this exact file (the mtime-window
|
||||||
|
// filter in collectRootJsonlSessionIds does). On the live resolution path this drops
|
||||||
|
// a second fs.stat of the same file per entry, every poll — and using a single stat
|
||||||
|
// snapshot is also more consistent than two reads that could straddle a write.
|
||||||
let fileStat: { mtimeMs: number; size: number; isFile: () => boolean };
|
let fileStat: { mtimeMs: number; size: number; isFile: () => boolean };
|
||||||
try {
|
if (precomputedStat) {
|
||||||
fileStat = await fs.stat(filePath);
|
fileStat = precomputedStat;
|
||||||
} catch {
|
} else {
|
||||||
return false;
|
try {
|
||||||
|
fileStat = await fs.stat(filePath);
|
||||||
|
} catch {
|
||||||
|
return false;
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
if (!fileStat.isFile()) {
|
if (!fileStat.isFile()) {
|
||||||
|
|
|
||||||
|
|
@ -637,7 +637,11 @@ describe('TeamTranscriptProjectResolver', () => {
|
||||||
headWindowFull: boolean;
|
headWindowFull: boolean;
|
||||||
};
|
};
|
||||||
type ResolverProbe = {
|
type ResolverProbe = {
|
||||||
fileBelongsToTeam: (filePath: string, teamName: string) => Promise<boolean>;
|
fileBelongsToTeam: (
|
||||||
|
filePath: string,
|
||||||
|
teamName: string,
|
||||||
|
precomputedStat?: { mtimeMs: number; size: number; isFile: () => boolean }
|
||||||
|
) => Promise<boolean>;
|
||||||
buildTeamAffinityFileCacheKey: (filePath: string, normalizedTeam: string) => string;
|
buildTeamAffinityFileCacheKey: (filePath: string, normalizedTeam: string) => string;
|
||||||
teamAffinityFileCache: Map<string, AffinityCacheEntry>;
|
teamAffinityFileCache: Map<string, AffinityCacheEntry>;
|
||||||
};
|
};
|
||||||
|
|
@ -716,4 +720,33 @@ describe('TeamTranscriptProjectResolver', () => {
|
||||||
expect(second?.belongsToTeam).toBe(true);
|
expect(second?.belongsToTeam).toBe(true);
|
||||||
expect(second!.size).toBeGreaterThan(sizeAfterFirst); // re-scanned + re-cached
|
expect(second!.size).toBeGreaterThan(sizeAfterFirst); // re-scanned + re-cached
|
||||||
});
|
});
|
||||||
|
|
||||||
|
// Regression: when the caller already statted the file (the mtime-window filter in
|
||||||
|
// collectRootJsonlSessionIds), fileBelongsToTeam must reuse that stat rather than
|
||||||
|
// issuing a second fs.stat of the same file. Proven without mocking fs: a precomputed
|
||||||
|
// stat with a deliberately distinct size/mtime must be the one recorded in the cache.
|
||||||
|
it('reuses a caller-supplied stat instead of re-statting the file', async () => {
|
||||||
|
await setupClaudeRoot();
|
||||||
|
const resolver = new TeamTranscriptProjectResolver() as unknown as ResolverProbe;
|
||||||
|
const team = 'absent-team';
|
||||||
|
const projectDir = path.join(tmpDir!, 'projects', encodePath('/repo/precomp'));
|
||||||
|
await fs.mkdir(projectDir, { recursive: true });
|
||||||
|
const jsonlPath = path.join(projectDir, 'f.jsonl');
|
||||||
|
await fs.writeFile(
|
||||||
|
jsonlPath,
|
||||||
|
`${Array.from({ length: 45 }, (_, i) =>
|
||||||
|
JSON.stringify({ type: 'user', message: { role: 'user', content: `x ${i}` } })
|
||||||
|
).join('\n')}\n`,
|
||||||
|
'utf8'
|
||||||
|
);
|
||||||
|
|
||||||
|
// Distinct sentinel values the real file does not have.
|
||||||
|
const precomputedStat = { mtimeMs: 123_456, size: 999_999, isFile: () => true };
|
||||||
|
expect(await resolver.fileBelongsToTeam(jsonlPath, team, precomputedStat)).toBe(false);
|
||||||
|
|
||||||
|
const key = resolver.buildTeamAffinityFileCacheKey(jsonlPath, team);
|
||||||
|
const entry = resolver.teamAffinityFileCache.get(key);
|
||||||
|
expect(entry?.size).toBe(999_999); // cache recorded the precomputed stat -> no re-stat
|
||||||
|
expect(entry?.mtimeMs).toBe(123_456);
|
||||||
|
});
|
||||||
});
|
});
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue