Add support for running inside a node 10.5 worker_thread

[rpetrich]

Properly tracks which event loop callbacks should be dispatched on so that node doesn't busy loop on the main thread if node-sqlite3 is called from a worker.

[springmeyer]

Very cool, thanks for putting up this PR. So, this enables node-sqlite3 to be used from a native node.js thread started via the new worker_threads module in node >= v10?

I'm interested to hear more about what usecases you'll use this for. Also, as someone on the bleeding edge of worker_threads, curious if you'd be interested in working on comparing perf like proposed at mapbox/node-cpp-skel#143. That ticket proposing comparing perf between pure JS and C++, but your PR here gets me thinking that it would also be beneficial to have an easy to run benchmark that compares perf of using something like node-sqlite3 from the main loop vs from a worker. Interest?

[rpetrich]

Yes. This PR makes it safe to use in a multithreaded context using node's new experimental worker threads feature. It does not introduce new APIs to enable sharing of connections in serialized mode between worker threads or to reduce overhead via sqlite's shared cache feature, but perhaps this could be explored in the future.

My basis for comparison is an existing worker implementation on top of child_process.fork with messages sent over their IPC channels. Behind the scenes node uses JSON.stringify/JSON.parse for this, so the bar for performance isn't exactly high 🙃. Within these workers lives userland code operating in a simulated browser environment with additional access to node APIs including node-sqlite3. Haven't assessed performance yet, but I expect worker threads to benchmark well for this use case (when I get there). My expectation is that node-sqlite3 performance will be the same regardless of whether used from a subprocess or a worker threads, but getting the data there will have less overhead when using worker threads.

If you are aware of benchmarks showing the overhead of JS worker threads vs C++ native modules with background threads, that would be very interesting though not directly relevant to the project that led to this PR.

[springmeyer]

My expectation is that node-sqlite3 performance will be the same regardless of whether used from a subprocess or a worker threads, but getting the data there will have less overhead when using worker threads.

I'll be interested to know if that expectation is met. One thing that could negatively impact the performance, when run inside a worker_thread, is if any of the mutex locks inside libsqlite3 interact with the locking that (I presume) is needed to keep the worker_thread implementation safe. Hopefully this will not happen, but I worry it could.

If you are aware of benchmarks showing the overhead of JS worker threads vs C++ native modules with background threads, that would be very interesting though not directly relevant to the project that led to this PR.

Got it, none that I'm aware of which is why I created mapbox/node-cpp-skel#143

[springmeyer]

Noticing that the only failure for this PR is unrelated (coverage build failed). So, merging now. This will likely land in v4.0.2 (when I cut it, but I don't have an ETA on that).

[rpetrich]

Unfortunately, I didn't do my due diligence and actually test with multiple workers. When I did so, it uncovered some additional adjustments that needed to be made. It also uncovered a bug in the way node's native module support interacts with thread_workers (see: nodejs/node#21517). Once this bug get's resolved, I'll follow up with an additional PR.

In the meantime, the performance script is available here: https://gist.github.com/rpetrich/2e49540a26e1e6e0dc2c57252b8a562c. It will require rpetrich/node-sqlite3@020a0eb and rpetrich/node@ffdc0c0

[springmeyer]

Thanks for the followup. I'll revert then for now until this has a chance to settle out.

[kewde]

@rpetrich

Could you inform me if the upstream bug has been resolved or not?