Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add node worker-thread support to jest-worker #7408

Merged
merged 38 commits into from
Dec 5, 2018

Conversation

rickhanlonii
Copy link
Member

Summary

This PR is a continuation of #6676

Node 10 shipped with a "threading API" that uses SharedBuffers to communicate between the main process and its child threads. Being jest-worker a parallelization library, we can take advantage of this API when available. At the same time, we'll decouple our scheduling logic from our communication logic for better re-usability.

Here are some key things:

  • Decouple the scheduling logic (i.e. everything that's not specifically to send and receive jobs)
  • Add the option to use "threads" into the library. If they are present, we'll favor them.

Test plan

Add unit and integration tests, near 100% coverage

I also tried as much as possible to move existing tests rather than writing new ones. In a few places where the refactoring broke the test irrecoverably I wrote new tests

screenshot 2018-11-25 22 29 33

Performance

Without experimental worker

total worker-farm: { wFGT: 3110, wFPT: 2947 }
total jest-worker: { jWGT: 2958, jWPT: 2577 }
---------------------------------------------------------------------------
% improvement over 10000 calls (global time): 4.887459807073955
% improvement over 10000 calls (processing time): 12.555140821174076

With experimental worker

❯ node --experimental-worker --expose-gc test.js empty 10000
total worker-farm: { wFGT: 3084, wFPT: 2913 }
total jest-worker: { jWGT: 2070, jWPT: 1504 }
---------------------------------------------------------------------------
% improvement over 10000 calls (global time): 32.87937743190661
% improvement over 10000 calls (processing time): 48.3693786474425

Copy link
Member

@SimenB SimenB left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Woo! Would be awesome to land before 24, although I guess it could go into a minor

packages/jest-resolve/src/isBuiltinModule.js Outdated Show resolved Hide resolved
packages/jest-worker/README.md Show resolved Hide resolved
packages/jest-worker/src/WorkerPool.js Outdated Show resolved Hide resolved
packages/jest-worker/src/__tests__/Farm.test.js Outdated Show resolved Hide resolved
Copy link
Contributor

@mjesun mjesun left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good to me. I addressed minor stuff (mostly the documentation), but the rest looks good!

packages/jest-worker/README.md Show resolved Hide resolved
packages/jest-worker/src/Farm.js Show resolved Hide resolved
@rickhanlonii rickhanlonii merged commit a169b27 into jestjs:master Dec 5, 2018
@rickhanlonii rickhanlonii deleted the rh-jest-worker-threads branch December 5, 2018 17:16
@SimenB
Copy link
Member

SimenB commented Dec 5, 2018

Woooooo! 🎉

thymikee added a commit to spion/jest that referenced this pull request Dec 18, 2018
* master: (24 commits)
  Add `jest.isolateModules` for scoped module initialization (jestjs#6701)
  Migrate to Babel 7 (jestjs#7016)
  docs: changed "Great Scott!" link (jestjs#7524)
  Use reduce instead of filter+map in dependency_resolver (jestjs#7522)
  Update Configuration.md (jestjs#7455)
  Support dashed args (jestjs#7497)
  Allow % based configuration of max workers (jestjs#7494)
  chore: Standardize filenames: jest-runner pkg (jestjs#7464)
  allow `bail` setting to control when to bail out of a failing test run (jestjs#7335)
  Add issue template labels (jestjs#7470)
  chore: standardize filenames in e2e/babel-plugin-jest-hoist (jestjs#7467)
  Add node worker-thread support to jest-worker (jestjs#7408)
  Add `testPathIgnorePatterns` to CLI documentation (jestjs#7440)
  pretty-format: Omit non-enumerable symbol properties (jestjs#7448)
  Add Jest Architecture overview to docs. (jestjs#7449)
  chore: run appveyor tests on node 10
  chore: fix failures e2e test for node 8 (jestjs#7446)
  chore: update docusaurus to v1.6.0 (jestjs#7445)
  Enhancement/expect-to-be-close-to-with-infinity (jestjs#7444)
  Update CHANGELOG formatting (jestjs#7429)
  ...
willsmythe pushed a commit to willsmythe/jest that referenced this pull request Dec 20, 2018
* Restructure workers (create a base class and inherit from it)

Clean up types, create missing interfaes and make Flow and Jest happy

* Move child.js to workers folder

* Restructure base classes and make a working POC

* Remove BaseWorker

* Remove MessageChannel and cleanup super() calls

* Use worker threads implementation if possible

* Restructure queues

* Support experimental modules in jest-resolver

* Rename child.js to processChild.js

* Remove private properties from WorkerPoolInterface

* Move common line outside of if-else

* Unify interface (use workerId) and remove recursion

* Remove opt-out option for worker_threads in node 10.5+

* Alphabetical import sorting

* Unlock worker after onEnd

* Cache queue head in the getNextJob loop

* Elegant while loop

* Remove redundand .binds

* Clean up interfaces and responsibilites

* Update jest-worker

* Add changelog and update jest-worker readme

* Fix lint lol

* Fixes from review

* Update Changelog

* rm function

* rm []

* Make imports alphabetical 🤮

* Go back to any

* Fix lint

* \n

* Fix formatting

* Add docs

* Revert canUseWorkerThreads

* Fix lint

* Fix pathing on windows
captain-yossarian pushed a commit to captain-yossarian/jest that referenced this pull request Jul 18, 2019
* Restructure workers (create a base class and inherit from it)

Clean up types, create missing interfaes and make Flow and Jest happy

* Move child.js to workers folder

* Restructure base classes and make a working POC

* Remove BaseWorker

* Remove MessageChannel and cleanup super() calls

* Use worker threads implementation if possible

* Restructure queues

* Support experimental modules in jest-resolver

* Rename child.js to processChild.js

* Remove private properties from WorkerPoolInterface

* Move common line outside of if-else

* Unify interface (use workerId) and remove recursion

* Remove opt-out option for worker_threads in node 10.5+

* Alphabetical import sorting

* Unlock worker after onEnd

* Cache queue head in the getNextJob loop

* Elegant while loop

* Remove redundand .binds

* Clean up interfaces and responsibilites

* Update jest-worker

* Add changelog and update jest-worker readme

* Fix lint lol

* Fixes from review

* Update Changelog

* rm function

* rm []

* Make imports alphabetical 🤮

* Go back to any

* Fix lint

* \n

* Fix formatting

* Add docs

* Revert canUseWorkerThreads

* Fix lint

* Fix pathing on windows
@github-actions
Copy link

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.
Please note this issue tracker is not a help forum. We recommend using StackOverflow or our discord channel for questions.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 12, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants