Node WorkerChannel native addon #1394

jmillan · 2024-05-09T16:09:09Z

Substitute spawning a process with mediasoup for running mediasoup in a thread created by this addon.

The communication is now done via message passing rather than using unixsocket, avoiding the kernel networking stack for each and every message passed from Node to C++ and viceversa.

Performance gains are enhanced by approximately 8 times. Tested by sending a request from Node to C++ and waiting for the response N times in a loop.

Caveats:

Since we don't use a separate process, a crash in mediasoup would crash Node, meaning it must be handled by node process signal handling.
- At the same time, the coredumps will be named something-node-something
- In order to debug coredumps the worker-channel.node binary can be used along with the coredump file and gdb/lldb

TODO:

Document removed 'died' and 'subprocessclose' Worker.ts events.
Document removed 'died()' and 'subprocessClosed()' Worker.ts methods.
Missing all the prebuilt binary upload/download stuff.

Substitute spawning a process with mediasoup for running mediasoup in a thread created by this addon. The communication is now done via message passing rather than using unixsocket, avoiding the kernel networking stack for each and every message passed from Node to C++ and viceversa. Performance gains are enhanced by approximately 8 times. Tested by sending a request from Node to C++ and waiting for the response N times in a loop.

.gitignore

ibc · 2024-05-09T16:25:29Z

.prettierignore and .eslintignore should include /node/workerChannel/lib.

node/src/Worker.ts

node/workerChannel/src/workerChannel.hpp

npm-scripts.mjs

package.json

ibc · 2024-05-10T13:14:58Z

Please merge v3 and check prettier and eslint path arrays in npm-scripts.mjs

assign JS instance of the addon to 'undefined' when closed, to indicate the Node GC that it can be freed. Otherwise the tests won't finish due to the correponsing handles being opened.

Lower npm versions than 10.3.0 fail building the addon. Error: No module named 'distutils'

jmillan · 2024-10-15T08:37:24Z

Update:

This PR is almost ready to be shipped if we want to.

The are two main concerns already defined in the PR description:

Since we don't use a separate process for the application and the worker, a crash in mediasoup would crash Node, meaning it must be handled by node process signal handling. This is true for Rust too.
- The coredumps will be named something-node-something
- In order to debug coredumps the worker-channel.node binary can be used along with the coredump file and gdb/lldb
Since the worker is now used in multi-thread fashion, the very same concerns apply to Node as to Rust, such us Multi-thread bug when using usrsctp in N Worker threads in Rust #1352

Having said this, do we still want to go in this direction?. As indicated in the description:

Performance gains are enhanced by approximately 8 times. Tested by sending a request from Node to C++ and waiting for the response N times in a loop.

But of course it comes with the mentioned caveats too.

What do you think @ibc, @nazar-pc?

nazar-pc · 2024-10-15T08:43:11Z

If there use cases where the performance gap is that large, I think it is worth it. The proper solution to crashes is largely to write code that doesn't crash rather than running it in a separate process.

ibc · 2024-10-15T09:40:05Z

I think that keeping the current design just to keep the "benefit" of not crashing the node process if the worker fails is not a good argument so I am ok but:

How does this change affect prebuilt worker binaries? Now they should not be "binaries" but "libraries" so more changes are needed specially in the naming of those assets and the function in npm-script that fetches them.

jmillan · 2024-10-15T14:09:51Z

How does this change affect prebuilt worker binaries? Now they should not be "binaries" but "libraries" so more changes are needed specially in the naming of those assets and the function in npm-script that fetches them.

Yes, there is a TODO for this.

👍,

jmillan added 2 commits May 9, 2024 18:07

remove worker-channel from dependencies in package.json

1e59102