From 11df35903da6f581c45e8b42e1d4fbd342bddc3c Mon Sep 17 00:00:00 2001 From: =?utf8?q?J=C3=A9r=C3=B4me=20Benoit?= Date: Fri, 14 Oct 2022 23:17:43 +0200 Subject: [PATCH] Fixes to worker selection strategies MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit Signed-off-by: Jérôme Benoit --- CHANGELOG.md | 10 +++++ package-lock.json | 4 +- package.json | 2 +- sonar-project.properties | 2 +- src/pools/abstract-pool.ts | 6 +-- .../fair-share-worker-choice-strategy.ts | 32 ++++++++-------- ...hted-round-robin-worker-choice-strategy.ts | 38 +++++++------------ .../selection-strategies.test.js | 20 ---------- ...round-robin-worker-choice-strategy.test.js | 2 - 9 files changed, 47 insertions(+), 69 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2f81e0fe..ed579f97 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,16 @@ 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.3.2] - 2022-14-10 + +### Changed + +- Optimize fair share worker selection strategy implementation. + +### Fixed + +- Fix WRR worker selection strategy: ensure the condition triggering the round robin can be fulfilled. + ## [2.3.1] - 2022-13-10 ### Added diff --git a/package-lock.json b/package-lock.json index cdfb3476..b297e2c0 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "poolifier", - "version": "2.3.1", + "version": "2.3.2", "lockfileVersion": 2, "requires": true, "packages": { "": { "name": "poolifier", - "version": "2.3.1", + "version": "2.3.2", "license": "MIT", "devDependencies": { "@types/node": "^18.8.5", diff --git a/package.json b/package.json index 298948de..85007e99 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "poolifier", - "version": "2.3.1", + "version": "2.3.2", "description": "A fast, easy to use Node.js Worker Thread Pool and Cluster Pool implementation", "main": "lib/index.js", "scripts": { diff --git a/sonar-project.properties b/sonar-project.properties index fb053f5e..893d9894 100644 --- a/sonar-project.properties +++ b/sonar-project.properties @@ -3,7 +3,7 @@ sonar.organization=pioardi sonar.javascript.lcov.reportPaths=coverage/lcov.info sonar.eslint.reportPaths=reports/eslint.json sonar.projectName=poolifier -sonar.projectVersion=2.3.1 +sonar.projectVersion=2.3.2 sonar.host.url=https://sonarcloud.io sonar.sources=src sonar.tests=tests diff --git a/src/pools/abstract-pool.ts b/src/pools/abstract-pool.ts index c4181e76..40e06b15 100644 --- a/src/pools/abstract-pool.ts +++ b/src/pools/abstract-pool.ts @@ -272,7 +272,7 @@ export abstract class AbstractPool< /** * Removes the given worker from the pool. * - * @param worker Worker that will be removed. + * @param worker The worker that will be removed. */ protected removeWorker (worker: Worker): void { // Clean worker from data structure @@ -305,8 +305,8 @@ export abstract class AbstractPool< /** * Registers a listener callback on a given worker. * - * @param worker A worker. - * @param listener A message listener callback. + * @param worker The worker which should register a listener. + * @param listener The message listener callback. */ protected abstract registerWorkerMessageListener< Message extends Data | Response diff --git a/src/pools/selection-strategies/fair-share-worker-choice-strategy.ts b/src/pools/selection-strategies/fair-share-worker-choice-strategy.ts index 3735f972..48f213ff 100644 --- a/src/pools/selection-strategies/fair-share-worker-choice-strategy.ts +++ b/src/pools/selection-strategies/fair-share-worker-choice-strategy.ts @@ -44,10 +44,10 @@ export class FairShareWorkerChoiceStrategy< /** @inheritDoc */ public choose (): Worker { - this.computeWorkerLastVirtualTaskTimestamp() let minWorkerVirtualTaskEndTimestamp = Infinity let chosenWorker!: Worker for (const worker of this.pool.workers) { + this.computeWorkerLastVirtualTaskTimestamp(worker) const workerLastVirtualTaskEndTimestamp = this.workerLastVirtualTaskTimestamp.get(worker)?.end ?? 0 if ( @@ -61,21 +61,21 @@ export class FairShareWorkerChoiceStrategy< } /** - * Computes workers last virtual task timestamp. + * Computes worker last virtual task timestamp. + * + * @param worker The worker. */ - private computeWorkerLastVirtualTaskTimestamp () { - for (const worker of this.pool.workers) { - const workerVirtualTaskStartTimestamp = Math.max( - Date.now(), - this.workerLastVirtualTaskTimestamp.get(worker)?.end ?? -Infinity - ) - const workerVirtualTaskEndTimestamp = - workerVirtualTaskStartTimestamp + - (this.pool.getWorkerAverageTasksRunTime(worker) ?? 0) - this.workerLastVirtualTaskTimestamp.set(worker, { - start: workerVirtualTaskStartTimestamp, - end: workerVirtualTaskEndTimestamp - }) - } + private computeWorkerLastVirtualTaskTimestamp (worker: Worker): void { + const workerVirtualTaskStartTimestamp = Math.max( + Date.now(), + this.workerLastVirtualTaskTimestamp.get(worker)?.end ?? -Infinity + ) + const workerVirtualTaskEndTimestamp = + workerVirtualTaskStartTimestamp + + (this.pool.getWorkerAverageTasksRunTime(worker) ?? 0) + this.workerLastVirtualTaskTimestamp.set(worker, { + start: workerVirtualTaskStartTimestamp, + end: workerVirtualTaskEndTimestamp + }) } } diff --git a/src/pools/selection-strategies/weighted-round-robin-worker-choice-strategy.ts b/src/pools/selection-strategies/weighted-round-robin-worker-choice-strategy.ts index c2f85145..2b9cfc74 100644 --- a/src/pools/selection-strategies/weighted-round-robin-worker-choice-strategy.ts +++ b/src/pools/selection-strategies/weighted-round-robin-worker-choice-strategy.ts @@ -30,10 +30,6 @@ export class WeightedRoundRobinWorkerChoiceStrategy< runTime: true } - /** - * Worker index where the previous task was submitted. - */ - private previousWorkerIndex: number = 0 /** * Worker index where the current task will be submitted. */ @@ -63,7 +59,6 @@ export class WeightedRoundRobinWorkerChoiceStrategy< /** @inheritDoc */ public reset (): boolean { - this.previousWorkerIndex = 0 this.currentWorkerIndex = 0 this.workersTaskRunTime.clear() this.initWorkersTaskRunTime() @@ -72,40 +67,35 @@ export class WeightedRoundRobinWorkerChoiceStrategy< /** @inheritDoc */ public choose (): Worker { - const currentWorker = this.pool.workers[this.currentWorkerIndex] + let chosenWorker = this.pool.workers[this.currentWorkerIndex] if ( this.isDynamicPool === true && - this.workersTaskRunTime.has(currentWorker) === false + this.workersTaskRunTime.has(chosenWorker) === false ) { - this.initWorkerTaskRunTime(currentWorker) + this.initWorkerTaskRunTime(chosenWorker) } - const workerVirtualTaskRunTime = - this.getWorkerVirtualTaskRunTime(currentWorker) ?? 0 const workerTaskWeight = - this.workersTaskRunTime.get(currentWorker)?.weight ?? + this.workersTaskRunTime.get(chosenWorker)?.weight ?? this.defaultWorkerWeight - if (this.currentWorkerIndex === this.previousWorkerIndex) { - const workerTaskRunTime = - (this.workersTaskRunTime.get(currentWorker)?.runTime ?? 0) + - workerVirtualTaskRunTime + if ( + (this.workersTaskRunTime.get(chosenWorker)?.runTime ?? 0) < + workerTaskWeight + ) { this.setWorkerTaskRunTime( - currentWorker, + chosenWorker, workerTaskWeight, - workerTaskRunTime + (this.workersTaskRunTime.get(chosenWorker)?.runTime ?? 0) + + (this.getWorkerVirtualTaskRunTime(chosenWorker) ?? 0) ) } else { - this.setWorkerTaskRunTime(currentWorker, workerTaskWeight, 0) - } - if (workerVirtualTaskRunTime < workerTaskWeight) { - this.previousWorkerIndex = this.currentWorkerIndex - } else { - this.previousWorkerIndex = this.currentWorkerIndex this.currentWorkerIndex = this.pool.workers.length - 1 === this.currentWorkerIndex ? 0 : this.currentWorkerIndex + 1 + chosenWorker = this.pool.workers[this.currentWorkerIndex] + this.setWorkerTaskRunTime(chosenWorker, workerTaskWeight, 0) } - return this.pool.workers[this.currentWorkerIndex] + return chosenWorker } private initWorkersTaskRunTime (): void { diff --git a/tests/pools/selection-strategies/selection-strategies.test.js b/tests/pools/selection-strategies/selection-strategies.test.js index 64b26cd5..b8341f09 100644 --- a/tests/pools/selection-strategies/selection-strategies.test.js +++ b/tests/pools/selection-strategies/selection-strategies.test.js @@ -402,10 +402,6 @@ describe('Selection strategies test suite', () => { expect(pool.opts.workerChoiceStrategy).toBe( WorkerChoiceStrategies.WEIGHTED_ROUND_ROBIN ) - expect( - pool.workerChoiceStrategyContext.getWorkerChoiceStrategy() - .previousWorkerIndex - ).toBe(0) expect( pool.workerChoiceStrategyContext.getWorkerChoiceStrategy() .currentWorkerIndex @@ -508,10 +504,6 @@ describe('Selection strategies test suite', () => { max, './tests/worker-files/thread/testWorker.js' ) - expect( - pool.workerChoiceStrategyContext.getWorkerChoiceStrategy() - .previousWorkerIndex - ).toBeUndefined() expect( pool.workerChoiceStrategyContext.getWorkerChoiceStrategy() .currentWorkerIndex @@ -525,10 +517,6 @@ describe('Selection strategies test suite', () => { .workersTaskRunTime ).toBeUndefined() pool.setWorkerChoiceStrategy(WorkerChoiceStrategies.WEIGHTED_ROUND_ROBIN) - expect( - pool.workerChoiceStrategyContext.getWorkerChoiceStrategy() - .previousWorkerIndex - ).toBe(0) expect( pool.workerChoiceStrategyContext.getWorkerChoiceStrategy() .currentWorkerIndex @@ -552,10 +540,6 @@ describe('Selection strategies test suite', () => { max, './tests/worker-files/thread/testWorker.js' ) - expect( - pool.workerChoiceStrategyContext.getWorkerChoiceStrategy() - .workerChoiceStrategy.previousWorkerIndex - ).toBeUndefined() expect( pool.workerChoiceStrategyContext.getWorkerChoiceStrategy() .workerChoiceStrategy.currentWorkerIndex @@ -569,10 +553,6 @@ describe('Selection strategies test suite', () => { .workerChoiceStrategy.workersTaskRunTime ).toBeUndefined() pool.setWorkerChoiceStrategy(WorkerChoiceStrategies.WEIGHTED_ROUND_ROBIN) - expect( - pool.workerChoiceStrategyContext.getWorkerChoiceStrategy() - .workerChoiceStrategy.previousWorkerIndex - ).toBe(0) expect( pool.workerChoiceStrategyContext.getWorkerChoiceStrategy() .workerChoiceStrategy.currentWorkerIndex diff --git a/tests/pools/selection-strategies/weighted-round-robin-worker-choice-strategy.test.js b/tests/pools/selection-strategies/weighted-round-robin-worker-choice-strategy.test.js index 9d87c699..d8ae8299 100644 --- a/tests/pools/selection-strategies/weighted-round-robin-worker-choice-strategy.test.js +++ b/tests/pools/selection-strategies/weighted-round-robin-worker-choice-strategy.test.js @@ -25,7 +25,6 @@ describe('Weighted round robin strategy worker choice strategy test suite', () = it('Verify that reset() resets internals', () => { const strategy = new WeightedRoundRobinWorkerChoiceStrategy(pool) - strategy.previousWorkerIndex = TestUtils.generateRandomInteger() strategy.currentWorkerIndex = TestUtils.generateRandomInteger() const workersTaskRunTimeClearStub = sinon .stub(strategy.workersTaskRunTime, 'clear') @@ -35,7 +34,6 @@ describe('Weighted round robin strategy worker choice strategy test suite', () = .returns() const resetResult = strategy.reset() expect(resetResult).toBe(true) - expect(strategy.previousWorkerIndex).toBe(0) expect(strategy.currentWorkerIndex).toBe(0) expect(workersTaskRunTimeClearStub.calledOnce).toBe(true) expect(initWorkersTaskRunTimeStub.calledOnce).toBe(true) -- 2.34.1