Skip to content

Commit 2bac59f

Browse files
Prithvi-Webclaude
andcommitted
fix(query): a node waiting on the query walk's stack holds its folder's path as the walk joins paths onto it - pieces never read - not the folder's node, whose own path a path: or in: term had read whole, so a deep chain whose every folder holds a file beside its subfolder costs memory in proportion to its depth, not its square (CI4 review F1.2)
The fix before this one let a node drop its folder once its own path was built, which covered a bare chain only. The walk pushes a folder's children in order and takes the last first, so a file listed before its subfolder waits on the stack while everything under the folder is walked. It held the folder's node, to build its own path from the folder's, and with it the folder's own path - which a path: or in: term had read, and reading a path copies it whole. Down a chain where every folder holds such a file that is a whole copy of every folder's path: 185.6 MB at 8,000 deep (23 KB a folder, for path:leaf and for in:/deep/d0 name:leaf alike), and Node 20 under a 2 GB heap aborted at 30,000 deep ("Ineffective mark-compacts near heap limit"). The Highlight box asks on every pause in typing, and in: is a common term. A node now holds a FolderPath: its folder's path as the walk joins its nodes' paths onto it, made once per folder as its children are pushed. While the walk runs it is handed only to childPath, which reads none of it, so a chain of them stays the pieces they were joined from. A folder's own path, the one terms and sort ties read, is a second string while the walk runs; once the walk is over nothing waits, and a folder's path is the one its children join onto, so each folder's path is still built once for the page (G07's counted tests hold: a size query builds no more than the 319 paths its hits need; with the two kept apart after the walk too, it builds 468). Measured (Node 20.20.2, 2 GB heap, the heap after a full collection when the deep path is built), 8,000 deep: a file beside every folder, path:leaf 236 bytes a folder, in: 241, name:leaf 2; a bare chain, path:leaf 61. At 30,000 deep with a file beside every folder, path:leaf and in: finish (134 and 143 MB resident). A deep chain's path is built from pieces, so reading the deepest one walks every piece: slower than copying a flat one, and only on depths no real tree has. Tests (tests/queryWalkWork.test.ts): - "a file waiting beside every folder ...": path:leaf, in:/deep/d0 name:leaf and name:leaf, one subtest each, down that 8,000-deep chain - the one file found, its path exact and built once, under 2 KiB a folder held. RED before the fix: 185.6 / 185.5 MB. - "a sort tie that reads a kept folder's path while its file still waits": folders sized by hand two at a time, type:dir, one kept, largest first - each odd folder ties with the even one kept above it, so the kept folder's path is read after its children were pushed. RED before the fix (185.5 MB); proves the "once the walk is over" gate. - The heap is now also read as the deepest folder's path is joined (the last step of a chain joined in one go), so the existing CI4 test still fails if the store's childPath reads the parent path again (name:leaf 184.0 MB). Mutants, each RED for the right reason, restored and checked by SHA-256: a folder's children join onto the path its readers read (path:leaf 184.2 MB); its path shared while the walk runs (tie: 93.6 MB, every second folder); never shared, or the walk never marked over (paths built: 468 for 319 needed); a node's own path not kept (built twice, not once); folders joined in the wrong order (the path is wrong); the store reading the parent path (name:leaf 184.0 MB); size ties not broken by path (the tie's path never built); the scanned folder not counted (total). Fixture checks: the file listed after its folder; folders not paired by size. Not changed: a kept hit, and a candidate of a query that looks facts up, hold their own paths - bounded by how many are kept (the page, or the 20,000-candidate cap), not by what waits on the stack. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
1 parent 44dfcb7 commit 2bac59f

2 files changed

Lines changed: 211 additions & 59 deletions

File tree

‎src/services/query/execute.ts‎

