Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions .jules/bolt.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
## 2024-05-15 - Batched concurrent I/O operations for large file iterations
**Learning:** Sequential await calls in loops for operations like `fs.stat` or `fs.rm` over large directories (e.g. ArtifactStore.gc) cause severe performance bottlenecks and can trigger EMFILE limits.
**Action:** Always batch I/O operations using `Promise.all` with a reasonable chunk size (e.g. 10) to parallelize without hitting EMFILE limits.
## 2024-05-15 - Array.find inside batched iterations causes O(N^2) regressions
**Learning:** In batched loops, such as when calling `isExpired` inside `evictExpiredEntries`, calling `this.cacheStore.get()` inside the loop performs an O(N) lookup. Since the loop runs O(N) times, the overall complexity becomes O(N^2), causing performance regressions.
**Action:** Always reuse the entry objects obtained during the initial O(N) iteration rather than fetching them again.
39 changes: 30 additions & 9 deletions src/core/sub-agent/artifacts/store.ts
Original file line number Diff line number Diff line change
Expand Up @@ -93,13 +93,27 @@ export class ArtifactStore {
const entries = await fs.readdir(root, { withFileTypes: true }).catch(() => []);

const files: Array<{ name: string; path: string; mtimeMs: number; size: number }> = [];
for (const entry of entries) {
if (!entry.isFile()) continue;
const validEntries = entries.filter((entry) => {
if (!entry.isFile()) return false;
const filePath = path.join(root, entry.name);
if (!isWithinDir(root, filePath)) continue;
const stat = await fs.stat(filePath).catch(() => null);
if (!stat) continue;
files.push({ name: entry.name, path: filePath, mtimeMs: stat.mtimeMs, size: stat.size });
return isWithinDir(root, filePath);
});

for (let i = 0; i < validEntries.length; i += 10) {
const chunk = validEntries.slice(i, i + 10);
const stats = await Promise.all(
chunk.map(async (entry) => {
const filePath = path.join(root, entry.name);
const stat = await fs.stat(filePath).catch(() => null);
return { entry, filePath, stat };
}),
);

for (const { entry, filePath, stat } of stats) {
if (stat) {
files.push({ name: entry.name, path: filePath, mtimeMs: stat.mtimeMs, size: stat.size });
}
}
}

const maxAgeMs = options?.maxAgeMs ?? LIMITS.artifactTtlMs;
Expand All @@ -118,8 +132,9 @@ export class ArtifactStore {
removedBytes += file.size;
};

for (const file of expired) {
await removeFile(file);
for (let i = 0; i < expired.length; i += 10) {
const chunk = expired.slice(i, i + 10);
await Promise.all(chunk.map((file) => removeFile(file)));
}

// Recompute remaining after TTL removal (newest first).
Expand All @@ -130,17 +145,23 @@ export class ArtifactStore {
let currentFiles = remaining.length;
let currentBytes = remaining.reduce((acc, f) => acc + f.size, 0);

const excessFilesToRemove = [];
for (let i = remaining.length - 1; i >= 0; i--) {
const tooManyFiles = currentFiles > maxFiles;
const tooManyBytes = currentBytes > maxTotalBytes;
if (!tooManyFiles && !tooManyBytes) break;

const oldest = remaining[i];
await removeFile(oldest);
excessFilesToRemove.push(oldest);
currentFiles -= 1;
currentBytes -= oldest.size;
}

for (let i = 0; i < excessFilesToRemove.length; i += 10) {
const chunk = excessFilesToRemove.slice(i, i + 10);
await Promise.all(chunk.map((file) => removeFile(file)));
}

return { removedFiles, removedBytes };
}

Expand Down
Loading