From 60da3e651a8bb2fea1327c5ba5045ae67d81418f Mon Sep 17 00:00:00 2001 From: =?utf8?q?J=C3=A9r=C3=B4me=20Benoit?= Date: Tue, 19 Aug 2025 19:06:15 +0200 Subject: [PATCH] refactor: cleanup worker selection strategies code MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit Signed-off-by: Jérôme Benoit --- src/circular-buffer.ts | 10 +++++----- .../abstract-worker-choice-strategy.ts | 10 ++++++---- .../fair-share-worker-choice-strategy.ts | 3 +++ ...ved-weighted-round-robin-worker-choice-strategy.ts | 2 +- .../least-busy-worker-choice-strategy.ts | 2 +- .../least-elu-worker-choice-strategy.ts | 2 +- .../weighted-round-robin-worker-choice-strategy.ts | 2 +- .../worker-choice-strategies-context.ts | 2 +- src/queues/abstract-fixed-queue.ts | 2 +- src/queues/priority-queue.ts | 4 ++-- src/utils.ts | 11 +++++------ tests/circular-buffer.test.mjs | 2 +- .../worker-choice-strategies-context.test.mjs | 4 ++-- tests/queues/priority-queue.test.mjs | 2 +- tests/utils.test.mjs | 8 ++++---- 15 files changed, 35 insertions(+), 31 deletions(-) diff --git a/src/circular-buffer.ts b/src/circular-buffer.ts index 8bd510cdf..4de6ce29f 100644 --- a/src/circular-buffer.ts +++ b/src/circular-buffer.ts @@ -76,13 +76,13 @@ export class CircularBuffer { * @returns Numbers' array. */ public toArray (): number[] { - const array: number[] = [] if (this.empty()) { - return array + return [] } + const array: number[] = new Array(this.size) let currentIdx = this.readIdx for (let i = 0; i < this.size; i++) { - array.push(this.items[currentIdx]) + array[i] = this.items[currentIdx] currentIdx = currentIdx === this.maxArrayIdx ? 0 : currentIdx + 1 } return array @@ -98,9 +98,9 @@ export class CircularBuffer { `Invalid circular buffer size: '${size.toString()}' is not an integer` ) } - if (size < 0) { + if (size <= 0) { throw new RangeError( - `Invalid circular buffer size: ${size.toString()} < 0` + `Invalid circular buffer size: ${size.toString()} <= 0` ) } } diff --git a/src/pools/selection-strategies/abstract-worker-choice-strategy.ts b/src/pools/selection-strategies/abstract-worker-choice-strategy.ts index 8814c7121..ed8af7607 100644 --- a/src/pools/selection-strategies/abstract-worker-choice-strategy.ts +++ b/src/pools/selection-strategies/abstract-worker-choice-strategy.ts @@ -37,9 +37,9 @@ export abstract class AbstractWorkerChoiceStrategy< /** @inheritDoc */ public readonly taskStatisticsRequirements: TaskStatisticsRequirements = Object.freeze({ - elu: DEFAULT_MEASUREMENT_STATISTICS_REQUIREMENTS, - runTime: DEFAULT_MEASUREMENT_STATISTICS_REQUIREMENTS, - waitTime: DEFAULT_MEASUREMENT_STATISTICS_REQUIREMENTS, + elu: { ...DEFAULT_MEASUREMENT_STATISTICS_REQUIREMENTS }, + runTime: { ...DEFAULT_MEASUREMENT_STATISTICS_REQUIREMENTS }, + waitTime: { ...DEFAULT_MEASUREMENT_STATISTICS_REQUIREMENTS }, }) /** @@ -163,7 +163,9 @@ export abstract class AbstractWorkerChoiceStrategy< */ protected setPreviousWorkerNodeKey (workerNodeKey: number | undefined): void { this.previousWorkerNodeKey = - workerNodeKey != null && workerNodeKey >= 0 + workerNodeKey != null && + workerNodeKey >= 0 && + workerNodeKey < this.pool.workerNodes.length ? workerNodeKey : this.previousWorkerNodeKey } 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 073ec18a6..a60cc1a65 100644 --- a/src/pools/selection-strategies/fair-share-worker-choice-strategy.ts +++ b/src/pools/selection-strategies/fair-share-worker-choice-strategy.ts @@ -90,6 +90,7 @@ export class FairShareWorkerChoiceStrategy< /** @inheritDoc */ public update (workerNodeKey: number): boolean { this.pool.workerNodes[workerNodeKey].strategyData = { + ...this.pool.workerNodes[workerNodeKey].strategyData, virtualTaskEndTimestamp: this.computeWorkerNodeVirtualTaskEndTimestamp(workerNodeKey), } @@ -118,6 +119,7 @@ export class FairShareWorkerChoiceStrategy< } if (minWorkerNodeKey === -1) { workerNode.strategyData = { + ...workerNode.strategyData, virtualTaskEndTimestamp: this.computeWorkerNodeVirtualTaskEndTimestamp(workerNodeKey), } @@ -125,6 +127,7 @@ export class FairShareWorkerChoiceStrategy< } if (workerNode.strategyData?.virtualTaskEndTimestamp == null) { workerNode.strategyData = { + ...workerNode.strategyData, virtualTaskEndTimestamp: this.computeWorkerNodeVirtualTaskEndTimestamp(workerNodeKey), } diff --git a/src/pools/selection-strategies/interleaved-weighted-round-robin-worker-choice-strategy.ts b/src/pools/selection-strategies/interleaved-weighted-round-robin-worker-choice-strategy.ts index 175653c3a..e8aa8f16c 100644 --- a/src/pools/selection-strategies/interleaved-weighted-round-robin-worker-choice-strategy.ts +++ b/src/pools/selection-strategies/interleaved-weighted-round-robin-worker-choice-strategy.ts @@ -31,7 +31,7 @@ export class InterleavedWeightedRoundRobinWorkerChoiceStrategy< /** @inheritDoc */ public override readonly taskStatisticsRequirements: TaskStatisticsRequirements = Object.freeze({ - elu: DEFAULT_MEASUREMENT_STATISTICS_REQUIREMENTS, + elu: { ...DEFAULT_MEASUREMENT_STATISTICS_REQUIREMENTS }, runTime: { aggregate: true, average: true, diff --git a/src/pools/selection-strategies/least-busy-worker-choice-strategy.ts b/src/pools/selection-strategies/least-busy-worker-choice-strategy.ts index bc22a3dc6..62170653f 100644 --- a/src/pools/selection-strategies/least-busy-worker-choice-strategy.ts +++ b/src/pools/selection-strategies/least-busy-worker-choice-strategy.ts @@ -30,7 +30,7 @@ export class LeastBusyWorkerChoiceStrategy< /** @inheritDoc */ public override readonly taskStatisticsRequirements: TaskStatisticsRequirements = Object.freeze({ - elu: DEFAULT_MEASUREMENT_STATISTICS_REQUIREMENTS, + elu: { ...DEFAULT_MEASUREMENT_STATISTICS_REQUIREMENTS }, runTime: { aggregate: true, average: false, diff --git a/src/pools/selection-strategies/least-elu-worker-choice-strategy.ts b/src/pools/selection-strategies/least-elu-worker-choice-strategy.ts index 011625f0e..cda8535c3 100644 --- a/src/pools/selection-strategies/least-elu-worker-choice-strategy.ts +++ b/src/pools/selection-strategies/least-elu-worker-choice-strategy.ts @@ -35,7 +35,7 @@ export class LeastEluWorkerChoiceStrategy< average: false, median: false, }, - runTime: DEFAULT_MEASUREMENT_STATISTICS_REQUIREMENTS, + runTime: { ...DEFAULT_MEASUREMENT_STATISTICS_REQUIREMENTS }, waitTime: { aggregate: true, average: false, 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 cf88d5106..365fda9df 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 @@ -32,7 +32,7 @@ export class WeightedRoundRobinWorkerChoiceStrategy< /** @inheritDoc */ public override readonly taskStatisticsRequirements: TaskStatisticsRequirements = Object.freeze({ - elu: DEFAULT_MEASUREMENT_STATISTICS_REQUIREMENTS, + elu: { ...DEFAULT_MEASUREMENT_STATISTICS_REQUIREMENTS }, runTime: { aggregate: true, average: true, diff --git a/src/pools/selection-strategies/worker-choice-strategies-context.ts b/src/pools/selection-strategies/worker-choice-strategies-context.ts index 846916837..41d2fef89 100644 --- a/src/pools/selection-strategies/worker-choice-strategies-context.ts +++ b/src/pools/selection-strategies/worker-choice-strategies-context.ts @@ -248,7 +248,7 @@ export class WorkerChoiceStrategiesContext< } if (workerNodeKey == null) { throw new Error( - `Worker node key chosen by ${workerChoiceStrategy.name} is null or undefined after ${retriesCount.toString()} retries` + `Worker node key chosen by ${workerChoiceStrategy.name} is null or undefined after ${retriesCount.toString()} retries (max: ${this.retries.toString()})` ) } return workerNodeKey diff --git a/src/queues/abstract-fixed-queue.ts b/src/queues/abstract-fixed-queue.ts index b22b24860..5d64719b3 100644 --- a/src/queues/abstract-fixed-queue.ts +++ b/src/queues/abstract-fixed-queue.ts @@ -114,7 +114,7 @@ export abstract class AbstractFixedQueue implements IFixedQueue { /** @inheritdoc */ public get (index: number): T | undefined { - if (this.empty() || index >= this.size) { + if (this.empty() || index < 0 || index >= this.size) { return undefined } index += this.start diff --git a/src/queues/priority-queue.ts b/src/queues/priority-queue.ts index 48be511c7..e90b4c330 100644 --- a/src/queues/priority-queue.ts +++ b/src/queues/priority-queue.ts @@ -70,8 +70,8 @@ export class PriorityQueue { `Invalid bucket size: '${bucketSize.toString()}' is not an integer` ) } - if (bucketSize < 0) { - throw new RangeError(`Invalid bucket size: ${bucketSize.toString()} < 0`) + if (bucketSize <= 0) { + throw new RangeError(`Invalid bucket size: ${bucketSize.toString()} <= 0`) } this.bucketSize = bucketSize this.priorityEnabled = enablePriority diff --git a/src/utils.ts b/src/utils.ts index c7946d8c5..a535d5d69 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -70,10 +70,10 @@ export const exponentialDelay = ( * @internal */ export const average = (dataSet: number[]): number => { - if (Array.isArray(dataSet) && dataSet.length === 0) { + if (!Array.isArray(dataSet) || dataSet.length === 0) { return 0 } - if (Array.isArray(dataSet) && dataSet.length === 1) { + if (dataSet.length === 1) { return dataSet[0] } return ( @@ -89,10 +89,10 @@ export const average = (dataSet: number[]): number => { * @internal */ export const median = (dataSet: number[]): number => { - if (Array.isArray(dataSet) && dataSet.length === 0) { + if (!Array.isArray(dataSet) || dataSet.length === 0) { return 0 } - if (Array.isArray(dataSet) && dataSet.length === 1) { + if (dataSet.length === 1) { return dataSet[0] } const sortedDataSet = dataSet.slice().sort((a, b) => a - b) @@ -105,7 +105,6 @@ export const median = (dataSet: number[]): number => { /** * Rounds the given number to the given scale. - * The rounding is done using the "round half away from zero" method. * @param num - The number to round. * @param scale - The scale to round to. * @returns The rounded number. @@ -113,7 +112,7 @@ export const median = (dataSet: number[]): number => { */ export const round = (num: number, scale = 2): number => { const rounder = 10 ** scale - return Math.round(num * rounder * (1 + Number.EPSILON)) / rounder + return Math.round((num + Math.sign(num) * Number.EPSILON) * rounder) / rounder } /** diff --git a/tests/circular-buffer.test.mjs b/tests/circular-buffer.test.mjs index b597aafe8..c69408c6c 100644 --- a/tests/circular-buffer.test.mjs +++ b/tests/circular-buffer.test.mjs @@ -27,7 +27,7 @@ describe('Circular buffer test suite', () => { new TypeError("Invalid circular buffer size: '0.25' is not an integer") ) expect(() => new CircularBuffer(-1)).toThrow( - new RangeError('Invalid circular buffer size: -1 < 0') + new RangeError('Invalid circular buffer size: -1 <= 0') ) expect(() => new CircularBuffer(Number.MAX_SAFE_INTEGER + 1)).toThrow( new TypeError( diff --git a/tests/pools/selection-strategies/worker-choice-strategies-context.test.mjs b/tests/pools/selection-strategies/worker-choice-strategies-context.test.mjs index 8d4d4fee3..9bb0ea793 100644 --- a/tests/pools/selection-strategies/worker-choice-strategies-context.test.mjs +++ b/tests/pools/selection-strategies/worker-choice-strategies-context.test.mjs @@ -96,7 +96,7 @@ describe('Worker choice strategies context test suite', () => { ) expect(() => workerChoiceStrategiesContext.execute()).toThrow( new Error( - `Worker node key chosen by ${workerChoiceStrategyUndefinedStub.name} is null or undefined after ${workerChoiceStrategiesContext.retries} retries` + `Worker node key chosen by ${workerChoiceStrategyUndefinedStub.name} is null or undefined after ${workerChoiceStrategiesContext.retries.toString()} retries (max: ${workerChoiceStrategiesContext.retries.toString()})` ) ) const workerChoiceStrategyNullStub = createStubInstance( @@ -111,7 +111,7 @@ describe('Worker choice strategies context test suite', () => { ) expect(() => workerChoiceStrategiesContext.execute()).toThrow( new Error( - `Worker node key chosen by ${workerChoiceStrategyNullStub.name} is null or undefined after ${workerChoiceStrategiesContext.retries} retries` + `Worker node key chosen by ${workerChoiceStrategyNullStub.name} is null or undefined after ${workerChoiceStrategiesContext.retries.toString()} retries (max: ${workerChoiceStrategiesContext.retries.toString()})` ) ) }) diff --git a/tests/queues/priority-queue.test.mjs b/tests/queues/priority-queue.test.mjs index 64174908e..5d3d5ac07 100644 --- a/tests/queues/priority-queue.test.mjs +++ b/tests/queues/priority-queue.test.mjs @@ -11,7 +11,7 @@ describe('Priority queue test suite', () => { new TypeError("Invalid bucket size: '' is not an integer") ) expect(() => new PriorityQueue(-1)).toThrow( - new RangeError('Invalid bucket size: -1 < 0') + new RangeError('Invalid bucket size: -1 <= 0') ) let priorityQueue = new PriorityQueue() expect(priorityQueue.bucketSize).toBe(defaultBucketSize) diff --git a/tests/utils.test.mjs b/tests/utils.test.mjs index 92bbcbc78..e3deab3c3 100644 --- a/tests/utils.test.mjs +++ b/tests/utils.test.mjs @@ -83,11 +83,11 @@ describe('Utils test suite', () => { expect(round(-0.5, 0)).toBe(-1) expect(round(-0.5)).toBe(-0.5) expect(round(1.005)).toBe(1.01) - expect(round(2.175)).toBe(2.18) - expect(round(5.015)).toBe(5.02) + expect(round(2.175)).toBe(2.17) + expect(round(5.015)).toBe(5.01) expect(round(-1.005)).toBe(-1.01) - expect(round(-2.175)).toBe(-2.18) - expect(round(-5.015)).toBe(-5.02) + expect(round(-2.175)).toBe(-2.17) + expect(round(-5.015)).toBe(-5.01) }) it('Verify isPlainObject() behavior', () => { -- 2.43.0