Make jest-worker use jest-serializer

[mjesun]

This PR implements jest-serializer as a new layer for data transmission between the parent process and all the child ones. Since IPC is a custom Node layer, we re-implemented the data transmission as well to work directly with binary buffers, using an extra file descriptor (number 4).

The Comms class wraps around the fd Socket, and provides an almost 1:1 compatible interface with process, providing a send method and a message event. This way, putting the Comms instance was relatively easy.

Tests cover all Comms class, plus some modification to the original jest-worker tests so that they keep passing.

[thymikee]

How about renaming it Communications? I guess that was the original intent, but shortened.

[cpojer]

Comms seems pretty weird, yeah. How about "Channel"?

[mjesun]

Done! 🙂

[mjesun]

Yeah, now that I read Channel, I prefer it rather than Communications. Changing it again 😅

[thymikee]

@mjesun This is v5+ API, so technically we break Node 4 compat :/

[SimenB]

Should probably use https://github.com/LinusU/buffer-alloc-unsafe or something, then. Would be great to keep around node 4 support (unofficial though it is) until it's EOL in a couple of months

[mjesun]

I'm still investigating how to merge this (if we are going to merge it at all). The internal IPC implementation seems to bypass a bunch of Node layers (e.g. writes directly into the socket via writeUtf8String), and it's quite hard to make a Buffer implementation as fast as theirs.

Performance results using __performance_tests__/test.js showed a 7% regression compared to worker-farm (so approx. 13-14% compared to the current jest-worker version, since that one is 6-7% faster than worker-farm).

The fact that v8.serialize is slower for simple, small objects compared to JSON.stringify does not help either 😞

[mjesun]

Some good news: after #5793, there might still be a chance for this PR to merge. I will rebase and re-measure, see how that goes. This is actually something that would benefit Jest, Metro and potentially other OSS projects.

cc / @rafeca @BYK for the Buffer transport in Metro.

[mjesun]

TL;DR: Using v8.serialize on small payloads has a huge performance cost (from 2 to 3x slower), but it can even make jest-worker slower than worker-farm.

Command node --expose-gc test.js loadTest 10000:

Before the change:

jest-worker: { globalTime: 758, processingTime: 715 }
jest-worker: { globalTime: 851, processingTime: 826 }
jest-worker: { globalTime: 824, processingTime: 800 }

After the change:

jest-worker: { globalTime: 1285, processingTime: 1233 }
jest-worker: { globalTime: 1025, processingTime: 986 }
jest-worker: { globalTime: 978, processingTime: 951 }

Command node --expose-gc test.js empty 100000:

Before the change:

jest-worker: { globalTime: 2501, processingTime: 2463 }
jest-worker: { globalTime: 2442, processingTime: 2413 }
jest-worker: { globalTime: 2502, processingTime: 2469 }

After the change:

jest-worker: { globalTime: 6525, processingTime: 6471 }
jest-worker: { globalTime: 6816, processingTime: 6783 }
jest-worker: { globalTime: 6552, processingTime: 6520 }

[codecov-io]

Codecov Report

Merging #5613 into master will increase coverage by 0.16%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master    #5613      +/-   ##
==========================================
+ Coverage   63.73%   63.89%   +0.16%     
==========================================
  Files         216      217       +1     
  Lines        7938     7969      +31     
  Branches        3        3              
==========================================
+ Hits         5059     5092      +33     
+ Misses       2878     2876       -2     
  Partials        1        1

Impacted Files	Coverage Δ
packages/jest-worker/src/channel.js	100% <100%> (ø)
packages/jest-worker/src/worker.js	98.41% <100%> (+0.13%)	⬆️
packages/jest-worker/src/child.js	100% <100%> (ø)	⬆️
packages/jest-serializer/src/index.js	100% <0%> (+3.92%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update feeb371...784b002. Read the comment docs.

[cpojer]

So what do you wanna do with this PR?

[cpojer]

Ping @mjesun shall we close this for now?

[mjesun]

Yep, let's close it for the moment. A follow up could be to be able to inject the serializer / deserializer; but I'm not going to do that ATM.

[github-actions]

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.