From 390300c363d3535fd622f07d54c40cfad9fdbb0b Mon Sep 17 00:00:00 2001 From: =?utf8?q?J=C3=A9r=C3=B4me=20Benoit?= Date: Tue, 13 Aug 2024 20:22:44 +0200 Subject: [PATCH] perf: do mapExecute() args sanity checks once MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit Signed-off-by: Jérôme Benoit --- src/pools/abstract-pool.ts | 74 ++++++++++++++++++------------ tests/pools/abstract-pool.test.mjs | 22 +++++++-- 2 files changed, 62 insertions(+), 34 deletions(-) diff --git a/src/pools/abstract-pool.ts b/src/pools/abstract-pool.ts index 29dd42db..a45eef05 100644 --- a/src/pools/abstract-pool.ts +++ b/src/pools/abstract-pool.ts @@ -1167,37 +1167,12 @@ export abstract class AbstractPool< ) } - /** @inheritDoc */ - public async execute ( + private async internalExecute ( data?: Data, name?: string, transferList?: readonly TransferListItem[] ): Promise { return await new Promise((resolve, reject) => { - if (!this.started) { - reject(new Error('Cannot execute a task on not started pool')) - return - } - if (this.destroying) { - reject(new Error('Cannot execute a task on destroying pool')) - return - } - if (name != null && typeof name !== 'string') { - reject(new TypeError('name argument must be a string')) - return - } - if ( - name != null && - typeof name === 'string' && - name.trim().length === 0 - ) { - reject(new TypeError('name argument must not be an empty string')) - return - } - if (transferList != null && !Array.isArray(transferList)) { - reject(new TypeError('transferList argument must be an array')) - return - } const timestamp = performance.now() const workerNodeKey = this.chooseWorkerNode(name) const task: Task = { @@ -1237,11 +1212,41 @@ export abstract class AbstractPool< } /** @inheritDoc */ - public mapExecute ( + public async execute ( + data?: Data, + name?: string, + transferList?: readonly TransferListItem[] + ): Promise { + if (!this.started) { + throw new Error('Cannot execute a task on not started pool') + } + if (this.destroying) { + throw new Error('Cannot execute a task on destroying pool') + } + if (name != null && typeof name !== 'string') { + throw new TypeError('name argument must be a string') + } + if (name != null && typeof name === 'string' && name.trim().length === 0) { + throw new TypeError('name argument must not be an empty string') + } + if (transferList != null && !Array.isArray(transferList)) { + throw new TypeError('transferList argument must be an array') + } + return await this.internalExecute(data, name, transferList) + } + + /** @inheritDoc */ + public async mapExecute ( data: Iterable, name?: string, transferList?: readonly TransferListItem[] ): Promise { + if (!this.started) { + throw new Error('Cannot execute task(s) on not started pool') + } + if (this.destroying) { + throw new Error('Cannot execute task(s) on destroying pool') + } // eslint-disable-next-line @typescript-eslint/no-unnecessary-condition if (data == null) { throw new TypeError('data argument must be a defined iterable') @@ -1249,11 +1254,22 @@ export abstract class AbstractPool< if (typeof data[Symbol.iterator] !== 'function') { throw new TypeError('data argument must be an iterable') } + if (name != null && typeof name !== 'string') { + throw new TypeError('name argument must be a string') + } + if (name != null && typeof name === 'string' && name.trim().length === 0) { + throw new TypeError('name argument must not be an empty string') + } + if (transferList != null && !Array.isArray(transferList)) { + throw new TypeError('transferList argument must be an array') + } if (!Array.isArray(data)) { data = [...data] } - return Promise.all( - (data as Data[]).map(data => this.execute(data, name, transferList)) + return await Promise.all( + (data as Data[]).map(data => + this.internalExecute(data, name, transferList) + ) ) } diff --git a/tests/pools/abstract-pool.test.mjs b/tests/pools/abstract-pool.test.mjs index f8b983d4..375d9653 100644 --- a/tests/pools/abstract-pool.test.mjs +++ b/tests/pools/abstract-pool.test.mjs @@ -927,9 +927,6 @@ describe('Abstract pool test suite', () => { expect(pool.info.ready).toBe(false) expect(pool.workerNodes).toStrictEqual([]) expect(pool.readyEventEmitted).toBe(false) - await expect(pool.execute()).rejects.toThrow( - new Error('Cannot execute a task on not started pool') - ) pool.start() expect(pool.info.started).toBe(true) expect(pool.info.ready).toBe(true) @@ -1767,12 +1764,24 @@ describe('Abstract pool test suite', () => { numberOfWorkers, './tests/worker-files/thread/testMultipleTaskFunctionsWorker.mjs' ) - expect(() => pool.mapExecute()).toThrow( + await expect(pool.mapExecute()).rejects.toThrow( new TypeError('data argument must be a defined iterable') ) - expect(() => pool.mapExecute(0)).toThrow( + await expect(pool.mapExecute(0)).rejects.toThrow( new TypeError('data argument must be an iterable') ) + await expect(pool.mapExecute([undefined], 0)).rejects.toThrow( + new TypeError('name argument must be a string') + ) + await expect(pool.mapExecute([undefined], '')).rejects.toThrow( + new TypeError('name argument must not be an empty string') + ) + await expect(pool.mapExecute([undefined], undefined, {})).rejects.toThrow( + new TypeError('transferList argument must be an array') + ) + await expect(pool.mapExecute([undefined], 'unknown')).rejects.toBe( + "Task function 'unknown' not found" + ) let results = await pool.mapExecute([{}, {}, {}, {}]) expect(results).toStrictEqual([{ ok: 1 }, { ok: 1 }, { ok: 1 }, { ok: 1 }]) expect(pool.info.executingTasks).toBe(0) @@ -1796,6 +1805,9 @@ describe('Abstract pool test suite', () => { expect(pool.info.executingTasks).toBe(0) expect(pool.info.executedTasks).toBe(12) await pool.destroy() + await expect(pool.mapExecute()).rejects.toThrow( + new Error('Cannot execute task(s) on not started pool') + ) }) it('Verify that task function objects worker is working', async () => { -- 2.34.1