Skip to content

Commit 854a944

Browse files
fix(core): make generator output ordering deterministic (#1193)
1 parent 9e21017 commit 854a944

7 files changed

Lines changed: 174 additions & 41 deletions

File tree

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'@doc-kit/core': patch
3+
---
4+
5+
Keep worker results and discovered source files in deterministic order so repeated documentation builds produce stable output.
Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
import assert from 'node:assert/strict';
2+
import { mkdtemp, rm, writeFile } from 'node:fs/promises';
3+
import { tmpdir } from 'node:os';
4+
import { join, sep } from 'node:path';
5+
import { it } from 'node:test';
6+
7+
import { setConfig } from '#utils/configuration/index.mjs';
8+
9+
import { generate } from '../generate.mjs';
10+
11+
it('submits JavaScript files in sorted path order', async () => {
12+
const dir = await mkdtemp(join(tmpdir(), 'ast-js-order-'));
13+
14+
try {
15+
for (const name of ['z.js', 'a.js', 'm.js', 'ignore.md']) {
16+
await writeFile(join(dir, name), '');
17+
}
18+
19+
const input = join(dir, '*').split(sep).join('/');
20+
await setConfig({ target: ['ast-js'], input: [input] });
21+
22+
let received;
23+
const worker = {
24+
async *stream(items) {
25+
received = items;
26+
yield [];
27+
},
28+
};
29+
30+
for await (const chunk of generate(undefined, worker)) {
31+
assert.deepStrictEqual(chunk, []);
32+
}
33+
34+
assert.deepStrictEqual(
35+
received.map(path => path.replaceAll('\\', '/').split('/').at(-1)),
36+
['a.js', 'm.js', 'z.js']
37+
);
38+
} finally {
39+
await rm(dir, { recursive: true, force: true });
40+
}
41+
});

‎packages/core/src/generators/ast-js/generate.mjs‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -44,9 +44,9 @@ export async function processChunk(inputSlice, itemIndices) {
4444
export async function* generate(_, worker) {
4545
const config = getConfig('ast-js');
4646

47-
const files = findFiles(config.input, { ignore: config.ignore }).filter(
48-
p => extname(p) === '.js'
49-
);
47+
const files = findFiles(config.input, { ignore: config.ignore })
48+
.filter(p => extname(p) === '.js')
49+
.sort();
5050

5151
// Parse the Javascript sources into ASTs in parallel using worker threads
5252
// source is both the items list and the fullInput since we use sliceInput

‎packages/core/src/generators/ast/__tests__/generate.test.mjs‎

Lines changed: 46 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,16 @@
11
import assert from 'node:assert/strict';
2-
import { mkdtemp, writeFile, rm } from 'node:fs/promises';
2+
import { mkdir, mkdtemp, writeFile, rm } from 'node:fs/promises';
33
import { tmpdir } from 'node:os';
44
import { join, sep } from 'node:path';
55
import { after, before, describe, it } from 'node:test';
66
import { pathToFileURL } from 'node:url';
77

88
import { loadGenerator } from '#generators/loader.mjs';
9+
import { setConfig } from '#utils/configuration/index.mjs';
910
import { loadMarkdownPlugins } from '#utils/markdown/plugins.mjs';
1011

1112
import { STABILITY_INDEX_URL } from '../constants.mjs';
12-
import { processChunk } from '../generate.mjs';
13+
import { generate, processChunk } from '../generate.mjs';
1314

1415
const ast = await loadGenerator(import.meta.resolve('../index.mjs'));
1516

@@ -216,3 +217,46 @@ describe('processChunk', () => {
216217
});
217218
});
218219
});
220+
221+
describe('generate', () => {
222+
it('sorts files inside each pattern without reordering configured patterns', async () => {
223+
const first = join(dir, 'ordered-first');
224+
const second = join(dir, 'ordered-second');
225+
await mkdir(first);
226+
await mkdir(second);
227+
228+
for (const name of ['z.md', 'a.md', 'm.md']) {
229+
await writeFile(join(first, name), '# Title\n');
230+
await writeFile(join(second, name), '# Title\n');
231+
}
232+
233+
await setConfig({
234+
target: ['ast'],
235+
input: [
236+
toPosixPath(join(second, '*.md')),
237+
toPosixPath(join(first, '*.md')),
238+
],
239+
});
240+
241+
let received;
242+
const worker = {
243+
async *stream(items) {
244+
received = items;
245+
yield [];
246+
},
247+
};
248+
249+
for await (const chunk of generate(undefined, worker)) {
250+
assert.deepStrictEqual(chunk, []);
251+
}
252+
253+
assert.deepStrictEqual(
254+
received.map(([child]) =>
255+
child.replaceAll('\\', '/').split('/').slice(-2).join('/')
256+
),
257+
['ordered-second', 'ordered-first'].flatMap(parent =>
258+
['a.md', 'm.md', 'z.md'].map(name => `${parent}/${name}`)
259+
)
260+
);
261+
});
262+
});

