Skip to content

Commit 1c16d08

Browse files
avivkelleravivkeller
committed
code review
1 parent 0da3e20 commit 1c16d08

File tree

5 files changed

+124
-111
lines changed

5 files changed

+124
-111
lines changed

‎bin/cli.mjs‎

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -2,21 +2,22 @@
22

33
import { resolve } from 'node:path';
44
import process from 'node:process';
5+
import { cpus } from 'node:os';
56

67
import { Command, Option } from 'commander';
78

89
import { coerce } from 'semver';
910
import { DOC_NODE_CHANGELOG_URL, DOC_NODE_VERSION } from '../src/constants.mjs';
1011
import createGenerator from '../src/generators.mjs';
11-
import generators from '../src/generators/index.mjs';
12+
import { publicGenerators } from '../src/generators/index.mjs';
1213
import createLinter from '../src/linter/index.mjs';
1314
import reporters from '../src/linter/reporters/index.mjs';
1415
import rules from '../src/linter/rules/index.mjs';
1516
import createMarkdownLoader from '../src/loaders/markdown.mjs';
1617
import createMarkdownParser from '../src/parsers/markdown.mjs';
1718
import createNodeReleases from '../src/releases.mjs';
1819

19-
const availableGenerators = Object.keys(generators);
20+
const availableGenerators = Object.keys(publicGenerators);
2021

2122
const program = new Command();
2223

@@ -79,14 +80,14 @@ program
7980
)
8081
.addOption(
8182
new Option(
82-
'--disable-parallelism',
83-
'Disable the use of multiple threads'
84-
).default(false)
83+
'-p, --threads <number>',
84+
'The maximum number of threads to use. Set to 1 to disable parallelism'
85+
).default(Math.max(1, cpus().length - 1))
8586
)
8687
.parse(process.argv);
8788

8889
/**
89-
* @typedef {keyof generators} Target A list of the available generator names.
90+
* @typedef {keyof publicGenerators} Target A list of the available generator names.
9091
*
9192
* @typedef {Object} Options
9293
* @property {Array<string>|string} input Specifies the glob/path for input files.
@@ -114,7 +115,7 @@ const {
114115
lintDryRun,
115116
gitRef,
116117
reporter,
117-
disableParallelism,
118+
threads,
118119
} = program.opts();
119120

120121
const linter = createLinter(lintDryRun, disableRule);
@@ -149,8 +150,8 @@ if (target) {
149150
// An URL containing a git ref URL pointing to the commit or ref that was used
150151
// to generate the API docs. This is used to link to the source code of the
151152
gitRef,
152-
// Disable the use of parallel threads
153-
disableParallelism,
153+
// How many threads should be used
154+
threads,
154155
});
155156
}
156157

‎src/generators.mjs‎

Lines changed: 8 additions & 99 deletions
Original file line numberDiff line numberDiff line change
@@ -1,38 +1,7 @@
11
'use strict';
22

3-
import { Worker, isMainThread, parentPort, workerData } from 'worker_threads';
4-
import os from 'os';
5-
6-
import publicGenerators from './generators/index.mjs';
7-
import astJs from './generators/ast-js/index.mjs';
8-
import oramaDb from './generators/orama-db/index.mjs';
9-
10-
const availableGenerators = {
11-
...publicGenerators,
12-
// This one is a little special since we don't want it to run unless we need
13-
// it and we also don't want it to be publicly accessible through the CLI.
14-
'ast-js': astJs,
15-
'orama-db': oramaDb,
16-
};
17-
18-
// Thread pool max limit
19-
const MAX_THREADS = Math.max(1, os.cpus().length - 1);
20-
21-
// If inside a worker thread, perform the generator logic here
22-
if (!isMainThread) {
23-
const { name, dependencyOutput, extra } = workerData;
24-
const generator = availableGenerators[name];
25-
26-
// Execute the generator and send the result back to the parent thread
27-
generator
28-
.generate(dependencyOutput, extra)
29-
.then(result => {
30-
parentPort.postMessage(result);
31-
})
32-
.catch(error => {
33-
parentPort.postMessage({ error });
34-
});
35-
}
3+
import { allGenerators } from './generators/index.mjs';
4+
import { WorkerPool } from './threading.mjs';
365

376
/**
387
* @typedef {{ ast: GeneratorMetadata<ApiDocMetadataEntry, ApiDocMetadataEntry>}} AstGenerator The AST "generator" is a facade for the AST tree and it isn't really a generator
@@ -65,74 +34,14 @@ const createGenerator = markdownInput => {
6534
*/
6635
const cachedGenerators = { ast: Promise.resolve(markdownInput) };
6736

