From e843b9042b77e0e4e17c193820a3e05ffc92cffe Mon Sep 17 00:00:00 2001 From: =?utf8?q?J=C3=A9r=C3=B4me=20Benoit?= Date: Sun, 7 Mar 2021 22:45:27 +0100 Subject: [PATCH] Fix strategy handling in pool options (#259) MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit And increase coverage a bit. Signed-off-by: Jérôme Benoit --- README.md | 2 +- benchmarks/internal/choose-worker.js | 4 +-- benchmarks/internal/cluster/dynamic.js | 5 ++-- benchmarks/internal/cluster/fixed.js | 5 ++-- benchmarks/internal/thread/dynamic.js | 5 ++-- benchmarks/internal/thread/fixed.js | 5 ++-- src/pools/abstract-pool.ts | 4 ++- tests/pools/abstract/abstract-pool.test.js | 13 ++++++++- tests/pools/selection-strategies.test.js | 33 ++++++++++++++++++++-- 9 files changed, 61 insertions(+), 15 deletions(-) diff --git a/README.md b/README.md index 1ddad503..8f343c39 100644 --- a/README.md +++ b/README.md @@ -205,7 +205,7 @@ We already have a bench folder where you can find some comparisons. Before to jump into each poolifier pool type, let highlight that **Node.js comes with a thread pool already**, the libuv thread pool where some particular tasks already run by default. Please take a look at [which tasks run on the libuv thread pool](https://nodejs.org/en/docs/guides/dont-block-the-event-loop/#what-code-runs-on-the-worker-pool). -Now **if your task runs on libuv thread pool**, you can try to: +**If your task runs on libuv thread pool**, you can try to: - Tune the libuv thread pool size setting the [UV_THREADPOOL_SIZE](https://nodejs.org/api/cli.html#cli_uv_threadpool_size_size) diff --git a/benchmarks/internal/choose-worker.js b/benchmarks/internal/choose-worker.js index e4e727a2..560a395b 100644 --- a/benchmarks/internal/choose-worker.js +++ b/benchmarks/internal/choose-worker.js @@ -9,7 +9,7 @@ function generateWorkersArray (numberOfWorkers) { const workers = generateWorkersArray(60) -let nextWorkerIndex = 0 +let nextWorkerIndex function chooseWorkerTernaryOffByOne () { nextWorkerIndex = @@ -48,7 +48,7 @@ suite nextWorkerIndex = 0 chooseWorkerTernaryWithNegation() }) - .add('Ternary with PreChoosing', function () { + .add('Ternary with pre-choosing', function () { nextWorkerIndex = 0 chooseWorkerTernaryWithPreChoosing() }) diff --git a/benchmarks/internal/cluster/dynamic.js b/benchmarks/internal/cluster/dynamic.js index 6838e07a..3c00c50b 100644 --- a/benchmarks/internal/cluster/dynamic.js +++ b/benchmarks/internal/cluster/dynamic.js @@ -5,6 +5,7 @@ const { const { runPoolifierTest } = require('../benchmark-utils') const size = 30 +const numberOfTasks = 1 const dynamicPool = new DynamicClusterPool( size / 2, @@ -20,13 +21,13 @@ const dynamicPoolLessRecentlyUsed = new DynamicClusterPool( ) async function dynamicClusterTest ( - { tasks, workerData } = { tasks: 1, workerData: { proof: 'ok' } } + { tasks, workerData } = { tasks: numberOfTasks, workerData: { proof: 'ok' } } ) { return runPoolifierTest(dynamicPool, { tasks, workerData }) } async function dynamicClusterTestLessRecentlyUsed ( - { tasks, workerData } = { tasks: 1, workerData: { proof: 'ok' } } + { tasks, workerData } = { tasks: numberOfTasks, workerData: { proof: 'ok' } } ) { return runPoolifierTest(dynamicPoolLessRecentlyUsed, { tasks, workerData }) } diff --git a/benchmarks/internal/cluster/fixed.js b/benchmarks/internal/cluster/fixed.js index ea61c7fc..fc4cc3ac 100644 --- a/benchmarks/internal/cluster/fixed.js +++ b/benchmarks/internal/cluster/fixed.js @@ -5,6 +5,7 @@ const { const { runPoolifierTest } = require('../benchmark-utils') const size = 30 +const numberOfTasks = 1 const fixedPool = new FixedClusterPool( size, @@ -18,13 +19,13 @@ const fixedPoolLessRecentlyUsed = new FixedClusterPool( ) async function fixedClusterTest ( - { tasks, workerData } = { tasks: 1, workerData: { proof: 'ok' } } + { tasks, workerData } = { tasks: numberOfTasks, workerData: { proof: 'ok' } } ) { return runPoolifierTest(fixedPool, { tasks, workerData }) } async function fixedClusterTestLessRecentlyUsed ( - { tasks, workerData } = { tasks: 1, workerData: { proof: 'ok' } } + { tasks, workerData } = { tasks: numberOfTasks, workerData: { proof: 'ok' } } ) { return runPoolifierTest(fixedPoolLessRecentlyUsed, { tasks, workerData }) } diff --git a/benchmarks/internal/thread/dynamic.js b/benchmarks/internal/thread/dynamic.js index fb74d898..29082077 100644 --- a/benchmarks/internal/thread/dynamic.js +++ b/benchmarks/internal/thread/dynamic.js @@ -5,6 +5,7 @@ const { const { runPoolifierTest } = require('../benchmark-utils') const size = 30 +const numberOfTasks = 1 const dynamicPool = new DynamicThreadPool( size / 2, @@ -20,13 +21,13 @@ const dynamicPoolLessRecentlyUsed = new DynamicThreadPool( ) async function dynamicThreadTest ( - { tasks, workerData } = { tasks: 1, workerData: { proof: 'ok' } } + { tasks, workerData } = { tasks: numberOfTasks, workerData: { proof: 'ok' } } ) { return runPoolifierTest(dynamicPool, { tasks, workerData }) } async function dynamicThreadTestLessRecentlyUsed ( - { tasks, workerData } = { tasks: 1, workerData: { proof: 'ok' } } + { tasks, workerData } = { tasks: numberOfTasks, workerData: { proof: 'ok' } } ) { return runPoolifierTest(dynamicPoolLessRecentlyUsed, { tasks, workerData }) } diff --git a/benchmarks/internal/thread/fixed.js b/benchmarks/internal/thread/fixed.js index cd7fdebf..617aa7e5 100644 --- a/benchmarks/internal/thread/fixed.js +++ b/benchmarks/internal/thread/fixed.js @@ -5,6 +5,7 @@ const { const { runPoolifierTest } = require('../benchmark-utils') const size = 30 +const numberOfTasks = 1 const fixedPool = new FixedThreadPool( size, @@ -18,13 +19,13 @@ const fixedPoolLessRecentlyUsed = new FixedThreadPool( ) async function fixedThreadTest ( - { tasks, workerData } = { tasks: 1, workerData: { proof: 'ok' } } + { tasks, workerData } = { tasks: numberOfTasks, workerData: { proof: 'ok' } } ) { return runPoolifierTest(fixedPool, { tasks, workerData }) } async function fixedThreadTestLessRecentlyUsed ( - { tasks, workerData } = { tasks: 1, workerData: { proof: 'ok' } } + { tasks, workerData } = { tasks: numberOfTasks, workerData: { proof: 'ok' } } ) { return runPoolifierTest(fixedPoolLessRecentlyUsed, { tasks, workerData }) } diff --git a/src/pools/abstract-pool.ts b/src/pools/abstract-pool.ts index ff7413dd..54447c82 100644 --- a/src/pools/abstract-pool.ts +++ b/src/pools/abstract-pool.ts @@ -191,7 +191,7 @@ export abstract class AbstractPool< }) return workerCreated }, - opts.workerChoiceStrategy ?? WorkerChoiceStrategies.ROUND_ROBIN + this.opts.workerChoiceStrategy ) } @@ -220,6 +220,8 @@ export abstract class AbstractPool< } private checkPoolOptions (opts: PoolOptions): void { + this.opts.workerChoiceStrategy = + opts.workerChoiceStrategy ?? WorkerChoiceStrategies.ROUND_ROBIN this.opts.enableEvents = opts.enableEvents ?? true } diff --git a/tests/pools/abstract/abstract-pool.test.js b/tests/pools/abstract/abstract-pool.test.js index 4eccde68..a2170491 100644 --- a/tests/pools/abstract/abstract-pool.test.js +++ b/tests/pools/abstract/abstract-pool.test.js @@ -1,5 +1,9 @@ const expect = require('expect') -const { FixedClusterPool, FixedThreadPool } = require('../../../lib/index') +const { + FixedClusterPool, + FixedThreadPool, + WorkerChoiceStrategies +} = require('../../../lib/index') const expectedError = new Error('Worker could not be found in tasks map') const numberOfWorkers = 1 @@ -102,16 +106,23 @@ describe('Abstract pool test suite', () => { ) expect(pool.opts.enableEvents).toEqual(true) expect(pool.emitter).toBeDefined() + expect(pool.opts.workerChoiceStrategy).toBe( + WorkerChoiceStrategies.ROUND_ROBIN + ) pool.destroy() pool = new FixedThreadPool( numberOfWorkers, './tests/worker-files/thread/testWorker.js', { + workerChoiceStrategy: WorkerChoiceStrategies.LESS_RECENTLY_USED, enableEvents: false } ) expect(pool.opts.enableEvents).toEqual(false) expect(pool.emitter).toBeUndefined() + expect(pool.opts.workerChoiceStrategy).toBe( + WorkerChoiceStrategies.LESS_RECENTLY_USED + ) pool.destroy() }) diff --git a/tests/pools/selection-strategies.test.js b/tests/pools/selection-strategies.test.js index d631afc6..f57eaed2 100644 --- a/tests/pools/selection-strategies.test.js +++ b/tests/pools/selection-strategies.test.js @@ -11,6 +11,37 @@ describe('Selection strategies test suite', () => { expect(WorkerChoiceStrategies.LESS_RECENTLY_USED).toBe('LESS_RECENTLY_USED') }) + it('Verify ROUND_ROBIN strategy is the default at pool creation', async () => { + const min = 0 + const max = 3 + const pool = new DynamicThreadPool( + min, + max, + './tests/worker-files/thread/testWorker.js' + ) + expect(pool.opts.workerChoiceStrategy).toBe( + WorkerChoiceStrategies.ROUND_ROBIN + ) + // We need to clean up the resources after our test + await pool.destroy() + }) + + it('Verify ROUND_ROBIN strategy can be set after pool creation', async () => { + const min = 0 + const max = 3 + const pool = new DynamicThreadPool( + min, + max, + './tests/worker-files/thread/testWorker.js' + ) + pool.setWorkerChoiceStrategy(WorkerChoiceStrategies.ROUND_ROBIN) + expect(pool.opts.workerChoiceStrategy).toBe( + WorkerChoiceStrategies.ROUND_ROBIN + ) + // We need to clean up the resources after our test + await pool.destroy() + }) + it('Verify LESS_RECENTLY_USED strategy is taken at pool creation', async () => { const max = 3 const pool = new FixedThreadPool( @@ -52,7 +83,6 @@ describe('Selection strategies test suite', () => { promises.push(pool.execute({ test: 'test' })) } await Promise.all(promises) - // We need to clean up the resources after our test await pool.destroy() }) @@ -72,7 +102,6 @@ describe('Selection strategies test suite', () => { promises.push(pool.execute({ test: 'test' })) } await Promise.all(promises) - // We need to clean up the resources after our test await pool.destroy() }) -- 2.34.1