‎packages/core/src/generators/ast/generate.mjs‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -116,10 +116,9 @@ export async function* generate(_, worker) {
116116
const files = config.input.flatMap(input => {
117117
const parent = globParent(input);
118118

119-
return findFiles(input, { ignore: config.ignore }).map(child => [
120-
child,
121-
parent,
122-
]);
119+
return findFiles(input, { ignore: config.ignore })
120+
.sort()
121+
.map(child => [child, parent]);
123122
});
124123

125124
// Parse markdown files in parallel using worker threads

‎packages/core/src/threading/__tests__/parallel.test.mjs‎

Lines changed: 54 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { deepStrictEqual, ok, strictEqual } from 'node:assert';
1+
import { deepStrictEqual, ok, rejects, strictEqual } from 'node:assert';
22
import { describe, it } from 'node:test';
33

44
import { loadGenerator } from '../../generators/loader.mjs';
@@ -106,7 +106,7 @@ describe('createParallelWorker', () => {
106106
await pool.destroy();
107107
});
108108

109-
it('should yield results as chunks complete', async () => {
109+
it('should yield one result per input chunk', async () => {
110110
const pool = createWorkerPool(2);
111111
const worker = createParallelWorker('metadata', metadataGenerator, pool, {
112112
threads: 2,
@@ -177,4 +177,56 @@ describe('createParallelWorker', () => {
177177

178178
await pool.destroy();
179179
});
180+
181+
it('yields concurrent worker results in submission order', async () => {
182+
const resolvers = [];
183+
const pool = {
184+
run: task =>
185+
new Promise(resolve => {
186+
resolvers.push(() => resolve(task.input));
187+
}),
188+
};
189+
const worker = createParallelWorker('ast-js', astJsGenerator, pool, {
190+
threads: 2,
191+
chunkSize: 1,
192+
});
193+
194+
const result = collectStream(worker.stream([0, 1, 2, 3, 4], {}));
195+
await Promise.resolve();
196+
197+
// All five tasks start before any result is yielded, but finish in reverse.
198+
strictEqual(resolvers.length, 5);
199+
for (const resolve of resolvers.toReversed()) {
200+
resolve();
201+
}
202+
203+
deepStrictEqual(await result, [0, 1, 2, 3, 4]);
204+
});
205+
206+
it('propagates a later worker failure without an unhandled rejection', async () => {
207+
const tasks = [];
208+
const pool = {
209+
run: task =>
210+
new Promise((resolve, reject) => {
211+
tasks.push({ resolve, reject, input: task.input });
212+
}),
213+
};
214+
const worker = createParallelWorker('ast-js', astJsGenerator, pool, {
215+
threads: 2,
216+
chunkSize: 1,
217+
});
218+
219+
const result = collectStream(worker.stream([0, 1, 2, 3], {}));
220+
await Promise.resolve();
221+
strictEqual(tasks.length, 4);
222+
223+
tasks[2].reject(new Error('chunk failed'));
224+
for (const [index, task] of tasks.entries()) {
225+
if (index !== 2) {
226+
task.resolve(task.input);
227+
}
228+
}
229+
230+
await rejects(result, /chunk failed/);
231+
});
180232
});

‎packages/core/src/threading/parallel.mjs‎

Lines changed: 22 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -77,12 +77,12 @@ export default function createParallelWorker(
7777

7878
return {
7979
/**
80-
* Processes items in parallel, yielding results as chunks complete.
80+
* Processes items in parallel, yielding results in submission order.
8181
*
8282
* @template T
8383
* @param {T[]} items - Items to process
8484
* @param {object} extra - Extra options
85-
* @yields {R[]} Chunk results as they complete
85+
* @yields {R[]} Chunk results in input order
8686
*/
8787
async *stream(items, extra) {
8888
if (items.length === 0) {
@@ -98,42 +98,34 @@ export default function createParallelWorker(
9898

9999
const runInOneGo = threads <= 1 || items.length <= 2;
100100

101-
// Submit all tasks to Piscina - each promise resolves to itself for removal
102-
const pending = new Set(
103-
chunks.map(indices => {
104-
if (runInOneGo) {
105-
const promise = generator
106-
.processChunk(items, indices, extra)
107-
.then(result => ({ promise, result }));
108-
109-
return promise;
110-
}
111-
112-
const promise = pool
113-
.run(
101+
// Start every chunk immediately, but emit results in submission order.
102+
// Handle errors as each task settles so a later failure cannot become an
103+
// unhandled rejection while earlier chunks are still running.
104+
const tasks = chunks.map(indices => {
105+
const task = runInOneGo
106+
? generator.processChunk(items, indices, extra)
107+
: pool.run(
114108
createTask(items, indices, extra, configuration, specifier, name)
115-
)
116-
.then(result => ({ promise, result }));
117-
118-
return promise;
119-
})
120-
);
121-
122-
// Yield results as they complete (true parallel collection)
123-
let completed = 0;
109+
);
124110

125-
while (pending.size > 0) {
126-
const { promise, result } = await Promise.race(pending);
111+
return task.then(
112+
result => ({ result }),
113+
error => ({ error })
114+
);
115+
});
127116

128-
pending.delete(promise);
117+
for (const [index, task] of tasks.entries()) {
118+
const outcome = await task;
129119

130-
completed++;
120+
if ('error' in outcome) {
121+
throw outcome.error;
122+
}
131123

132-
parallelLogger.debug(`Chunk ${completed}/${chunks.length} completed`, {
124+
parallelLogger.debug(`Chunk ${index + 1}/${chunks.length} completed`, {
133125
generator: name,
134126
});
135127

136-
yield result;
128+
yield outcome.result;
137129
}
138130
},
139131
};

0 commit comments

Comments
 (0)