From bdacc2d25f190728221f7fb8c8cd2aba175cb18d Mon Sep 17 00:00:00 2001 From: =?utf8?q?J=C3=A9r=C3=B4me=20Benoit?= Date: Fri, 7 Oct 2022 21:16:17 +0200 Subject: [PATCH] Report some code cleanups from work in progress PR MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit Signed-off-by: Jérôme Benoit --- CHANGELOG.md | 4 ++ benchmarks/internal/benchmark-utils.js | 28 ++++++++++++- benchmarks/internal/cluster/worker.js | 8 +--- benchmarks/internal/thread/worker.js | 8 +--- src/pools/abstract-pool.ts | 21 +++++----- tests/pools/cluster/dynamic.test.js | 4 +- tests/pools/cluster/fixed.test.js | 4 +- tests/pools/thread/dynamic.test.js | 4 +- tests/pools/thread/fixed.test.js | 4 +- tests/test-utils.js | 40 +++++++++++++++++++ tests/worker-files/cluster/asyncWorker.js | 5 +-- .../cluster/longRunningWorkerHardBehavior.js | 5 +-- .../cluster/longRunningWorkerSoftBehavior.js | 5 +-- tests/worker-files/cluster/testWorker.js | 8 +--- tests/worker-files/thread/asyncWorker.js | 5 +-- .../thread/longRunningWorkerHardBehavior.js | 5 +-- 16 files changed, 105 insertions(+), 53 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ee4d6fd9..cbaf1909 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,10 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## [2.2.1] - 2022-05-01 + +- + ## [2.2.0] - 2022-05-01 ### Breaking Changes diff --git a/benchmarks/internal/benchmark-utils.js b/benchmarks/internal/benchmark-utils.js index a915a044..5f0c643b 100644 --- a/benchmarks/internal/benchmark-utils.js +++ b/benchmarks/internal/benchmark-utils.js @@ -16,6 +16,15 @@ async function runPoolifierTest (pool, { tasks, workerData }) { }) } +function jsonIntegerSerialization (n) { + for (let i = 0; i < n; i++) { + const o = { + a: i + } + JSON.stringify(o) + } +} + function generateRandomInteger (max, min = 0) { max = Math.floor(max) if (min) { @@ -25,9 +34,26 @@ function generateRandomInteger (max, min = 0) { return Math.floor(Math.random() * (max + 1)) } +/** + * Intentionally inefficient implementation. + * + * @param {*} n + * @returns {number} + */ +function fibonacci (n) { + if (n <= 1) return 1 + return fibonacci(n - 1) + fibonacci(n - 2) +} + const LIST_FORMATTER = new Intl.ListFormat('en-US', { style: 'long', type: 'conjunction' }) -module.exports = { generateRandomInteger, LIST_FORMATTER, runPoolifierTest } +module.exports = { + runPoolifierTest, + jsonIntegerSerialization, + generateRandomInteger, + fibonacci, + LIST_FORMATTER +} diff --git a/benchmarks/internal/cluster/worker.js b/benchmarks/internal/cluster/worker.js index b4d54fca..9817322c 100644 --- a/benchmarks/internal/cluster/worker.js +++ b/benchmarks/internal/cluster/worker.js @@ -1,13 +1,9 @@ 'use strict' const { ClusterWorker } = require('../../../lib/index') +const { jsonIntegerSerialization } = require('../benchmark-utils') function yourFunction (data) { - for (let i = 0; i < 1000; i++) { - const o = { - a: i - } - JSON.stringify(o) - } + jsonIntegerSerialization(1000) // console.log('This is the main thread ' + isMaster) return { ok: 1 } } diff --git a/benchmarks/internal/thread/worker.js b/benchmarks/internal/thread/worker.js index f5f36ed8..6437c0af 100644 --- a/benchmarks/internal/thread/worker.js +++ b/benchmarks/internal/thread/worker.js @@ -1,13 +1,9 @@ 'use strict' const { ThreadWorker } = require('../../../lib/index') +const { jsonIntegerSerialization } = require('../benchmark-utils') function yourFunction (data) { - for (let i = 0; i < 1000; i++) { - const o = { - a: i - } - JSON.stringify(o) - } + jsonIntegerSerialization(1000) // console.log('This is the main thread ' + isMainThread) return { ok: 1 } } diff --git a/src/pools/abstract-pool.ts b/src/pools/abstract-pool.ts index 5efecb58..484b6916 100644 --- a/src/pools/abstract-pool.ts +++ b/src/pools/abstract-pool.ts @@ -190,10 +190,9 @@ export abstract class AbstractPool< () => { const workerCreated = this.createAndSetupWorker() this.registerWorkerMessageListener(workerCreated, message => { - const tasksInProgress = this.tasks.get(workerCreated) if ( isKillBehavior(KillBehaviors.HARD, message.kill) || - tasksInProgress === 0 + this.tasks.get(workerCreated) === 0 ) { // Kill received from the worker, means that no new tasks are submitted to that worker for a while ( > maxInactiveTime) this.destroyWorker(workerCreated) as void @@ -281,7 +280,7 @@ export abstract class AbstractPool< const messageId = ++this.nextMessageId const res = this.internalExecute(worker, messageId) this.checkAndEmitBusy() - this.sendToWorker(worker, { data: data || ({} as Data), id: messageId }) + this.sendToWorker(worker, { data: data ?? ({} as Data), id: messageId }) return res } @@ -417,7 +416,7 @@ export abstract class AbstractPool< * @returns New, completely set up worker. */ protected createAndSetupWorker (): Worker { - const worker: Worker = this.createWorker() + const worker = this.createWorker() worker.on('message', this.opts.messageHandler ?? EMPTY_FUNCTION) worker.on('error', this.opts.errorHandler ?? EMPTY_FUNCTION) @@ -438,16 +437,16 @@ export abstract class AbstractPool< /** * This function is the listener registered for each worker. * - * @returns The listener function to execute when a message is sent from a worker. + * @returns The listener function to execute when a message is received from a worker. */ protected workerListener (): (message: MessageValue) => void { return message => { - if (message.id) { - const value = this.promiseMap.get(message.id) - if (value) { - this.decreaseWorkersTasks(value.worker) - if (message.error) value.reject(message.error) - else value.resolve(message.data as Response) + if (message.id !== undefined) { + const promise = this.promiseMap.get(message.id) + if (promise !== undefined) { + this.decreaseWorkersTasks(promise.worker) + if (message.error) promise.reject(message.error) + else promise.resolve(message.data as Response) this.promiseMap.delete(message.id) } } diff --git a/tests/pools/cluster/dynamic.test.js b/tests/pools/cluster/dynamic.test.js index b1201ea5..61e1d6e2 100644 --- a/tests/pools/cluster/dynamic.test.js +++ b/tests/pools/cluster/dynamic.test.js @@ -54,8 +54,8 @@ describe('Dynamic cluster pool test suite', () => { it('Shutdown test', async () => { const exitPromise = TestUtils.waitExits(pool, min) await pool.destroy() - const res = await exitPromise - expect(res).toBe(min) + const numberOfExitEvents = await exitPromise + expect(numberOfExitEvents).toBe(min) }) it('Validation of inputs test', () => { diff --git a/tests/pools/cluster/fixed.test.js b/tests/pools/cluster/fixed.test.js index 2a0a8e2e..2ef709c1 100644 --- a/tests/pools/cluster/fixed.test.js +++ b/tests/pools/cluster/fixed.test.js @@ -130,8 +130,8 @@ describe('Fixed cluster pool test suite', () => { it('Shutdown test', async () => { const exitPromise = TestUtils.waitExits(pool, numberOfWorkers) await pool.destroy() - const res = await exitPromise - expect(res).toBe(numberOfWorkers) + const numberOfExitEvents = await exitPromise + expect(numberOfExitEvents).toBe(numberOfWorkers) }) it('Should work even without opts in input', async () => { diff --git a/tests/pools/thread/dynamic.test.js b/tests/pools/thread/dynamic.test.js index ae78649f..3ae35ac9 100644 --- a/tests/pools/thread/dynamic.test.js +++ b/tests/pools/thread/dynamic.test.js @@ -30,8 +30,8 @@ describe('Dynamic thread pool test suite', () => { // The `busy` event is triggered when the number of submitted tasks at once reach the max number of workers in the dynamic pool. // So in total numberOfWorkers + 1 times for a loop submitting up to numberOfWorkers * 2 tasks to the dynamic pool. expect(poolBusy).toBe(max + 1) - const res = await TestUtils.waitExits(pool, max - min) - expect(res).toBe(max - min) + const numberOfExitEvents = await TestUtils.waitExits(pool, max - min) + expect(numberOfExitEvents).toBe(max - min) }) it('Verify scale thread up and down is working', async () => { diff --git a/tests/pools/thread/fixed.test.js b/tests/pools/thread/fixed.test.js index d0a0e367..763e3f88 100644 --- a/tests/pools/thread/fixed.test.js +++ b/tests/pools/thread/fixed.test.js @@ -132,8 +132,8 @@ describe('Fixed thread pool test suite', () => { it('Shutdown test', async () => { const exitPromise = TestUtils.waitExits(pool, numberOfThreads) await pool.destroy() - const res = await exitPromise - expect(res).toBe(numberOfThreads) + const numberOfExitEvents = await exitPromise + expect(numberOfExitEvents).toBe(numberOfThreads) }) it('Should work even without opts in input', async () => { diff --git a/tests/test-utils.js b/tests/test-utils.js index 690f4b37..4668e072 100644 --- a/tests/test-utils.js +++ b/tests/test-utils.js @@ -16,6 +16,46 @@ class TestUtils { static async sleep (ms) { return new Promise(resolve => setTimeout(resolve, ms)) } + + static async workerSleepFunction (data, ms) { + return new Promise((resolve, reject) => { + setTimeout(() => resolve(data), ms) + }) + } + + static jsonIntegerSerialization (n) { + for (let i = 0; i < n; i++) { + const o = { + a: i + } + JSON.stringify(o) + } + } + + /** + * Intentionally inefficient implementation. + * + * @param {*} n + * @returns {number} + */ + static fibonacci (n) { + if (n <= 1) return 1 + return TestUtils.fibonacci(n - 1) + TestUtils.fibonacci(n - 2) + } + + /** + * Intentionally inefficient implementation. + * + * @param {*} n + * @returns {number} + */ + static factorial (n) { + if (n === 0) { + return 1 + } else { + return TestUtils.factorial(n - 1) * n + } + } } module.exports = TestUtils diff --git a/tests/worker-files/cluster/asyncWorker.js b/tests/worker-files/cluster/asyncWorker.js index b5c784df..bceaffae 100644 --- a/tests/worker-files/cluster/asyncWorker.js +++ b/tests/worker-files/cluster/asyncWorker.js @@ -1,10 +1,9 @@ 'use strict' const { ClusterWorker, KillBehaviors } = require('../../../lib/index') +const TestUtils = require('../../test-utils') async function sleep (data) { - return new Promise((resolve, reject) => { - setTimeout(() => resolve(data), 2000) - }) + return TestUtils.workerSleepFunction(data, 2000) } module.exports = new ClusterWorker(sleep, { diff --git a/tests/worker-files/cluster/longRunningWorkerHardBehavior.js b/tests/worker-files/cluster/longRunningWorkerHardBehavior.js index 04c78f46..73fdad01 100644 --- a/tests/worker-files/cluster/longRunningWorkerHardBehavior.js +++ b/tests/worker-files/cluster/longRunningWorkerHardBehavior.js @@ -1,10 +1,9 @@ 'use strict' const { ClusterWorker, KillBehaviors } = require('../../../lib/index') +const TestUtils = require('../../test-utils') async function sleep (data) { - return new Promise((resolve, reject) => { - setTimeout(() => resolve(data), 50000) - }) + return TestUtils.workerSleepFunction(data, 50000) } module.exports = new ClusterWorker(sleep, { diff --git a/tests/worker-files/cluster/longRunningWorkerSoftBehavior.js b/tests/worker-files/cluster/longRunningWorkerSoftBehavior.js index c4c00f6a..5498752f 100644 --- a/tests/worker-files/cluster/longRunningWorkerSoftBehavior.js +++ b/tests/worker-files/cluster/longRunningWorkerSoftBehavior.js @@ -1,10 +1,9 @@ 'use strict' const { ClusterWorker } = require('../../../lib/index') +const TestUtils = require('../../test-utils') async function sleep (data) { - return new Promise((resolve, reject) => { - setTimeout(() => resolve(data), 50000) - }) + return TestUtils.workerSleepFunction(data, 50000) } module.exports = new ClusterWorker(sleep, { diff --git a/tests/worker-files/cluster/testWorker.js b/tests/worker-files/cluster/testWorker.js index 54938607..f6115cde 100644 --- a/tests/worker-files/cluster/testWorker.js +++ b/tests/worker-files/cluster/testWorker.js @@ -1,14 +1,10 @@ 'use strict' const { ClusterWorker, KillBehaviors } = require('../../../lib/index') const { isMaster } = require('cluster') +const TestUtils = require('../../test-utils') function test (data) { - for (let i = 0; i < 50; i++) { - const o = { - a: i - } - JSON.stringify(o) - } + TestUtils.jsonIntegerSerialization(50) return isMaster } diff --git a/tests/worker-files/thread/asyncWorker.js b/tests/worker-files/thread/asyncWorker.js index 0bf5d244..6508d6da 100644 --- a/tests/worker-files/thread/asyncWorker.js +++ b/tests/worker-files/thread/asyncWorker.js @@ -1,10 +1,9 @@ 'use strict' const { ThreadWorker, KillBehaviors } = require('../../../lib/index') +const TestUtils = require('../../test-utils') async function sleep (data) { - return new Promise((resolve, reject) => { - setTimeout(() => resolve(data), 2000) - }) + return TestUtils.workerSleepFunction(data, 2000) } module.exports = new ThreadWorker(sleep, { diff --git a/tests/worker-files/thread/longRunningWorkerHardBehavior.js b/tests/worker-files/thread/longRunningWorkerHardBehavior.js index 7d9714a8..3c707eb5 100644 --- a/tests/worker-files/thread/longRunningWorkerHardBehavior.js +++ b/tests/worker-files/thread/longRunningWorkerHardBehavior.js @@ -1,10 +1,9 @@ 'use strict' const { ThreadWorker, KillBehaviors } = require('../../../lib/index') +const TestUtils = require('../../test-utils') async function sleep (data) { - return new Promise((resolve, reject) => { - setTimeout(() => resolve(data), 50000) - }) + return TestUtils.workerSleepFunction(data, 50000) } module.exports = new ThreadWorker(sleep, { -- 2.34.1