Skip to content

Commit

Permalink
feat: add option to disable run/wait time recording
Browse files Browse the repository at this point in the history
An additional `Piscina` constructor option is now available named `recordTiming`
which can be used to control the underlying recording of wait and run times
via Node.js histograms. By default this option is `true` to retain existing behavior
but can be set to `false` to disable recording. The `Piscina` instance properties
`runTime`, `waitTime`, and `utilization` will not be available if this option is
disabled.
  • Loading branch information
clydin committed Feb 20, 2024
1 parent 436b6ce commit 0c3912b
Show file tree
Hide file tree
Showing 3 changed files with 81 additions and 12 deletions.
2 changes: 2 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -358,6 +358,8 @@ This class extends [`EventEmitter`][] from Node.js.
all Node.js versions higher than 14.6.0).
* `closeTimeout`: (`number`) An optional time (in milliseconds) to wait for the pool to
complete all in-flight tasks when `close()` is called. The default is `30000`
* `recordTiming`: (`boolean`) By default, run and wait time will be recorded
for the pool. To disable, set to `false`.

Use caution when setting resource limits. Setting limits that are too low may
result in the `Piscina` worker threads being unusable.
Expand Down
43 changes: 32 additions & 11 deletions src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -177,7 +177,8 @@ interface Options {
taskQueue? : TaskQueue,
niceIncrement? : number,
trackUnmanagedFds? : boolean,
closeTimeout?: number
closeTimeout?: number,
recordTiming?: boolean
}

interface FilledOptions extends Options {
Expand All @@ -191,7 +192,8 @@ interface FilledOptions extends Options {
useAtomics: boolean,
taskQueue : TaskQueue,
niceIncrement : number,
closeTimeout : number
closeTimeout : number,
recordTiming : boolean
}

const kDefaultOptions : FilledOptions = {
Expand All @@ -206,7 +208,8 @@ const kDefaultOptions : FilledOptions = {
taskQueue: new ArrayTaskQueue(),
niceIncrement: 0,
trackUnmanagedFds: true,
closeTimeout: 30000
closeTimeout: 30000,
recordTiming: true
};

interface RunOptions {
Expand Down Expand Up @@ -457,7 +460,9 @@ const Errors = {
NoTaskQueueAvailable:
() => new Error('No task queue available and all Workers are busy'),
CloseTimeout:
() => new Error('Close operation timed out')
() => new Error('Close operation timed out'),
TimeRecordingDisabled:
() => new Error('Time recording is disabled')
};

class WorkerInfo extends AsynchronouslyCreatedResource {
Expand Down Expand Up @@ -590,8 +595,8 @@ class ThreadPool {
taskQueue : TaskQueue;
skipQueue : TaskInfo[] = [];
completed : number = 0;
runTime : RecordableHistogram;
waitTime : RecordableHistogram;
runTime? : RecordableHistogram;
waitTime? : RecordableHistogram;
needsDrain : boolean;
start : number = performance.now();
inProcessPendingMessages : boolean = false;
Expand All @@ -603,12 +608,16 @@ class ThreadPool {
constructor (publicInterface : Piscina, options : Options) {
this.publicInterface = publicInterface;
this.taskQueue = options.taskQueue || new ArrayTaskQueue();
this.runTime = createHistogram();
this.waitTime = createHistogram();

const filename =
options.filename ? maybeFileURLToPath(options.filename) : null;
this.options = { ...kDefaultOptions, ...options, filename, maxQueue: 0 };

if (this.options.recordTiming) {
this.runTime = createHistogram();
this.waitTime = createHistogram();
}

// The >= and <= could be > and < but this way we get 100 % coverage 🙃
if (options.maxThreads !== undefined &&
this.options.minThreads >= options.maxThreads) {
Expand Down Expand Up @@ -805,7 +814,7 @@ class ThreadPool {
break;
}
const now = performance.now();
this.waitTime.record(toHistogramIntegerNano(now - taskInfo.created));
this.waitTime?.record(toHistogramIntegerNano(now - taskInfo.created));
taskInfo.started = now;
workerInfo.postTask(taskInfo);
this._maybeDrain();
Expand Down Expand Up @@ -866,7 +875,7 @@ class ThreadPool {
(err : Error | null, result : any) => {
this.completed++;
if (taskInfo.started) {
this.runTime.record(toHistogramIntegerNano(performance.now() - taskInfo.started));
this.runTime?.record(toHistogramIntegerNano(performance.now() - taskInfo.started));
}
if (err !== null) {
reject(err);
Expand Down Expand Up @@ -956,7 +965,7 @@ class ThreadPool {

// TODO(addaleax): Clean up the waitTime/runTime recording.
const now = performance.now();
this.waitTime.record(toHistogramIntegerNano(now - taskInfo.created));
this.waitTime?.record(toHistogramIntegerNano(now - taskInfo.created));
taskInfo.started = now;
workerInfo.postTask(taskInfo);
this._maybeDrain();
Expand Down Expand Up @@ -1267,14 +1276,26 @@ class Piscina extends EventEmitterAsyncResource {
}

get waitTime () : any {
if (!this.#pool.waitTime) {
throw Errors.TimeRecordingDisabled();
}

return createHistogramSummary(this.#pool.waitTime);
}

get runTime () : any {
if (!this.#pool.runTime) {
throw Errors.TimeRecordingDisabled();
}

return createHistogramSummary(this.#pool.runTime);
}

get utilization () : number {
if (!this.#pool.runTime) {
throw Errors.TimeRecordingDisabled();
}

// count is available as of Node.js v16.14.0 but not present in the types
const count = (this.#pool.runTime as RecordableHistogram & { count: number}).count;
if (count === 0) {
Expand Down
48 changes: 47 additions & 1 deletion test/histogram.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import Piscina from '..';
import { test } from 'tap';
import { resolve } from 'path';

test('pool will maintain run and wait time histograms', async ({ equal, ok }) => {
test('pool will maintain run and wait time histograms by default', async ({ equal, ok }) => {
const pool = new Piscina({
filename: resolve(__dirname, 'fixtures/eval.js')
});
Expand All @@ -29,3 +29,49 @@ test('pool will maintain run and wait time histograms', async ({ equal, ok }) =>
equal(typeof runTime.min, 'number');
equal(typeof runTime.max, 'number');
});

test('pool will maintain run and wait time histograms when recordTiming is true', async ({ ok }) => {
const pool = new Piscina({
filename: resolve(__dirname, 'fixtures/eval.js'),
recordTiming: true
});

const tasks = [];
for (let n = 0; n < 10; n++) {
tasks.push(pool.runTask('42'));
}
await Promise.all(tasks);

const waitTime = pool.waitTime as any;
ok(waitTime);

const runTime = pool.runTime as any;
ok(runTime);
});

test('pool does not maintain run and wait time histograms when recordTiming is false', async ({ equal, fail }) => {
const pool = new Piscina({
filename: resolve(__dirname, 'fixtures/eval.js'),
recordTiming: false
});

const tasks = [];
for (let n = 0; n < 10; n++) {
tasks.push(pool.runTask('42'));
}
await Promise.all(tasks);

try {
pool.waitTime as any;
fail('Expected time recording disabled error');
} catch (error) {
equal(error.message, 'Time recording is disabled');
}

try {
pool.runTime as any;
fail('Expected time recording disabled error');
} catch (error) {
equal(error.message, 'Time recording is disabled');
}
});

0 comments on commit 0c3912b

Please sign in to comment.