Lines changed: 84 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -413,20 +413,25 @@ class QueryNode implements EvalNode {
413413
readonly childCount: number | undefined;
414414
private nameRead: string | undefined;
415415
private pathRead: string | undefined;
416+
/** What this folder's children join their paths onto: made as they are pushed. */
417+
private within: FolderPath | null = null;
416418

417419
constructor(
418420
private readonly store: ScanStore,
419421
readonly id: number,
420422
/**
421-
* Held only to build this node's path, and let go once it is built (CI4):
422-
* kept after, a node held every folder above it alive, and each folder's
423-
* path with it — a whole copy once a path term had read it — so a walk
424-
* down a deep chain held the square of its depth. Null for the root, and
425-
* for a node whose path is known.
423+
* The path of the folder this node is in, as its nodes join theirs onto
424+
* it — never the folder's node (CI4 review F1.2). A node waiting on the
425+
* walk's stack holds this; holding the folder's node instead held the
426+
* folder's own path, which a path term had read whole, so a file listed
427+
* before its subfolder kept that copy alive while the walk went down —
428+
* the square of the depth down a chain where every folder holds one.
429+
* Null for the root.
426430
*/
427-
private folder: QueryNode | null,
431+
private readonly folder: FolderPath | null,
428432
readonly depth: number,
429433
childlessKnown: boolean,
434+
private readonly walk: WalkProgress,
430435
) {
431436
this.isDir = store.isDir(id);
432437
this.size = store.size(id);
@@ -439,24 +444,71 @@ class QueryNode implements EvalNode {
439444
}
440445

441446
/**
442-
* Built down from the nearest folder whose path is known, never by asking
443-
* the folder's own getter: a query that reads no path while it walks leaves
444-
* every folder's unbuilt, and a stack frame per level of a 30k-deep chain
445-
* overflows (review F1.2; the store's own rule, tests/packedStore.test.ts).
447+
* Built from its folder's path, which is built down from the nearest
448+
* folder whose path is known, never by asking a folder's own getter: a
449+
* query that reads no path while it walks leaves every folder's unbuilt,
450+
* and a stack frame per level of a 30k-deep chain overflows (review F1.2;
451+
* the store's own rule, tests/packedStore.test.ts).
452+
*
453+
* While the walk runs, a folder's path and the one its children join onto
454+
* are two strings, though they spell the same path: whatever reads this one
455+
* copies it whole, and that copy must not be what the nodes waiting in the
456+
* folder hold. Once the walk is over nothing waits, and a folder's path is
457+
* the one its children join onto, so a folder among the hits shares one
458+
* string with the hits beneath it and each folder's path is still built
459+
* once for the page (G07 review F2.3, F3.1).
446460
*/
447461
get path(): string {
448462
if (this.pathRead !== undefined) return this.pathRead;
449-
const unbuilt: QueryNode[] = [];
450-
let known: QueryNode = this;
451-
while (known.pathRead === undefined && known.folder !== null) {
452-
unbuilt.push(known);
453-
known = known.folder;
463+
if (this.walk.over && this.within !== null) return (this.pathRead = this.within.path(this.store));
464+
return (this.pathRead = this.folder === null ? this.store.rootPath : this.store.childPath(this.id, this.folder.path(this.store)));
465+
}
466+
467+
/** The folder its children's paths join onto — made once, as the walk pushes them. */
468+
folderForChildren(): FolderPath {
469+
return (this.within ??= this.folder === null
470+
? new FolderPath(this.id, null, this.store.rootPath)
471+
: new FolderPath(this.id, this.folder, undefined));
472+
}
473+
}
474+
475+
/** Shared by every node of one walk: whether it is over, so nothing waits on its stack any more. */
476+
interface WalkProgress {
477+
over: boolean;
478+
}
479+
480+
/**
481+
* A folder's path as the paths of the nodes in it are joined onto it (CI4
482+
* review F1.2). While the walk runs it is handed only to `childPath`, which
483+
* reads none of it, so it stays the pieces it was joined from — its folder's,
484+
* shared, and its own name with a separator — and a chain of them costs a few
485+
* bytes a folder however deep it goes. Nothing that reads a path is handed
486+
* it until the walk is over, because reading copies a path whole, and every
487+
* node waiting on the walk's stack holds its folder's: a folder's path read
488+
* whole there would stay whole as long as anything in that folder waited.
489+
*/
490+
class FolderPath {
491+
constructor(
492+
private readonly id: number,
493+
/** The folder this one is in, until this one's path is joined. */
494+
private folder: FolderPath | null,
495+
private joined: string | undefined,
496+
) {}
497+
498+
/** Joined down from the nearest folder whose path is, without a stack frame per folder (review F1.2). */
499+
path(store: ScanStore): string {
500+
if (this.joined !== undefined) return this.joined;
501+
const unjoined: FolderPath[] = [];
502+
let known: FolderPath = this;
503+
while (known.joined === undefined) {
504+
unjoined.push(known);
505+
known = known.folder as FolderPath; // the root's is joined from the start
454506
}
455-
let path = (known.pathRead ??= this.store.rootPath); // known, or the root
456-
for (let i = unbuilt.length - 1; i >= 0; i--) {
457-
const node = unbuilt[i];
458-
path = node.pathRead = this.store.childPath(node.id, path);
459-
node.folder = null;
507+
let path = known.joined;
508+
for (let i = unjoined.length - 1; i >= 0; i--) {
509+
const folder = unjoined[i];
510+
path = folder.joined = store.childPath(folder.id, path);
511+
folder.folder = null;
460512
}
461513
return path;
462514
}
@@ -490,14 +542,15 @@ class PassOneWalk implements SlicedWalk {
490542
readonly doubted = new WeakSet<EvalNode>();
491543
readonly kept: { offer(item: QueryNode): void; sorted(): QueryNode[] };
492544
private readonly stack: QueryNode[];
545+
private readonly progress: WalkProgress = { over: false };
493546

494547
constructor(private readonly store: ScanStore, private readonly ctx: PassOneContext) {
495548
this.kept = firstK<QueryNode>(ctx.keep, ctx.sort);
496-
this.stack = [new QueryNode(store, store.rootId, null, 0, ctx.childlessKnown)];
549+
this.stack = [new QueryNode(store, store.rootId, null, 0, ctx.childlessKnown, this.progress)];
497550
}
498551

499552
step(budget: number): boolean {
500-
const { store, stack, ctx } = this;
553+
const { store, stack, ctx, progress } = this;
501554
for (let left = budget; stack.length > 0; left--) {
502555
if (left <= 0) return false;
503556
if (ctx.signal.aborted) { this.aborted = true; return true; }
@@ -522,11 +575,18 @@ class PassOneWalk implements SlicedWalk {
522575

523576
if (node.isDir) {
524577
const depth = node.depth + 1;
525-
store.forEachChild(node.id, (child) => { stack.push(new QueryNode(store, child, node, depth, ctx.childlessKnown)); });
578+
store.forEachChild(node.id, (child) => {
579+
stack.push(new QueryNode(store, child, node.folderForChildren(), depth, ctx.childlessKnown, progress));
580+
});
526581
}
527582
}
528583
return true;
529584
}
585+
586+
/** Called once the walk is over, finished or stopped: nothing waits on its stack any more. */
587+
end(): void {
588+
this.progress.over = true;
589+
}
530590
}
531591

532592
/**
@@ -613,6 +673,7 @@ export async function executeAgainstScan(
613673
opts.slices ?? {},
614674
QUERY_SLICE_NODES,
615675
);
676+
pass.end();
616677
const { candidates, kept, doubted, examined, capped } = pass;
617678
let { matched, undecided, childlessUndecided, aborted } = pass;
618679

‎tests/queryWalkWork.test.ts‎

Lines changed: 127 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -269,11 +269,18 @@ test('G07: a tree that changes during a slice\'s turn is walked again from the s
269269
assert.ok(!out.hits.some((h) => h.path === victim), 'the deleted file is not among the hits');
270270
});
271271

272-
/** A completed scan of `depth` folders, one inside the next under /deep, with one file at the bottom. */
273-
function deepChain(depth: number): { scan: ScanResult; store: PackedScanStore; leaf: number; leafPath: string } {
272+
/**
273+
* A completed scan of `depth` folders, one inside the next under /deep, with
274+
* one file at the bottom. With `fileBeside`, every folder on the way also
275+
* holds a file, f<i>.bin, listed before its subfolder.
276+
*/
277+
function deepChain(depth: number, fileBeside = false): { scan: ScanResult; store: PackedScanStore; leaf: number; leafPath: string } {
274278
const store = new PackedScanStore('/deep', '/', { name: 'deep', isDir: true, size: 0, modifiedAt: 1, isHidden: false });
275279
let parent = store.rootId;
276-
for (let i = 0; i < depth; i++) parent = store.addNode(parent, { name: `d${i}`, isDir: true, size: 0, modifiedAt: 1, isHidden: false });
280+
for (let i = 0; i < depth; i++) {
281+
if (fileBeside) store.addNode(parent, { name: `f${i}.bin`, isDir: false, size: 1, modifiedAt: 1, isHidden: false, extension: 'bin' });
282+
parent = store.addNode(parent, { name: `d${i}`, isDir: true, size: 0, modifiedAt: 1, isHidden: false });
283+
}
277284
store.addNode(parent, { name: 'leaf.bin', isDir: false, size: 7, modifiedAt: 1, isHidden: false, extension: 'bin' });
278285
store.finalize();
279286
store.sumSizes();
@@ -283,7 +290,10 @@ function deepChain(depth: number): { scan: ScanResult; store: PackedScanStore; l
283290
scan.engine = 'walker';
284291
// Found by walking down after finalize, which may renumber.
285292
let leaf = store.rootId;
286-
while (store.isDir(leaf)) leaf = store.childIds(leaf)[0];
293+
while (store.isDir(leaf)) {
294+
const kids = store.childIds(leaf);
295+
leaf = kids.find((k) => store.isDir(k)) ?? kids[0];
296+
}
287297
let deepest = '/deep';
288298
for (let i = 0; i < depth; i++) deepest += `/d${i}`;
289299
return { scan, store, leaf, leafPath: `${deepest}/leaf.bin` };
@@ -305,13 +315,65 @@ test('G07 review F1.2: a hit at the bottom of a 30k-deep chain is answered, its
305315

306316
/**
307317
* What a deep hit's path may hold while it is built: a few hundred bytes per
308-
* folder on its way down (the node the walk holds, and the folder's share of
309-
* a path joined from its folder's) — never a whole copy of every folder's
310-
* path, which costs half the deep path's length per folder: 26 KB at 8,000
311-
* folders deep.
318+
* folder on its way down (the nodes the walk holds — a file waiting beside
319+
* the folder too — and the folder's share of a path joined from its
320+
* folder's) — never a whole copy of every folder's path, which costs half the
321+
* deep path's length per folder: 23 KB at 8,000 folders deep.
312322
*/
313323
const HELD_PER_FOLDER = 2048;
314324

325+
/**
326+
* Run `q` and read the heap each time `probe`'s path is built, and each time
327+
* a path of a node in `watch` is — the most the walk holds at once is at the
328+
* bottom: as the deepest node's path is built, and as the deepest folder's is
329+
* joined, the last of a chain joined in one go — after a full collection: a
330+
* byte count, never a time. Answers the most held, and how often `probe`'s
331+
* path was built.
332+
*/
333+
async function heldAsPathIsBuilt(
334+
scan: ScanResult, store: PackedScanStore, probe: number, q: string, gc: () => void, opts: { limit?: number; watch?: number[] } = {},
335+
): Promise<{ out: QueryOutcome; probes: number; held: number }> {
336+
const limit = opts.limit ?? 1000;
337+
const watched = new Set([probe, ...(opts.watch ?? [])]);
338+
await run(scan, q, { limit }); // whatever the first query loads is not the walk's
339+
let probes = 0;
340+
let held = -1;
341+
const base = heldBytes(gc);
342+
scan.store = new Proxy(store, {
343+
get(target, prop, receiver) {
344+
const value = Reflect.get(target, prop, receiver);
345+
if (typeof value !== 'function') return value;
346+
if (prop !== 'childPath') return (value as (...a: unknown[]) => unknown).bind(target);
347+
return (child: number, parentPath: string): string => {
348+
const built = (value as (c: number, p: string) => string).call(target, child, parentPath);
349+
if (child === probe) probes++;
350+
if (watched.has(child)) held = Math.max(held, heldBytes(gc) - base);
351+
return built;
352+
};
353+
},
354+
}) as ScanStore;
355+
const out = await run(scan, q, { limit });
356+
scan.store = store;
357+
return { out, probes, held };
358+
}
359+
360+
const heldMessage = (q: string, held: number, depth: number, deepPath: string): string =>
361+
`${q}: ${(held / 1e6).toFixed(1)} MB held as the deep path is built, ${Math.round(held / depth)} bytes per folder of ${depth} (at most ${HELD_PER_FOLDER}; a whole copy of every folder's path is about ${Math.round(deepPath.length / 2)})`;
362+
363+
/** The deep chain's one file found by `q`, its path exactly right and built once, and the walk's heap held to a few hundred bytes a folder as it is. */
364+
async function heldAsTheDeepPathIsBuilt(depth: number, fileBeside: boolean, q: string, gc: () => void): Promise<void> {
365+
const { scan, store, leaf, leafPath } = deepChain(depth, fileBeside);
366+
if (fileBeside) {
367+
const top = store.childIds(store.rootId);
368+
assert.ok(top.length === 2 && !store.isDir(top[0]) && store.isDir(top[1]), 'each folder lists its file before its subfolder');
369+
}
370+
const { out, probes, held } = await heldAsPathIsBuilt(scan, store, leaf, q, gc, { watch: [store.parent(leaf)] });
371+
assert.equal(out.total, 1, `${q}: the one file matches`);
372+
assert.equal(out.hits[0].path, leafPath, `${q}: its path, all ${depth} folders of it`);
373+
assert.equal(probes, 1, `${q}: the deep path is built once, from its folder's`);
374+
assert.ok(held < HELD_PER_FOLDER * depth, heldMessage(q, held, depth, leafPath));
375+
}
376+
315377
test('CI4: a deep hit costs memory in proportion to its depth, not its square — with a path term or without, no folder on its way down keeps a whole copy of its path', async () => {
316378
// CI run 39095045009: the 30k-deep chain above ran macOS's 2 GB heap out
317379
// (it passed on this Mac's 4 GB). Each folder's path, built from its
@@ -320,34 +382,63 @@ test('CI4: a deep hit costs memory in proportion to its depth, not its square
320382
// copy of its own path: the depth squared, about 2.7 GB at 30k. A path
321383
// term reads every node's path whole anyway, and a folder kept reachable
322384
// from below after its path was built then held that copy alive.
323-
// Measured where the deep path is built: the most the walk holds at once.
324-
const DEPTH = 8_000;
325385
const gc = collector();
326-
for (const q of ['name:leaf', 'path:leaf']) {
327-
const { scan, store, leaf, leafPath } = deepChain(DEPTH);
328-
await run(scan, q); // whatever the first query loads is not the walk's
329-
let probes = 0;
330-
let held = -1;
331-
const base = heldBytes(gc);
332-
scan.store = new Proxy(store, {
333-
get(target, prop, receiver) {
334-
const value = Reflect.get(target, prop, receiver);
335-
if (typeof value !== 'function') return value;
336-
if (prop !== 'childPath') return (value as (...a: unknown[]) => unknown).bind(target);
337-
return (child: number, parentPath: string): string => {
338-
const built = (value as (c: number, p: string) => string).call(target, child, parentPath);
339-
if (child === leaf) { probes++; held = heldBytes(gc) - base; }
340-
return built;
341-
};
342-
},
343-
}) as ScanStore;
344-
const out = await run(scan, q);
345-
assert.equal(out.total, 1, `${q}: the one file matches`);
346-
assert.equal(out.hits[0].path, leafPath, `${q}: its path, all ${DEPTH} folders of it`);
347-
assert.equal(probes, 1, `${q}: the deep path is built once, from its folder's`);
348-
assert.ok(
349-
held < HELD_PER_FOLDER * DEPTH,
350-
`${q}: ${(held / 1e6).toFixed(1)} MB held as the deep path is built, ${Math.round(held / DEPTH)} bytes per folder of ${DEPTH} (at most ${HELD_PER_FOLDER}; a whole copy of every folder's path is about ${Math.round(leafPath.length / 2)})`,
351-
);
386+
for (const q of ['name:leaf', 'path:leaf']) await heldAsTheDeepPathIsBuilt(8_000, false, q, gc);
387+
});
388+
389+
test('CI4 review F1.2: a file waiting beside every folder of a deep chain keeps no folder\'s whole path alive — with a path term or an in: term reading every path, or neither', async (t) => {
390+
// Every folder holds a file listed before its subfolder. The walk pushes a
391+
// folder's children in order and takes the last first, so each file waits
392+
// on its stack while everything under its folder is walked, and it needs
393+
// its folder's path for its own. It held the folder's node for that, and
394+
// with it the folder's path, which a path: or in: term had read whole: the
395+
// depth squared again — 185 MB at 8,000 deep, and a 2 GB heap run out at
396+
// 30,000 — though the chain without the files held a few bytes a folder.
397+
const gc = collector();
398+
for (const q of ['path:leaf', 'in:/deep/d0 name:leaf', 'name:leaf']) {
399+
await t.test(q, () => heldAsTheDeepPathIsBuilt(8_000, true, q, gc));
400+
}
401+
});
402+
403+
/**
404+
* deepChain(depth, true) with its sizes set by hand, two folders at a time:
405+
* d0 and d1 hold 1 byte, d2 and d3 hold 2, and so on; every file holds none.
406+
*/
407+
function pairedChain(depth: number): { scan: ScanResult; store: PackedScanStore; deepest: number; keptPath: string } {
408+
const store = new PackedScanStore('/deep', '/', { name: 'deep', isDir: true, size: 0, modifiedAt: 1, isHidden: false });
409+
let parent = store.rootId;
410+
for (let i = 0; i < depth; i++) {
411+
store.addNode(parent, { name: `f${i}.bin`, isDir: false, size: 0, modifiedAt: 1, isHidden: false, extension: 'bin' });
412+
parent = store.addNode(parent, { name: `d${i}`, isDir: true, size: Math.floor(i / 2) + 1, modifiedAt: 1, isHidden: false });
352413
}
414+
store.addNode(parent, { name: 'leaf.bin', isDir: false, size: 0, modifiedAt: 1, isHidden: false, extension: 'bin' });
415+
store.finalize();
416+
const scan = createScanRecord('/deep');
417+
scan.store = store;
418+
scan.status = 'complete';
419+
scan.engine = 'walker';
420+
let deepest = store.rootId;
421+
for (let next = store.rootId; next !== -1; next = store.childIds(next).find((k) => store.isDir(k)) ?? -1) deepest = next;
422+
let keptPath = '/deep';
423+
for (let i = 0; i < depth - 1; i++) keptPath += `/d${i}`;
424+
return { scan, store, deepest, keptPath };
425+
}
426+
427+
test('CI4 review F1.2: a sort tie that reads a kept folder\'s path while its file still waits leaves the path that file holds in pieces', async () => {
428+
// Folders only, the largest kept, one: each even folder is kept as it is
429+
// met, being larger than the one kept before, and the odd folder inside it
430+
// is the same size — a tie, broken by path, so the kept folder's path is
431+
// read after its children were pushed, while its file still waits. Were
432+
// its own path and the one its children join onto one string while the
433+
// walk runs, that read would copy the joined one whole, and every second
434+
// folder's would stay whole on the stack: the depth squared.
435+
const DEPTH = 8_000; // even, so the deepest folder ties with the one above it as it is met
436+
const gc = collector();
437+
const { scan, store, deepest, keptPath } = pairedChain(DEPTH);
438+
assert.equal(store.size(deepest), DEPTH / 2, 'the deepest folder is the size of the one above it');
439+
const { out, probes, held } = await heldAsPathIsBuilt(scan, store, deepest, 'type:dir', gc, { limit: 1 });
440+
assert.equal(out.total, DEPTH + 1, 'every folder matches, the scanned one too');
441+
assert.deepEqual(out.hits.map((h) => h.path), [keptPath], 'the largest, the tie broken by path: the folder above the deepest');
442+
assert.equal(probes, 1, 'the deepest folder\'s path is built once, for its tie');
443+
assert.ok(held < HELD_PER_FOLDER * DEPTH, heldMessage('type:dir', held, DEPTH, keptPath));
353444
});

0 commit comments

Comments
 (0)