68-
// Keep track of how many threads are currently running
69-
let activeThreads = 0;
70-
const threadQueue = [];
71-
72-
/**
73-
* Run the input generator within a worker thread
74-
* @param {keyof AllGenerators} name
75-
* @param {any} dependencyOutput
76-
* @param {Partial<GeneratorOptions>} extra
77-
*/
78-
const runInWorker = (name, dependencyOutput, extra) => {
79-
return new Promise((resolve, reject) => {
80-
/**
81-
* Run the generator
82-
*/
83-
const run = () => {
84-
activeThreads++;
85-
86-
const worker = new Worker(new URL(import.meta.url), {
87-
workerData: { name, dependencyOutput, extra },
88-
});
89-
90-
worker.on('message', result => {
91-
activeThreads--;
92-
processQueue();
93-
94-
if (result && result.error) {
95-
reject(result.error);
96-
} else {
97-
resolve(result);
98-
}
99-
});
100-
101-
worker.on('error', err => {
102-
activeThreads--;
103-
processQueue();
104-
reject(err);
105-
});
106-
};
107-
108-
if (activeThreads >= MAX_THREADS) {
109-
threadQueue.push(run);
110-
} else {
111-
run();
112-
}
113-
});
114-
};
115-
116-
/**
117-
* Process the worker thread queue
118-
*/
119-
const processQueue = () => {
120-
if (threadQueue.length > 0 && activeThreads < MAX_THREADS) {
121-
const next = threadQueue.shift();
122-
next();
123-
}
124-
};
37+
const threadPool = new WorkerPool();
12538

