From 7fbbde46b76cda9fc9def85c80984bf97f8fc529 Mon Sep 17 00:00:00 2001 From: Micha Reiser Date: Wed, 2 Dec 2020 11:32:37 +0000 Subject: [PATCH 1/8] Schedule on first available worker --- packages/jest-worker/src/Farm.ts | 6 +----- .../jest-worker/src/__tests__/Farm.test.js | 6 +++--- .../src/__tests__/process-integration.test.js | 16 ++++++++++++---- .../src/__tests__/thread-integration.test.js | 18 +++++++++++++----- 4 files changed, 29 insertions(+), 17 deletions(-) diff --git a/packages/jest-worker/src/Farm.ts b/packages/jest-worker/src/Farm.ts index 2c3950c7a30f..79c56c427429 100644 --- a/packages/jest-worker/src/Farm.ts +++ b/packages/jest-worker/src/Farm.ts @@ -27,7 +27,6 @@ export default class Farm { private _last: Array; private _locks: Array; private _numOfWorkers: number; - private _offset: number; private _queue: Array; constructor( @@ -40,7 +39,6 @@ export default class Farm { this._last = []; this._locks = []; this._numOfWorkers = numOfWorkers; - this._offset = 0; this._queue = []; if (computeWorkerKey) { @@ -173,11 +171,9 @@ export default class Farm { private _push(task: QueueChildMessage): Farm { for (let i = 0; i < this._numOfWorkers; i++) { - this._enqueue(task, (this._offset + i) % this._numOfWorkers); + this._enqueue(task, i); } - this._offset++; - return this; } diff --git a/packages/jest-worker/src/__tests__/Farm.test.js b/packages/jest-worker/src/__tests__/Farm.test.js index fcdaef7c54cd..b216a90843da 100644 --- a/packages/jest-worker/src/__tests__/Farm.test.js +++ b/packages/jest-worker/src/__tests__/Farm.test.js @@ -135,11 +135,11 @@ describe('Farm', () => { const farm = new Farm(4, callback, computeWorkerKey); const p0 = farm.doWork('foo', 42); - workerReply(0); - await p0; - const p1 = farm.doWork('foo1', 43); + + workerReply(0); workerReply(1); + await p0; await p1; const p2 = farm.doWork('foo2', 44); diff --git a/packages/jest-worker/src/__tests__/process-integration.test.js b/packages/jest-worker/src/__tests__/process-integration.test.js index b2d716669915..1db2f1f063e0 100644 --- a/packages/jest-worker/src/__tests__/process-integration.test.js +++ b/packages/jest-worker/src/__tests__/process-integration.test.js @@ -75,7 +75,7 @@ describe('Jest Worker Integration', () => { expect(await promise).toBe(42); }); - it('distributes sequential calls across child processes', async () => { + it('schedules the task on the first available child processes', async () => { const farm = new Farm('/tmp/baz.js', { exposedMethods: ['foo', 'bar'], numWorkers: 4, @@ -83,17 +83,25 @@ describe('Jest Worker Integration', () => { // The first call will go to the first child process. const promise0 = farm.foo('param-0'); - assertCallsToChild(0, ['foo', 'param-0']); - replySuccess(0, 'worker-0'); - expect(await promise0).toBe('worker-0'); // The second call will go to the second child process. const promise1 = farm.foo(1); + // The first task on worker 0 completes + replySuccess(0, 'worker-0'); + expect(await promise0).toBe('worker-0'); + + // The second task on worker 1 completes assertCallsToChild(1, ['foo', 1]); replySuccess(1, 'worker-1'); expect(await promise1).toBe('worker-1'); + + // The third call will go to the first child process + const promise2 = farm.foo('param-2'); + assertCallsToChild(0, ['foo', 'param-0'], ['foo', 'param-2']); + replySuccess(0, 'worker-0'); + expect(await promise2).toBe('worker-0'); }); it('distributes concurrent calls across child processes', async () => { diff --git a/packages/jest-worker/src/__tests__/thread-integration.test.js b/packages/jest-worker/src/__tests__/thread-integration.test.js index fb97ecf24b30..f167d304cdf5 100644 --- a/packages/jest-worker/src/__tests__/thread-integration.test.js +++ b/packages/jest-worker/src/__tests__/thread-integration.test.js @@ -77,7 +77,7 @@ describe('Jest Worker Process Integration', () => { expect(await promise).toBe(42); }); - it('distributes sequential calls across child processes', async () => { + it('schedules the task on the first available child processes', async () => { const farm = new Farm('/tmp/baz.js', { enableWorkerThreads: true, exposedMethods: ['foo', 'bar'], @@ -86,17 +86,25 @@ describe('Jest Worker Process Integration', () => { // The first call will go to the first child process. const promise0 = farm.foo('param-0'); - assertCallsToChild(0, ['foo', 'param-0']); - replySuccess(0, 'worker-0'); - expect(await promise0).toBe('worker-0'); // The second call will go to the second child process. const promise1 = farm.foo(1); - assertCallsToChild(1, ['foo', 1]); + + // task 1 on worker 0 completes + replySuccess(0, 'worker-0'); + expect(await promise0).toBe('worker-0'); + + // task 2 on worker 0 completes replySuccess(1, 'worker-1'); expect(await promise1).toBe('worker-1'); + + // The third call will go to the first process because it became available + const promise2 = farm.foo('param-2'); + assertCallsToChild(0, ['foo', 'param-0'], ['foo', 'param-2']); + replySuccess(0, 'worker-0'); + expect(await promise2).toBe('worker-0'); }); it('distributes concurrent calls across child processes', async () => { From 2d2fde4e2858fd8144600812347d3187a404ad86 Mon Sep 17 00:00:00 2001 From: Micha Reiser Date: Wed, 2 Dec 2020 16:03:25 +0000 Subject: [PATCH 2/8] Update changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index cfd8a5b022bd..416ac18a80a9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -45,6 +45,7 @@ - `[jest-mock]` [**BREAKING**] Migrate to ESM ([#10887](https://github.com/facebook/jest/pull/10887)) - `[jest-runner]` [**BREAKING**] Migrate to ESM ([#10900](https://github.com/facebook/jest/pull/10900)) - `[jest-util]` No longer checking `enumerable` when adding `process.domain` ([#10862](https://github.com/facebook/jest/pull/10862)) +- `[jest-worker]` Change from round-robin to first available worker scheduling ([10902](https://github.com/facebook/jest/pull/10902)) ### Performance From 84d81f61c02bb875c6d828451bb222e861cac920 Mon Sep 17 00:00:00 2001 From: Micha Reiser Date: Thu, 3 Dec 2020 17:37:59 +0000 Subject: [PATCH 3/8] Make scheduling policy configurable --- CHANGELOG.md | 2 +- packages/jest-worker/src/Farm.ts | 18 +++++-- .../jest-worker/src/__tests__/Farm.test.js | 20 +++++--- .../src/__tests__/process-integration.test.js | 22 ++++++++- .../src/__tests__/thread-integration.test.js | 49 +++++++++++++++++-- packages/jest-worker/src/index.ts | 5 +- packages/jest-worker/src/types.ts | 1 + 7 files changed, 97 insertions(+), 20 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 416ac18a80a9..a85babd97bba 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,7 @@ - `[jest-snapshot]` [**BREAKING**] Make prettier optional for inline snapshots - fall back to string replacement ([#7792](https://github.com/facebook/jest/pull/7792)) - `[jest-runner]` [**BREAKING**] Run transforms over `runnner` ([#8823](https://github.com/facebook/jest/pull/8823)) - `[jest-runner]` [**BREAKING**] Run transforms over `testRunnner` ([#8823](https://github.com/facebook/jest/pull/8823)) +- `[jest-worker]` Add first-come scheduling policy to jest worker ([10902](https://github.com/facebook/jest/pull/10902)) ### Fixes @@ -45,7 +46,6 @@ - `[jest-mock]` [**BREAKING**] Migrate to ESM ([#10887](https://github.com/facebook/jest/pull/10887)) - `[jest-runner]` [**BREAKING**] Migrate to ESM ([#10900](https://github.com/facebook/jest/pull/10900)) - `[jest-util]` No longer checking `enumerable` when adding `process.domain` ([#10862](https://github.com/facebook/jest/pull/10862)) -- `[jest-worker]` Change from round-robin to first available worker scheduling ([10902](https://github.com/facebook/jest/pull/10902)) ### Performance diff --git a/packages/jest-worker/src/Farm.ts b/packages/jest-worker/src/Farm.ts index 79c56c427429..4f1c1f112af9 100644 --- a/packages/jest-worker/src/Farm.ts +++ b/packages/jest-worker/src/Farm.ts @@ -22,17 +22,22 @@ import { export default class Farm { private _computeWorkerKey: FarmOptions['computeWorkerKey']; + private _workerSchedulingPolicy: 'round-robin' | 'first-come'; private _cacheKeys: Record; private _callback: Function; private _last: Array; private _locks: Array; private _numOfWorkers: number; + private _offset: number = 0; private _queue: Array; constructor( numOfWorkers: number, callback: Function, - computeWorkerKey?: FarmOptions['computeWorkerKey'], + options: { + computeWorkerKey?: FarmOptions['computeWorkerKey']; + workerSchedulingPolicy?: FarmOptions['workerSchedulingPolicy']; + } = {}, ) { this._cacheKeys = Object.create(null); this._callback = callback; @@ -41,9 +46,9 @@ export default class Farm { this._numOfWorkers = numOfWorkers; this._queue = []; - if (computeWorkerKey) { - this._computeWorkerKey = computeWorkerKey; - } + this._computeWorkerKey = options.computeWorkerKey; + this._workerSchedulingPolicy = + options.workerSchedulingPolicy ?? 'round-robin'; } doWork( @@ -170,8 +175,11 @@ export default class Farm { } private _push(task: QueueChildMessage): Farm { + const offset = + this._workerSchedulingPolicy === 'round-robin' ? this._offset++ : 0; + for (let i = 0; i < this._numOfWorkers; i++) { - this._enqueue(task, i); + this._enqueue(task, (i + offset) % this._numOfWorkers); } return this; diff --git a/packages/jest-worker/src/__tests__/Farm.test.js b/packages/jest-worker/src/__tests__/Farm.test.js index b216a90843da..b8aacfe0242b 100644 --- a/packages/jest-worker/src/__tests__/Farm.test.js +++ b/packages/jest-worker/src/__tests__/Farm.test.js @@ -105,7 +105,7 @@ describe('Farm', () => { it('handles null computeWorkerKey, sending to first worker', async () => { const computeWorkerKey = jest.fn(() => null); - const farm = new Farm(4, callback, computeWorkerKey); + const farm = new Farm(4, callback, {computeWorkerKey}); const p0 = farm.doWork('foo', 42); workerReply(0); @@ -132,14 +132,14 @@ describe('Farm', () => { .mockReturnValueOnce('two') .mockReturnValueOnce('one'); - const farm = new Farm(4, callback, computeWorkerKey); + const farm = new Farm(4, callback, {computeWorkerKey}); const p0 = farm.doWork('foo', 42); - const p1 = farm.doWork('foo1', 43); - workerReply(0); - workerReply(1); await p0; + + const p1 = farm.doWork('foo1', 43); + workerReply(1); await p1; const p2 = farm.doWork('foo2', 44); @@ -208,7 +208,9 @@ describe('Farm', () => { }); it('checks that once a sticked task finishes, next time is sent to that worker', async () => { - const farm = new Farm(4, callback, () => '1234567890abcdef'); + const farm = new Farm(4, callback, { + computeWorkerKey: () => '1234567890abcdef', + }); // Worker 1 successfully replies with "17" as a result. const p0 = farm.doWork('car', 'plane'); @@ -248,7 +250,9 @@ describe('Farm', () => { }); it('checks that even before a sticked task finishes, next time is sent to that worker', async () => { - const farm = new Farm(4, callback, () => '1234567890abcdef'); + const farm = new Farm(4, callback, { + computeWorkerKey: () => '1234567890abcdef', + }); // Note that the worker is sending a start response synchronously. const p0 = farm.doWork('car', 'plane'); @@ -310,7 +314,7 @@ describe('Farm', () => { // Push onto the second queue; potentially wiping the earlier job. .mockReturnValueOnce(1); - const farm = new Farm(2, callback, hash); + const farm = new Farm(2, callback, {computeWorkerKey: hash}); // First and second jobs get resolved, so that their hash is sticked to // the right worker: worker assignment is performed when workers reply, not diff --git a/packages/jest-worker/src/__tests__/process-integration.test.js b/packages/jest-worker/src/__tests__/process-integration.test.js index 1db2f1f063e0..496c5c79b914 100644 --- a/packages/jest-worker/src/__tests__/process-integration.test.js +++ b/packages/jest-worker/src/__tests__/process-integration.test.js @@ -75,12 +75,32 @@ describe('Jest Worker Integration', () => { expect(await promise).toBe(42); }); - it('schedules the task on the first available child processes', async () => { + it('distributes sequential calls across child processes', async () => { const farm = new Farm('/tmp/baz.js', { exposedMethods: ['foo', 'bar'], numWorkers: 4, }); + // The first call will go to the first child process. + const promise0 = farm.foo('param-0'); + assertCallsToChild(0, ['foo', 'param-0']); + replySuccess(0, 'worker-0'); + expect(await promise0).toBe('worker-0'); + + // The second call will go to the second child process. + const promise1 = farm.foo(1); + assertCallsToChild(1, ['foo', 1]); + replySuccess(1, 'worker-1'); + expect(await promise1).toBe('worker-1'); + }); + + it('schedules the task on the first available child processes if the scheduling policy is first come', async () => { + const farm = new Farm('/tmp/baz.js', { + exposedMethods: ['foo', 'bar'], + numWorkers: 4, + workerSchedulingPolicy: 'first-come', + }); + // The first call will go to the first child process. const promise0 = farm.foo('param-0'); assertCallsToChild(0, ['foo', 'param-0']); diff --git a/packages/jest-worker/src/__tests__/thread-integration.test.js b/packages/jest-worker/src/__tests__/thread-integration.test.js index f167d304cdf5..3061ea60d924 100644 --- a/packages/jest-worker/src/__tests__/thread-integration.test.js +++ b/packages/jest-worker/src/__tests__/thread-integration.test.js @@ -77,7 +77,7 @@ describe('Jest Worker Process Integration', () => { expect(await promise).toBe(42); }); - it('schedules the task on the first available child processes', async () => { + it('distributes sequential calls across child processes', async () => { const farm = new Farm('/tmp/baz.js', { enableWorkerThreads: true, exposedMethods: ['foo', 'bar'], @@ -87,26 +87,67 @@ describe('Jest Worker Process Integration', () => { // The first call will go to the first child process. const promise0 = farm.foo('param-0'); assertCallsToChild(0, ['foo', 'param-0']); + replySuccess(0, 'worker-0'); + expect(await promise0).toBe('worker-0'); // The second call will go to the second child process. const promise1 = farm.foo(1); assertCallsToChild(1, ['foo', 1]); + replySuccess(1, 'worker-1'); + expect(await promise1).toBe('worker-1'); + }); + + it('schedules the task on the first available child processes if the scheduling policy is first come', async () => { + const farm = new Farm('/tmp/baz.js', { + enableWorkerThreads: true, + exposedMethods: ['foo', 'bar'], + numWorkers: 4, + workerSchedulingPolicy: 'first-come', + }); + + // The first call will go to the first child process. + const promise0 = farm.foo('param-0'); + assertCallsToChild(0, ['foo', 'param-0']); - // task 1 on worker 0 completes + // The second call will go to the second child process. + const promise1 = farm.foo(1); + + // The first task on worker 0 completes replySuccess(0, 'worker-0'); expect(await promise0).toBe('worker-0'); - // task 2 on worker 0 completes + // The second task on worker 1 completes + assertCallsToChild(1, ['foo', 1]); replySuccess(1, 'worker-1'); expect(await promise1).toBe('worker-1'); - // The third call will go to the first process because it became available + // The third call will go to the first child process const promise2 = farm.foo('param-2'); assertCallsToChild(0, ['foo', 'param-0'], ['foo', 'param-2']); replySuccess(0, 'worker-0'); expect(await promise2).toBe('worker-0'); }); + it('schedules the task on the first available child processes', async () => { + const farm = new Farm('/tmp/baz.js', { + enableWorkerThreads: true, + exposedMethods: ['foo', 'bar'], + numWorkers: 4, + }); + + // The first call will go to the first child process. + const promise0 = farm.foo('param-0'); + assertCallsToChild(0, ['foo', 'param-0']); + replySuccess(0, 'worker-0'); + expect(await promise0).toBe('worker-0'); + + // The second call will go to the second child process. + const promise1 = farm.foo(1); + assertCallsToChild(1, ['foo', 1]); + replySuccess(1, 'worker-1'); + expect(await promise1).toBe('worker-1'); + }); + it('distributes concurrent calls across child processes', async () => { const farm = new Farm('/tmp/baz.js', { enableWorkerThreads: true, diff --git a/packages/jest-worker/src/index.ts b/packages/jest-worker/src/index.ts index 5b2613acda37..01e791ff1eb3 100644 --- a/packages/jest-worker/src/index.ts +++ b/packages/jest-worker/src/index.ts @@ -99,7 +99,10 @@ export default class JestWorker { this._farm = new Farm( workerPoolOptions.numWorkers, this._workerPool.send.bind(this._workerPool), - this._options.computeWorkerKey, + { + computeWorkerKey: this._options.computeWorkerKey, + workerSchedulingPolicy: this._options.workerSchedulingPolicy, + }, ); this._bindExposedWorkerMethods(workerPath, this._options); diff --git a/packages/jest-worker/src/types.ts b/packages/jest-worker/src/types.ts index 9afa7f92f3c5..86c640ccc244 100644 --- a/packages/jest-worker/src/types.ts +++ b/packages/jest-worker/src/types.ts @@ -79,6 +79,7 @@ export type FarmOptions = { computeWorkerKey?: (method: string, ...args: Array) => string | null; exposedMethods?: ReadonlyArray; forkOptions?: ForkOptions; + workerSchedulingPolicy?: 'round-robin' | 'first-come'; resourceLimits?: ResourceLimits; setupArgs?: Array; maxRetries?: number; From f944dcfaef46ded5a0ea98279bfa60740131d245 Mon Sep 17 00:00:00 2001 From: Micha Reiser Date: Thu, 3 Dec 2020 19:43:46 +0000 Subject: [PATCH 4/8] Update documentation and rename first-come to in-order --- packages/jest-worker/README.md | 9 +++++++++ packages/jest-worker/src/Farm.ts | 2 +- .../src/__tests__/process-integration.test.js | 4 ++-- .../jest-worker/src/__tests__/thread-integration.test.js | 4 ++-- packages/jest-worker/src/types.ts | 2 +- 5 files changed, 15 insertions(+), 6 deletions(-) diff --git a/packages/jest-worker/README.md b/packages/jest-worker/README.md index d0523c940040..464b9cad7544 100644 --- a/packages/jest-worker/README.md +++ b/packages/jest-worker/README.md @@ -91,6 +91,15 @@ Provide a custom worker pool to be used for spawning child processes. By default `jest-worker` will automatically detect if `worker_threads` are available, but will not use them unless passed `enableWorkerThreads: true`. +### `workerSchedulingPolicy: 'round-robin' | 'first-come'` (optional) + +Specifies the policy how tasks are assigned to workers if multiple workers are *idle*: + +* `round-robin` (default): The task will be sequentially distributed onto the workers. The first task is assigned to the worker 1, the second to the worker 2, to ensure that the work is distributed across workers. +* `in-order`: The task will be assigned to the first free worker starting with worker 1 and only assign the work to worker 2 if the worker 1 is busy. + +Tasks are always assigned to the first free worker as soon as tasks start to queue up. The scheduling policy does not define the task scheduling which is always first-in, first-out. + ## JestWorker ### Methods diff --git a/packages/jest-worker/src/Farm.ts b/packages/jest-worker/src/Farm.ts index 4f1c1f112af9..35b7ccecffc1 100644 --- a/packages/jest-worker/src/Farm.ts +++ b/packages/jest-worker/src/Farm.ts @@ -22,7 +22,7 @@ import { export default class Farm { private _computeWorkerKey: FarmOptions['computeWorkerKey']; - private _workerSchedulingPolicy: 'round-robin' | 'first-come'; + private _workerSchedulingPolicy: FarmOptions['workerSchedulingPolicy']; private _cacheKeys: Record; private _callback: Function; private _last: Array; diff --git a/packages/jest-worker/src/__tests__/process-integration.test.js b/packages/jest-worker/src/__tests__/process-integration.test.js index 496c5c79b914..187d312c44a9 100644 --- a/packages/jest-worker/src/__tests__/process-integration.test.js +++ b/packages/jest-worker/src/__tests__/process-integration.test.js @@ -94,11 +94,11 @@ describe('Jest Worker Integration', () => { expect(await promise1).toBe('worker-1'); }); - it('schedules the task on the first available child processes if the scheduling policy is first come', async () => { + it('schedules the task on the first available child processes if the scheduling policy is in-order', async () => { const farm = new Farm('/tmp/baz.js', { exposedMethods: ['foo', 'bar'], numWorkers: 4, - workerSchedulingPolicy: 'first-come', + workerSchedulingPolicy: 'in-order', }); // The first call will go to the first child process. diff --git a/packages/jest-worker/src/__tests__/thread-integration.test.js b/packages/jest-worker/src/__tests__/thread-integration.test.js index 3061ea60d924..18c277bdedc2 100644 --- a/packages/jest-worker/src/__tests__/thread-integration.test.js +++ b/packages/jest-worker/src/__tests__/thread-integration.test.js @@ -97,12 +97,12 @@ describe('Jest Worker Process Integration', () => { expect(await promise1).toBe('worker-1'); }); - it('schedules the task on the first available child processes if the scheduling policy is first come', async () => { + it('schedules the task on the first available child processes if the scheduling policy is in-order', async () => { const farm = new Farm('/tmp/baz.js', { enableWorkerThreads: true, exposedMethods: ['foo', 'bar'], numWorkers: 4, - workerSchedulingPolicy: 'first-come', + workerSchedulingPolicy: 'in-order', }); // The first call will go to the first child process. diff --git a/packages/jest-worker/src/types.ts b/packages/jest-worker/src/types.ts index 86c640ccc244..fa83ea7af2d4 100644 --- a/packages/jest-worker/src/types.ts +++ b/packages/jest-worker/src/types.ts @@ -79,7 +79,7 @@ export type FarmOptions = { computeWorkerKey?: (method: string, ...args: Array) => string | null; exposedMethods?: ReadonlyArray; forkOptions?: ForkOptions; - workerSchedulingPolicy?: 'round-robin' | 'first-come'; + workerSchedulingPolicy?: 'round-robin' | 'in-order'; resourceLimits?: ResourceLimits; setupArgs?: Array; maxRetries?: number; From 2e229ffb6e5486087c64c4ae504c6725e6b9534f Mon Sep 17 00:00:00 2001 From: Micha Reiser Date: Thu, 3 Dec 2020 19:46:08 +0000 Subject: [PATCH 5/8] Update CHANGELOG.md --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a85babd97bba..072177ad7f0e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,7 +9,7 @@ - `[jest-snapshot]` [**BREAKING**] Make prettier optional for inline snapshots - fall back to string replacement ([#7792](https://github.com/facebook/jest/pull/7792)) - `[jest-runner]` [**BREAKING**] Run transforms over `runnner` ([#8823](https://github.com/facebook/jest/pull/8823)) - `[jest-runner]` [**BREAKING**] Run transforms over `testRunnner` ([#8823](https://github.com/facebook/jest/pull/8823)) -- `[jest-worker]` Add first-come scheduling policy to jest worker ([10902](https://github.com/facebook/jest/pull/10902)) +- `[jest-worker]` Add in-order scheduling policy to jest worker ([10902](https://github.com/facebook/jest/pull/10902)) ### Fixes From 9f000d014383a079682f84ac599f0bc2adda4c48 Mon Sep 17 00:00:00 2001 From: Micha Reiser Date: Thu, 3 Dec 2020 20:03:50 +0000 Subject: [PATCH 6/8] Format Readme --- packages/jest-worker/README.md | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/jest-worker/README.md b/packages/jest-worker/README.md index 464b9cad7544..5efd016a054d 100644 --- a/packages/jest-worker/README.md +++ b/packages/jest-worker/README.md @@ -91,12 +91,12 @@ Provide a custom worker pool to be used for spawning child processes. By default `jest-worker` will automatically detect if `worker_threads` are available, but will not use them unless passed `enableWorkerThreads: true`. -### `workerSchedulingPolicy: 'round-robin' | 'first-come'` (optional) +### `workerSchedulingPolicy: 'round-robin' | 'in-order'` (optional) -Specifies the policy how tasks are assigned to workers if multiple workers are *idle*: +Specifies the policy how tasks are assigned to workers if multiple workers are _idle_: -* `round-robin` (default): The task will be sequentially distributed onto the workers. The first task is assigned to the worker 1, the second to the worker 2, to ensure that the work is distributed across workers. -* `in-order`: The task will be assigned to the first free worker starting with worker 1 and only assign the work to worker 2 if the worker 1 is busy. +- `round-robin` (default): The task will be sequentially distributed onto the workers. The first task is assigned to the worker 1, the second to the worker 2, to ensure that the work is distributed across workers. +- `in-order`: The task will be assigned to the first free worker starting with worker 1 and only assign the work to worker 2 if the worker 1 is busy. Tasks are always assigned to the first free worker as soon as tasks start to queue up. The scheduling policy does not define the task scheduling which is always first-in, first-out. From 03606edd4aa2ec99555a74a571c41348380d8168 Mon Sep 17 00:00:00 2001 From: Micha Reiser Date: Sat, 26 Dec 2020 14:14:18 +0000 Subject: [PATCH 7/8] Fix incorrect merge --- packages/jest-worker/src/Farm.ts | 2 ++ packages/jest-worker/src/index.ts | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/packages/jest-worker/src/Farm.ts b/packages/jest-worker/src/Farm.ts index 7138b14e5a7e..e886adc37405 100644 --- a/packages/jest-worker/src/Farm.ts +++ b/packages/jest-worker/src/Farm.ts @@ -149,6 +149,8 @@ export default class Farm { } private _push(task: QueueChildMessage): Farm { + this._taskQueue.enqueue(task); + const offset = this._workerSchedulingPolicy === 'round-robin' ? this._offset++ : 0; diff --git a/packages/jest-worker/src/index.ts b/packages/jest-worker/src/index.ts index 60e3bd264175..50be5683dfba 100644 --- a/packages/jest-worker/src/index.ts +++ b/packages/jest-worker/src/index.ts @@ -104,8 +104,8 @@ export class Worker { this._workerPool.send.bind(this._workerPool), { computeWorkerKey: this._options.computeWorkerKey, - workerSchedulingPolicy: this._options.workerSchedulingPolicy, taskQueue: this._options.taskQueue, + workerSchedulingPolicy: this._options.workerSchedulingPolicy, }, ); From 111abcdbb67755f721ce3335218a98f98604b58c Mon Sep 17 00:00:00 2001 From: Micha Reiser Date: Sun, 27 Dec 2020 10:43:53 +0000 Subject: [PATCH 8/8] Address review comments --- packages/jest-worker/src/Farm.ts | 43 +++++++++++++++++++------------- 1 file changed, 25 insertions(+), 18 deletions(-) diff --git a/packages/jest-worker/src/Farm.ts b/packages/jest-worker/src/Farm.ts index e886adc37405..c87897a78d1a 100644 --- a/packages/jest-worker/src/Farm.ts +++ b/packages/jest-worker/src/Farm.ts @@ -22,29 +22,26 @@ import { } from './types'; export default class Farm { - private _computeWorkerKey: FarmOptions['computeWorkerKey']; - private _workerSchedulingPolicy: FarmOptions['workerSchedulingPolicy']; - private _cacheKeys: Record; - private _callback: Function; - private _locks: Array; - private _numOfWorkers: number; - private _offset: number = 0; - private _taskQueue: TaskQueue; + private readonly _computeWorkerKey: FarmOptions['computeWorkerKey']; + private readonly _workerSchedulingPolicy: NonNullable< + FarmOptions['workerSchedulingPolicy'] + >; + private readonly _cacheKeys: Record = Object.create( + null, + ); + private readonly _locks: Array = []; + private _offset = 0; + private readonly _taskQueue: TaskQueue; constructor( - numOfWorkers: number, - callback: Function, + private _numOfWorkers: number, + private _callback: Function, options: { computeWorkerKey?: FarmOptions['computeWorkerKey']; workerSchedulingPolicy?: FarmOptions['workerSchedulingPolicy']; taskQueue?: TaskQueue; } = {}, ) { - this._cacheKeys = Object.create(null); - this._callback = callback; - this._locks = []; - this._numOfWorkers = numOfWorkers; - this._computeWorkerKey = options.computeWorkerKey; this._workerSchedulingPolicy = options.workerSchedulingPolicy ?? 'round-robin'; @@ -151,9 +148,7 @@ export default class Farm { private _push(task: QueueChildMessage): Farm { this._taskQueue.enqueue(task); - const offset = - this._workerSchedulingPolicy === 'round-robin' ? this._offset++ : 0; - + const offset = this._getNextWorkerOffset(); for (let i = 0; i < this._numOfWorkers; i++) { this._process((offset + i) % this._numOfWorkers); @@ -165,6 +160,18 @@ export default class Farm { return this; } + // Typescript ensures that the switch statement is exhaustive. + // Adding an explicit return at the end would disable the exhaustive check void. + // eslint-disable-next-line consistent-return + private _getNextWorkerOffset(): number { + switch (this._workerSchedulingPolicy) { + case 'in-order': + return 0; + case 'round-robin': + return this._offset++; + } + } + private _lock(workerId: number): void { this._locks[workerId] = true; }