From 9cfcd81fd7a3f0b2dc9c004bd53a479035765ae9 Mon Sep 17 00:00:00 2001 From: Simen Bekkhus Date: Thu, 24 Jan 2019 13:20:29 +0100 Subject: [PATCH] make worker_threads opt-in, not opt-out (#7693) --- CHANGELOG.md | 2 +- packages/jest-cli/src/reporters/coverage_reporter.js | 1 - packages/jest-haste-map/src/index.js | 1 - packages/jest-runner/src/index.js | 1 - packages/jest-worker/README.md | 8 +++----- packages/jest-worker/src/WorkerPool.js | 2 +- packages/jest-worker/src/__tests__/WorkerPool.test.js | 4 ++-- .../jest-worker/src/__tests__/thread-integration.test.js | 4 ++++ packages/jest-worker/src/index.js | 2 +- packages/jest-worker/src/types.js | 4 ++-- 10 files changed, 14 insertions(+), 15 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1cd60cd8b786..cc93f37545ed 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -52,7 +52,7 @@ - `[jest-runner]` Instantiate the test environment class with the current `testPath` ([#7442](https://github.com/facebook/jest/pull/7442)) - `[jest-config]` Always resolve jest-environment-jsdom from jest-config ([#7476](https://github.com/facebook/jest/pull/7476)) - `[expect]` Improve report when assertion fails, part 6 ([#7621](https://github.com/facebook/jest/pull/7621)) -- `[jest-worker]` Add `disableWorkerThreads` option to explicitly opt out of `worker_threads` even if available ([#7681](https://github.com/facebook/jest/pull/7681)) +- `[jest-worker]` Add `enableWorkerThreads` option to explicitly opt-in to `worker_threads` if available ([#7681](https://github.com/facebook/jest/pull/7681)) ### Fixes diff --git a/packages/jest-cli/src/reporters/coverage_reporter.js b/packages/jest-cli/src/reporters/coverage_reporter.js index ba9dccf0cca7..81e45ec08a1e 100644 --- a/packages/jest-cli/src/reporters/coverage_reporter.js +++ b/packages/jest-cli/src/reporters/coverage_reporter.js @@ -155,7 +155,6 @@ export default class CoverageReporter extends BaseReporter { } else { // $FlowFixMe: assignment of a worker with custom properties. worker = new Worker(require.resolve('./coverage_worker'), { - disableWorkerThreads: true, exposedMethods: ['worker'], maxRetries: 2, numWorkers: this._globalConfig.maxWorkers, diff --git a/packages/jest-haste-map/src/index.js b/packages/jest-haste-map/src/index.js index a16457fabe32..c7c6a79ac841 100644 --- a/packages/jest-haste-map/src/index.js +++ b/packages/jest-haste-map/src/index.js @@ -672,7 +672,6 @@ class HasteMap extends EventEmitter { } else { // $FlowFixMe: assignment of a worker with custom properties. this._worker = (new Worker(require.resolve('./worker'), { - disableWorkerThreads: true, exposedMethods: ['getSha1', 'worker'], maxRetries: 3, numWorkers: this._options.maxWorkers, diff --git a/packages/jest-runner/src/index.js b/packages/jest-runner/src/index.js index bbb6abb026a3..ca4ccc141895 100644 --- a/packages/jest-runner/src/index.js +++ b/packages/jest-runner/src/index.js @@ -96,7 +96,6 @@ class TestRunner { ) { // $FlowFixMe: class object is augmented with worker when instantiating. const worker: WorkerInterface = new Worker(TEST_WORKER_PATH, { - disableWorkerThreads: true, exposedMethods: ['worker'], forkOptions: {stdio: 'pipe'}, maxRetries: 3, diff --git a/packages/jest-worker/README.md b/packages/jest-worker/README.md index 37394a9b92a3..e6ca3a798d10 100644 --- a/packages/jest-worker/README.md +++ b/packages/jest-worker/README.md @@ -43,9 +43,7 @@ export function hello(param) { Node 10 shipped with [worker-threads](https://nodejs.org/api/worker_threads.html), a "threading API" that uses SharedArrayBuffers to communicate between the main process and its child threads. This experimental Node feature can significantly improve the communication time between parent and child processes in `jest-worker`. -We will use worker threads where available. To enable in Node 10+, run the Node process with the `--experimental-worker` flag. - -You can explicitly opt-out of this by passing `disableWorkerThreads: true`. +Since `worker_threads` are considered experimental in Node, you have to opt-in to this behavior by passing `enableWorkerThreads: true` when instantiating the worker. While the feature was unflagged in Node 11.7.0, you'll need to run the Node process with the `--experimental-worker` flag for Node 10. ## API @@ -91,9 +89,9 @@ Provide a custom worker pool to be used for spawning child processes. By default The arguments that will be passed to the `setup` method during initialization. -#### `disableWorkerThreads: boolean` (optional) +#### `enableWorkerThreads: boolean` (optional) -`jest-worker` will automatically detect if `worker_threads` are available and use them. However, running under threads comes with [some caveats](https://nodejs.org/api/worker_threads.html#worker_threads_class_worker), and is still experimental, so you can `opt-out` of this and use `disableWorkerThreads: true`. +`jest-worker` will automatically detect if `worker_threads` are available, but will not use them unless passed `enableWorkerThreads: true`. ## Worker diff --git a/packages/jest-worker/src/WorkerPool.js b/packages/jest-worker/src/WorkerPool.js index 0d4491bfd456..0c6828288653 100644 --- a/packages/jest-worker/src/WorkerPool.js +++ b/packages/jest-worker/src/WorkerPool.js @@ -42,7 +42,7 @@ class WorkerPool extends BaseWorkerPool implements WorkerPoolInterface { createWorker(workerOptions: WorkerOptions): WorkerInterface { let Worker; - if (!this._options.disableWorkerThreads && canUseWorkerThreads()) { + if (this._options.enableWorkerThreads && canUseWorkerThreads()) { Worker = require('./workers/NodeThreadsWorker').default; } else { Worker = require('./workers/ChildProcessWorker').default; diff --git a/packages/jest-worker/src/__tests__/WorkerPool.test.js b/packages/jest-worker/src/__tests__/WorkerPool.test.js index 7d8e5a35eb9a..a5bafa91f0f0 100644 --- a/packages/jest-worker/src/__tests__/WorkerPool.test.js +++ b/packages/jest-worker/src/__tests__/WorkerPool.test.js @@ -77,6 +77,7 @@ describe('WorkerPool', () => { it('should create a NodeThreadWorker and send to it', () => { jest.mock('worker_threads', () => 'Defined'); const workerPool = new WorkerPool('/path', { + enableWorkerThreads: true, forkOptions: {}, maxRetries: 1, numWorkers: 1, @@ -102,10 +103,9 @@ describe('WorkerPool', () => { ); }); - it('should avoid NodeThreadWorker if passed disableWorkerThreads', () => { + it('should avoid NodeThreadWorker if not passed enableWorkerThreads', () => { jest.mock('worker_threads', () => 'Defined'); const workerPool = new WorkerPool('/path', { - disableWorkerThreads: true, forkOptions: {}, maxRetries: 1, numWorkers: 1, diff --git a/packages/jest-worker/src/__tests__/thread-integration.test.js b/packages/jest-worker/src/__tests__/thread-integration.test.js index 078568604462..30f8d6241d04 100644 --- a/packages/jest-worker/src/__tests__/thread-integration.test.js +++ b/packages/jest-worker/src/__tests__/thread-integration.test.js @@ -66,6 +66,7 @@ describe('Jest Worker Process Integration', () => { it('calls a single method from the worker', async () => { const farm = new Farm('/tmp/baz.js', { + enableWorkerThreads: true, exposedMethods: ['foo', 'bar'], numWorkers: 4, }); @@ -79,6 +80,7 @@ describe('Jest Worker Process Integration', () => { it('distributes sequential calls across child processes', async () => { const farm = new Farm('/tmp/baz.js', { + enableWorkerThreads: true, exposedMethods: ['foo', 'bar'], numWorkers: 4, }); @@ -100,6 +102,7 @@ describe('Jest Worker Process Integration', () => { it('distributes concurrent calls across child processes', async () => { const farm = new Farm('/tmp/baz.js', { + enableWorkerThreads: true, exposedMethods: ['foo', 'bar'], numWorkers: 4, }); @@ -128,6 +131,7 @@ describe('Jest Worker Process Integration', () => { it('sticks parallel calls to children', async () => { const farm = new Farm('/tmp/baz.js', { computeWorkerKey: () => '1234567890abcdef', + enableWorkerThreads: true, exposedMethods: ['foo', 'bar'], numWorkers: 4, }); diff --git a/packages/jest-worker/src/index.js b/packages/jest-worker/src/index.js index 55775e091f23..dcd2c633164f 100644 --- a/packages/jest-worker/src/index.js +++ b/packages/jest-worker/src/index.js @@ -77,7 +77,7 @@ export default class JestWorker { this._options = {...options}; const workerPoolOptions: WorkerPoolOptions = { - disableWorkerThreads: this._options.disableWorkerThreads || false, + enableWorkerThreads: this._options.enableWorkerThreads || false, forkOptions: this._options.forkOptions || {}, maxRetries: this._options.maxRetries || 3, numWorkers: this._options.numWorkers || Math.max(os.cpus().length - 1, 1), diff --git a/packages/jest-worker/src/types.js b/packages/jest-worker/src/types.js index 24fcd7a6f47e..bb12434eca1a 100644 --- a/packages/jest-worker/src/types.js +++ b/packages/jest-worker/src/types.js @@ -70,7 +70,7 @@ export type FarmOptions = { workerPath: string, options?: WorkerPoolOptions, ) => WorkerPoolInterface, - disableWorkerThreads?: boolean, + enableWorkerThreads?: boolean, }; export type WorkerPoolOptions = {| @@ -78,7 +78,7 @@ export type WorkerPoolOptions = {| forkOptions: ForkOptions, maxRetries: number, numWorkers: number, - disableWorkerThreads: boolean, + enableWorkerThreads: boolean, |}; export type WorkerOptions = {|