12639
/**
12740
* Runs the Generator engine with the provided top-level input and the given generator options
12841
*
12942
* @param {GeneratorOptions} options The options for the generator runtime
13043
*/
131-
const runGenerators = async ({
132-
generators,
133-
disableParallelism = false,
134-
...extra
135-
}) => {
44+
const runGenerators = async ({ generators, threads, ...extra }) => {
13645
// Note that this method is blocking, and will only execute one generator per-time
13746
// but it ensures all dependencies are resolved, and that multiple bottom-level generators
13847
// can reuse the already parsed content from the top-level/dependency generators
@@ -141,14 +50,14 @@ const createGenerator = markdownInput => {
14150
dependsOn,
14251
generate,
14352
parallizable = true,
144-
} = availableGenerators[generatorName];
53+
} = allGenerators[generatorName];
14554

14655
// If the generator dependency has not yet been resolved, we resolve
14756
// the dependency first before running the current generator
14857
if (dependsOn && !(dependsOn in cachedGenerators)) {
14958
await runGenerators({
15059
...extra,
151-
disableParallelism,
60+
threads,
15261
generators: [dependsOn],
15362
});
15463
}
@@ -159,9 +68,9 @@ const createGenerator = markdownInput => {
15968

16069
// Adds the current generator execution Promise to the cache
16170
cachedGenerators[generatorName] =
162-
disableParallelism || !parallizable
71+
threads < 2 || !parallizable
16372
? generate(dependencyOutput, extra) // Run in main thread
164-
: runInWorker(generatorName, dependencyOutput, extra); // Offload to worker thread
73+
: threadPool.run(generatorName, dependencyOutput, threads, extra); // Offload to worker thread
16574
}
16675

16776
// Returns the value of the last generator of the current pipeline

‎src/generators/index.mjs‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,8 +9,9 @@ import legacyJsonAll from './legacy-json-all/index.mjs';
99
import addonVerify from './addon-verify/index.mjs';
1010
import apiLinks from './api-links/index.mjs';
1111
import oramaDb from './orama-db/index.mjs';
12+
import astJs from './ast-js/index.mjs';
1213

13-
export default {
14+
export const publicGenerators = {
1415
'json-simple': jsonSimple,
1516
'legacy-html': legacyHtml,
1617
'legacy-html-all': legacyHtmlAll,
@@ -21,3 +22,10 @@ export default {
2122
'api-links': apiLinks,
2223
'orama-db': oramaDb,
2324
};
25+
26+
export const allGenerators = {
27+
...publicGenerators,
28+
// This one is a little special since we don't want it to run unless we need
29+
// it and we also don't want it to be publicly accessible through the CLI.
30+
'ast-js': astJs,
31+
};

‎src/generators/types.d.ts‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,11 @@
11
import type { SemVer } from 'semver';
22
import type { ApiDocReleaseEntry } from '../types';
3-
import type availableGenerators from './index.mjs';
3+
import type { publicGenerators } from './index.mjs';
44

55
declare global {
66
// All available generators as an inferable type, to allow Generator interfaces
77
// to be type complete and runtime friendly within `runGenerators`
8-
export type AvailableGenerators = typeof availableGenerators;
8+
export type AvailableGenerators = typeof publicGenerators;
99

1010
// This is the runtime config passed to the API doc generators
1111
export interface GeneratorOptions {
@@ -36,6 +36,9 @@ declare global {
3636
// i.e. https://github.com/nodejs/node/tree/2cb1d07e0f6d9456438016bab7db4688ab354fd2
3737
// i.e. https://gitlab.com/someone/node/tree/HEAD
3838
gitRef: string;
39+
40+
// The number of threads the process is allowed to use
41+
threads: number;
3942
}
4043

4144
export interface GeneratorMetadata<I extends any, O extends any> {

‎src/threading.mjs‎

Lines changed: 92 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,92 @@
1+
import {
2+
Worker,
3+
isMainThread,
4+
parentPort,
5+
workerData,
6+
} from 'node:worker_threads';
7+
import { allGenerators } from './generators/index.mjs';
8+
9+
// If inside a worker thread, perform the generator logic here
10+
if (!isMainThread) {
11+
const { name, dependencyOutput, extra } = workerData;
12+
const generator = allGenerators[name];
13+
14+
// Execute the generator and send the result or error back to the parent thread
15+
generator
16+
.generate(dependencyOutput, extra)
17+
.then(result => parentPort.postMessage(result))
18+
.catch(error => parentPort.postMessage({ error }));
19+
}
20+
21+
/**
22+
* WorkerPool class to manage a pool of worker threads
23+
*/
24+
export class WorkerPool {
25+
/** @private {number} - Number of active threads */
26+
activeThreads = 0;
27+
/** @private {Array<Function>} - Queue of pending tasks */
28+
queue = [];
29+
30+
/**
31+
* Runs a generator within a worker thread.
32+
* @param {string} name - The name of the generator to execute
33+
* @param {any} dependencyOutput - Input data for the generator
34+
* @param {number} threads - Maximum number of threads to run concurrently
35+
* @param {Object} extra - Additional options for the generator
36+
* @returns {Promise<any>} Resolves with the generator result, or rejects with an error
37+
*/
38+
run(name, dependencyOutput, threads, extra) {
39+
return new Promise((resolve, reject) => {
40+
/**
41+
* Function to run the generator in a worker thread
42+
*/
43+
const run = () => {
44+
this.activeThreads++;
45+
46+
// Create and start the worker thread
47+
const worker = new Worker(new URL(import.meta.url), {
48+
workerData: { name, dependencyOutput, extra },
49+
});
50+
51+
// Handle worker thread messages (result or error)
52+
worker.on('message', result => {
53+
this.activeThreads--;
54+
this.processQueue(threads);
55+
56+
if (result?.error) {
57+
reject(result.error);
58+
} else {
59+
resolve(result);
60+
}
61+
});
62+
63+
// Handle worker thread errors
64+
worker.on('error', err => {
65+
this.activeThreads--;
66+
this.processQueue(threads);
67+
reject(err);
68+
});
69+
};
70+
71+
// If the active thread count exceeds the limit, add the task to the queue
72+
if (this.activeThreads >= threads) {
73+
this.queue.push(run);
74+
} else {
75+
run();
76+
}
77+
});
78+
}
79+
80+
/**
81+
* Process the worker thread queue to start the next available task
82+
* when there is room for more threads.
83+
* @param {number} threads - Maximum number of threads to run concurrently
84+
* @private
85+
*/
86+
processQueue(threads) {
87+
if (this.queue.length > 0 && this.activeThreads < threads) {
88+
const next = this.queue.shift();
89+
if (next) next();
90+
}
91+
}
92+
}

0 commit comments

Comments
 (0)