From 1927ee6758147bb8a2479b987322564cea20992b Mon Sep 17 00:00:00 2001 From: Alessandro Pio Ardizio Date: Tue, 23 Feb 2021 09:33:46 +0100 Subject: [PATCH] Removed max tasks (#225) * Removed max tasks * Coverage on abstract pool default * Remove default option from the abstract pool since the default is already specified in any child class * Fix missing default opts * Few small changes * Remove default from abstract pool constructor JSDoc Co-authored-by: Shinigami --- CHANGELOG.md | 4 +++- README.md | 1 - benchmarks/internal/cluster/dynamic.js | 5 +---- benchmarks/internal/cluster/fixed.js | 5 +---- benchmarks/internal/thread/dynamic.js | 4 +--- benchmarks/internal/thread/fixed.js | 4 +--- .../versus-external-pools/dynamic-poolifier.js | 5 +---- benchmarks/versus-external-pools/fixed-poolifier.js | 5 +---- src/pools/abstract-pool.ts | 13 ++----------- src/pools/cluster/dynamic.ts | 4 ++-- src/pools/cluster/fixed.ts | 8 +++----- src/pools/thread/dynamic.ts | 4 ++-- src/pools/thread/fixed.ts | 8 +++----- tests/pools/abstract/abstract-pool.test.js | 5 +---- tests/pools/cluster/fixed.test.js | 11 +---------- tests/pools/selection-strategies.test.js | 5 +---- tests/pools/thread/fixed.test.js | 9 +-------- 17 files changed, 25 insertions(+), 75 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 03763a6e..b838afe8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,9 +13,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Breaking Changes -- `maxInactiveTime` default behavior is now changed, if you want to keep the old behavior set `killBehavior` to `KillBehaviors.HARD`. +- `maxInactiveTime` on `ThreadWorker` default behavior is now changed, if you want to keep the old behavior set `killBehavior` to `KillBehaviors.HARD`. _Find more details on our JSDoc._ +- `maxTasks` option on `FixedThreadPool` and `DynamicThreadPool` is now removed since is no more needed. + - We changed some internal structures, but you shouldn't be too affected by them as these are internal changes. ### Pool options types declaration merge diff --git a/README.md b/README.md index 96008eb2..83ec2122 100644 --- a/README.md +++ b/README.md @@ -155,7 +155,6 @@ You can use node versions 12.x, 13.x, 14.x - `errorHandler` - A function that will listen for error event on each worker - `onlineHandler` - A function that will listen for online event on each worker - `exitHandler` - A function that will listen for exit event on each worker -- `maxTasks` - This is just to avoid not useful warnings message, is used to set [maxListeners](https://nodejs.org/dist/latest-v12.x/docs/api/events.html#events_emitter_setmaxlisteners_n) on event emitters (workers are event emitters) ### `pool = new DynamicThreadPool/DynamicClusterPool(min, max, filePath, opts)` diff --git a/benchmarks/internal/cluster/dynamic.js b/benchmarks/internal/cluster/dynamic.js index 468eee3b..7e607c87 100644 --- a/benchmarks/internal/cluster/dynamic.js +++ b/benchmarks/internal/cluster/dynamic.js @@ -5,10 +5,7 @@ const size = 30 const dynamicPool = new DynamicClusterPool( size / 2, size * 3, - './benchmarks/internal/cluster/worker.js', - { - maxTasks: 10000 - } + './benchmarks/internal/cluster/worker.js' ) async function dynamicClusterTest ( diff --git a/benchmarks/internal/cluster/fixed.js b/benchmarks/internal/cluster/fixed.js index f2b80c71..1fd8d679 100644 --- a/benchmarks/internal/cluster/fixed.js +++ b/benchmarks/internal/cluster/fixed.js @@ -4,10 +4,7 @@ const size = 30 const fixedPool = new FixedClusterPool( size, - './benchmarks/internal/cluster/worker.js', - { - maxTasks: 10000 - } + './benchmarks/internal/cluster/worker.js' ) async function fixedClusterTest ( diff --git a/benchmarks/internal/thread/dynamic.js b/benchmarks/internal/thread/dynamic.js index dde75474..f3f0e049 100644 --- a/benchmarks/internal/thread/dynamic.js +++ b/benchmarks/internal/thread/dynamic.js @@ -2,9 +2,7 @@ const { DynamicThreadPool } = require('../../../lib/index') const size = 30 -const dynamicPool = new DynamicThreadPool(size / 2, size * 3, './worker.js', { - maxTasks: 10000 -}) +const dynamicPool = new DynamicThreadPool(size / 2, size * 3, './worker.js') async function dynamicThreadTest ( { tasks, workerData } = { tasks: 1, workerData: { proof: 'ok' } } diff --git a/benchmarks/internal/thread/fixed.js b/benchmarks/internal/thread/fixed.js index 8db41acb..8168a6dc 100644 --- a/benchmarks/internal/thread/fixed.js +++ b/benchmarks/internal/thread/fixed.js @@ -2,9 +2,7 @@ const { FixedThreadPool } = require('../../../lib/index') const size = 30 -const fixedPool = new FixedThreadPool(size, './worker.js', { - maxTasks: 10000 -}) +const fixedPool = new FixedThreadPool(size, './worker.js') async function fixedThreadTest ( { tasks, workerData } = { tasks: 1, workerData: { proof: 'ok' } } diff --git a/benchmarks/versus-external-pools/dynamic-poolifier.js b/benchmarks/versus-external-pools/dynamic-poolifier.js index 6a7406bc..42c06e62 100644 --- a/benchmarks/versus-external-pools/dynamic-poolifier.js +++ b/benchmarks/versus-external-pools/dynamic-poolifier.js @@ -11,10 +11,7 @@ const data = { const dynamicPool = new DynamicThreadPool( size, size * 3, - './workers/poolifier/function-to-bench-worker.js', - { - maxTasks: 10000 - } + './workers/poolifier/function-to-bench-worker.js' ) async function run () { diff --git a/benchmarks/versus-external-pools/fixed-poolifier.js b/benchmarks/versus-external-pools/fixed-poolifier.js index c9946208..345fd806 100644 --- a/benchmarks/versus-external-pools/fixed-poolifier.js +++ b/benchmarks/versus-external-pools/fixed-poolifier.js @@ -10,10 +10,7 @@ const data = { const fixedPool = new FixedThreadPool( size, - './workers/poolifier/function-to-bench-worker.js', - { - maxTasks: 100000 - } + './workers/poolifier/function-to-bench-worker.js' ) async function run () { diff --git a/src/pools/abstract-pool.ts b/src/pools/abstract-pool.ts index 38187edc..9d7e4f0c 100644 --- a/src/pools/abstract-pool.ts +++ b/src/pools/abstract-pool.ts @@ -82,15 +82,6 @@ export interface PoolOptions { * A function that will listen for exit event on each worker. */ exitHandler?: ExitHandler - /** - * This is just to avoid non-useful warning messages. - * - * Will be used to set `maxListeners` on event emitters (workers are event emitters). - * - * @default 1000 - * @see [Node events emitter.setMaxListeners(n)](https://nodejs.org/api/events.html#events_emitter_setmaxlisteners_n) - */ - maxTasks?: number /** * The work choice strategy to use in this pool. */ @@ -152,12 +143,12 @@ export abstract class AbstractPool< * * @param numberOfWorkers Number of workers that this pool should manage. * @param filePath Path to the worker-file. - * @param opts Options for the pool. Default: `{ maxTasks: 1000 }` + * @param opts Options for the pool. */ public constructor ( public readonly numberOfWorkers: number, public readonly filePath: string, - public readonly opts: PoolOptions = { maxTasks: 1000 } + public readonly opts: PoolOptions ) { if (!this.isMain()) { throw new Error('Cannot start a pool from a worker!') diff --git a/src/pools/cluster/dynamic.ts b/src/pools/cluster/dynamic.ts index 5445bd03..4a5f720a 100644 --- a/src/pools/cluster/dynamic.ts +++ b/src/pools/cluster/dynamic.ts @@ -23,13 +23,13 @@ export class DynamicClusterPool< * @param min Minimum number of workers which are always active. * @param max Maximum number of workers that can be created by this pool. * @param filePath Path to an implementation of a `ClusterWorker` file, which can be relative or absolute. - * @param opts Options for this dynamic cluster pool. Default: `{ maxTasks: 1000 }` + * @param opts Options for this dynamic cluster pool. Default: `{}` */ public constructor ( min: number, public readonly max: number, filePath: string, - opts: ClusterPoolOptions = { maxTasks: 1000 } + opts: ClusterPoolOptions = {} ) { super(min, filePath, opts) } diff --git a/src/pools/cluster/fixed.ts b/src/pools/cluster/fixed.ts index 4ec81c07..f651d3ad 100644 --- a/src/pools/cluster/fixed.ts +++ b/src/pools/cluster/fixed.ts @@ -38,12 +38,12 @@ export class FixedClusterPool< * * @param numberOfWorkers Number of workers for this pool. * @param filePath Path to an implementation of a `ClusterWorker` file, which can be relative or absolute. - * @param opts Options for this fixed cluster pool. Default: `{ maxTasks: 1000 }` + * @param opts Options for this fixed cluster pool. Default: `{}` */ public constructor ( numberOfWorkers: number, filePath: string, - public readonly opts: ClusterPoolOptions = { maxTasks: 1000 } + public readonly opts: ClusterPoolOptions = {} ) { super(numberOfWorkers, filePath, opts) } @@ -81,9 +81,7 @@ export class FixedClusterPool< } protected afterWorkerSetup (worker: Worker): void { - // We will attach a listener for every task, - // when task is completed the listener will be removed but to avoid warnings we are increasing the max listeners size - worker.setMaxListeners(this.opts.maxTasks ?? 1000) + // Listen worker messages. this.registerWorkerMessageListener(worker, super.workerListener()) } } diff --git a/src/pools/thread/dynamic.ts b/src/pools/thread/dynamic.ts index fcf4b2ff..04125854 100644 --- a/src/pools/thread/dynamic.ts +++ b/src/pools/thread/dynamic.ts @@ -24,13 +24,13 @@ export class DynamicThreadPool< * @param min Minimum number of threads which are always active. * @param max Maximum number of threads that can be created by this pool. * @param filePath Path to an implementation of a `ThreadWorker` file, which can be relative or absolute. - * @param opts Options for this dynamic thread pool. Default: `{ maxTasks: 1000 }` + * @param opts Options for this dynamic thread pool. Default: `{}` */ public constructor ( min: number, public readonly max: number, filePath: string, - opts: PoolOptions = { maxTasks: 1000 } + opts: PoolOptions = {} ) { super(min, filePath, opts) } diff --git a/src/pools/thread/fixed.ts b/src/pools/thread/fixed.ts index 1c91caa7..6fec9aa0 100644 --- a/src/pools/thread/fixed.ts +++ b/src/pools/thread/fixed.ts @@ -30,12 +30,12 @@ export class FixedThreadPool< * * @param numberOfThreads Number of threads for this pool. * @param filePath Path to an implementation of a `ThreadWorker` file, which can be relative or absolute. - * @param opts Options for this fixed thread pool. Default: `{ maxTasks: 1000 }` + * @param opts Options for this fixed thread pool. Default: `{}` */ public constructor ( numberOfThreads: number, filePath: string, - opts: PoolOptions = { maxTasks: 1000 } + opts: PoolOptions = {} ) { super(numberOfThreads, filePath, opts) } @@ -78,9 +78,7 @@ export class FixedThreadPool< worker.postMessage({ parent: port1 }, [port1]) worker.port1 = port1 worker.port2 = port2 - // We will attach a listener for every task, - // when the task is completed the listener will be removed but to avoid warnings we are increasing the max listeners size. - worker.port2.setMaxListeners(this.opts.maxTasks ?? 1000) + // Listen worker messages. this.registerWorkerMessageListener(worker, super.workerListener()) } } diff --git a/tests/pools/abstract/abstract-pool.test.js b/tests/pools/abstract/abstract-pool.test.js index 482ce5e3..5d6311b9 100644 --- a/tests/pools/abstract/abstract-pool.test.js +++ b/tests/pools/abstract/abstract-pool.test.js @@ -18,10 +18,7 @@ describe('Abstract pool test suite', () => { it('Simulate worker not found during increaseWorkersTask', () => { const pool = new StubPoolWithTasksMapClear( 1, - './tests/worker-files/thread/testWorker.js', - { - errorHandler: e => console.error(e) - } + './tests/worker-files/thread/testWorker.js' ) // Simulate worker not found. pool.removeAllWorker() diff --git a/tests/pools/cluster/fixed.test.js b/tests/pools/cluster/fixed.test.js index 0eb21d07..a5eaa29a 100644 --- a/tests/pools/cluster/fixed.test.js +++ b/tests/pools/cluster/fixed.test.js @@ -2,7 +2,6 @@ const expect = require('expect') const { FixedClusterPool } = require('../../../lib/index') const TestUtils = require('../../test-utils') const numberOfWorkers = 10 -const maxTasks = 500 const pool = new FixedClusterPool( numberOfWorkers, './tests/worker-files/cluster/testWorker.js', @@ -34,10 +33,7 @@ const asyncErrorPool = new FixedClusterPool( ) const asyncPool = new FixedClusterPool( 1, - './tests/worker-files/cluster/asyncWorker.js', - { - maxTasks: maxTasks - } + './tests/worker-files/cluster/asyncWorker.js' ) describe('Fixed cluster pool test suite', () => { @@ -118,11 +114,6 @@ describe('Fixed cluster pool test suite', () => { expect(usedTime).toBeGreaterThanOrEqual(2000) }) - it('Verify that maxTasks is set properly', async () => { - const worker = asyncPool.chooseWorker() - expect(worker.getMaxListeners()).toBe(maxTasks) - }) - it('Shutdown test', async () => { const exitPromise = TestUtils.waitExits(pool, numberOfWorkers) await pool.destroy() diff --git a/tests/pools/selection-strategies.test.js b/tests/pools/selection-strategies.test.js index 0c5e4611..604dda3d 100644 --- a/tests/pools/selection-strategies.test.js +++ b/tests/pools/selection-strategies.test.js @@ -66,10 +66,7 @@ describe('Selection strategies test suite', () => { min, max, './tests/worker-files/thread/testWorker.js', - { - maxTasks: 1000, - workerChoiceStrategy: 'UNKNOWN_STRATEGY' - } + { workerChoiceStrategy: 'UNKNOWN_STRATEGY' } ) ).toThrowError( new Error("Worker choice strategy 'UNKNOWN_STRATEGY' not found") diff --git a/tests/pools/thread/fixed.test.js b/tests/pools/thread/fixed.test.js index 9a2d74d3..cc60a9b5 100644 --- a/tests/pools/thread/fixed.test.js +++ b/tests/pools/thread/fixed.test.js @@ -2,7 +2,6 @@ const expect = require('expect') const { FixedThreadPool } = require('../../../lib/index') const TestUtils = require('../../test-utils') const numberOfThreads = 10 -const maxTasks = 400 const pool = new FixedThreadPool( numberOfThreads, './tests/worker-files/thread/testWorker.js', @@ -28,8 +27,7 @@ const errorPool = new FixedThreadPool( ) const asyncPool = new FixedThreadPool( 1, - './tests/worker-files/thread/asyncWorker.js', - { maxTasks: maxTasks } + './tests/worker-files/thread/asyncWorker.js' ) describe('Fixed thread pool test suite', () => { @@ -96,11 +94,6 @@ describe('Fixed thread pool test suite', () => { expect(usedTime).toBeGreaterThanOrEqual(2000) }) - it('Verify that maxTasks is set properly', async () => { - const worker = asyncPool.chooseWorker() - expect(worker.port2.getMaxListeners()).toBe(maxTasks) - }) - it('Shutdown test', async () => { const exitPromise = TestUtils.waitExits(pool, numberOfThreads) await pool.destroy() -- 2.34.1