Usage with Worker Threads #237
Comments
|
The ability to use native addons within worker threads is very new (Node 11.11.0 and up). Node.js also requires the addon ( |
|
Hijacking this to muse about how it would work with threads. I really want to use this library, but I would have to run it in a different process to have a responsive webserver at all times, and worker threads look like a nice middle ground, even giving parallelization for read-only workloads. Unfortunately, the communication with the worker needs to undergo a serialization/deserialization step. The most performant way to communicate with worker threads seems to be a SharedArrayBuffer. I wonder what would be a nice way to serialize sqlite responses directly without having to go through the Node abstractions, and then do the deserialization in the main thread from the SharedArrayBuffer. |
|
I encountered this problem too, here is cleaner code to reproduce: const { Worker, isMainThread } = require('worker_threads');
if (isMainThread) {
require('better-sqlite3');
new Worker(__filename);
} else {
require('better-sqlite3'); // Error: Module did not self-register
} |
|
Hi @JoshuaWise. Thank you for the awesome library and giving this solution. But the polling process is eating my CPU up when using your solution. One core is pretty much being dedicated to it. Even in idle since It's constantly poking the queue. Any alternative? I'm looking for a solution to be used directly in electron renderer which doesn't support worker threads. Web worker is a mess too. I need to support very frequent queries done through user input without blocking the UI even for a few milliseconds. |
|
@m4heshd from what I can find, worker_threads should work in electron in the main process: electron/electron#18540 (comment) What do you mean with queue polling? |
|
Hi @wmertens. Thanks for the input. It does work with the main process but unfortunately electron seems not stable enough to handle that. The process always ends up with an access violation error (code
It constantly checks the queue for new tasks which takes a toll on the CPU. You can clearly see It if you take a closer look at all the cores. One core is being utilized 100% for this task. Might create different results on different libuv versions and CPUs. Although I thought of a solution while writing this but still doesn't matter since Electron plays angry with it. It was so much effort get webpack to properly bundle workers but Electron web worker doesn't like to load external node modules even though the documentation clearly says so. One disappointment followed by another. It's an endless journey of jumping through hoops. I love Electron with all my heart and better-sqlite3 is the sweet cherry on top. Done some really impressive projects in the past with Electron. But the amount of time wasted on these tiny little unstable stuff is infuriating coming from other statically written more stable languages and all the accompanied technologies. (Really really sorry about the rant in advance. Just have to pour out my frustration somewhere.) |
|
@m4heshd Hmm, that sucks. From my perspective, I would
Too bad about your code, but take solace from the fact that we've all been there :) |
@wmertens haha thank you for relating. I'll definitely get to those tasks.
You couldn't be more right. Even their documentation says that. Mostly upon evidence of how unstable these modules are, I should be stupid to go down this road of ✨ threads in Electron ✨. And really glad you said it because I've come to a conclusion now. This whole day I was thinking, screw this sync async bull. I've always appreciated synchronous code. It's much faster, easy to control and super reliable. I'll let the GUI hang for a few milliseconds. So what. My clients would appreciate data stability over some crisp smooth GUI like it always has been for them (with Java + threads). It's not a competitive FPS game to have that much sharp responsiveness 🤣. People are going super overboard with this async stuff (It's a good thing but not always). Meanwhile, do let me know if you stumble upon any alternatives to overcome this. |
|
Do note that:
Not sure how your app is structured, but IMHO the bridging from main to render shouldn't be a big issue? Especially if you have an abstraction layer around the DB that converts chatty SQL to compact internal representation before bridging? It does sound like Electron worker threads aren't ready for it though, I'd really chase down those problems too... But of course, it's always a matter of picking your battles. |
If you're hitting 100% CPU usage and you're referring to the polling that's being used in this example, then there is almost certainly something wrong with your runtime's implementation of As for the using |
|
@JoshuaWise so in the example, the queue polling could be replaced by immediately assigning a worker on receiving the query, no? I don't understand what the queue is for. |
|
Each worker thread can only execute one query at a time. If all of the workers are busy, you need to put the query into a queue. By having a shared queue, you ensure that the work is distributed evenly among the workers. You could, instead, give each worker its own private queue, and rotate between them as new work comes in. However, the problem with that approach is that some queries could take much longer than others, which means you could get unlucky and a single worker could get overloaded with large queries while the other workers wait around doing nothing. Using a shared queue prevents this from ever happening. If you simply send new messages to the workers without using any queues, it's equivalent to using a private queue for each worker, because each worker has a built-in private queue for receiving messages. |
@wmertens I know. I didn't misunderstand it. I tested pretty much everything for hours but all useless since Electron can't play nice with any good approach. So sad. @JoshuaWise Yes I was referring to the example since the first post. I know, everything's good. Only issue is that Electron failing to support any of this. While I was asking for an alternative on the first comment, I haven't gotten to the Electron issues yet other than the polling problem. So no complains from me about
You posted this while I was writing the same explanation 😁 although I mentioned about the storing and calling of each promise. That was a really good approach. I actually have a solution to the polling issue. Lemme try it quickly.
You don't have to. It's pretty straight forward. Thank you for everything that you've already done. More than enough. |
|
Ok @JoshuaWise here you go. I had this halfway done yesterday but gave up because of the Electron incompatibility. Works for me perfectly. Do test and let me know. I'll create a PR for this right away. Here's the CPU usage. Before:
After:
Code:const { Worker } = require('worker_threads');
const os = require('os');
/*
Export a function that queues pending work.
*/
const queue = [];
exports.asyncQuery = (sql, ...parameters) => {
return new Promise((resolve, reject) => {
queue.push({
resolve,
reject,
message: { sql, parameters },
});
callWorkers();
});
};
/*
Call poll() in every worker upon new asyncQuery request.
*/
let workers = [];
function callWorkers() {
workers.forEach((worker) => {
worker.poll();
})
}
/*
Spawn workers that try to drain the queue.
*/
os.cpus().forEach(function spawn() {
const worker = new Worker('./worker.js');
const threadId = worker.threadId;
let job = null; // Current item from the queue
let error = null; // Error that caused the worker to crash
function poll() {
if (!job && queue.length) {
// If there's a job in the queue, send it to the worker
job = queue.shift();
worker.postMessage(job.message);
}
}
worker
.on('online', poll)
.on('message', (result) => {
job.resolve(result);
job = null;
poll(); // Check if there's more work to do
})
.on('error', (err) => {
console.error(err);
error = err;
})
.on('exit', (code) => {
workers = workers.filter(w => w.threadId !== threadId);
if (job) {
job.reject(error || new Error('worker died'));
}
if (code !== 0) {
console.error(`worker exited with code ${code}`);
spawn(); // Worker died, so spawn a new one
}
});
workers.push({threadId, poll});
});The issue was |
|
I would call the callWorkers function from the asyncQuery after queuing. I don't understand why the polling is needed. Don't the worker messages get queued? This way, either a worker is available at the moment of queuing a job, or they will check when they complete. No polling. |
@wmertens I already do that in the above example. You might've seen an earlier edit where i accidentally left out some stuff while merging my code with the example from repo.
Because when all the workers comes to an idle state and the queue was empty at that time, they won't know when the new jobs arrive anymore because there won't be any events fired from worker's scope. Any type of queueing needs some kind of polling. This way it's much efficient and faster than constantly checking for updates in a loop. This code needs some more adjustments for destroyed workers. I'm testing those updates right now. Will update the code soon. UPDATE: Just updated it |
|
Ah yes, now it's good. With no polling I meant no setimmediate, and that is now gone. I expect this to eat no CPU at all? |
|
Yes it is. As you can see from above stats, CPU is chilling with all four threads idling. Only if this worked with Electron properly. 😒 UPDATE: My issue has been fixed in Electron 12 since it's using Node v14 ABI. All good now. |
|
I have tried this solution on the back end of an electron application and it works well whilst in debug mode however when I build the application into a release require('better-sqlite3') is only found if the better-sqlite3 node_modules is located within the project. Do you know how this could be built in and be used from the worker thread like it is in does in the main thread? |
|
@JonathanWoodward You can't have your worker code packed into the build {
asarUnpack: [
"**/node_modules/better-sqlite3/**/*",
"**/node_modules/bindings/**/*",
"**/node_modules/file-uri-to-path/**/*",
// also include your worker files here....
]
}Then you might run into another issue where Electron's own const workerScript = process.env.NODE_ENV === 'production' ?
path.join(process.resourcesPath, 'app.asar.unpacked', 'my-worker.js') :
path.join(__dirname, 'my-worker.js'); |
|
Hi @m4heshd , thank you for your reply. Would this impose a security vulnerability where the packed source could be modified leading to some form of injection? |
It most definitely does. That's something very difficult to avoid when it comes to Electron even if the app is fully packaged into the npm i -g asar
asar extract app.asar extractedBest bet here for you would be either properly obfuscating your production code (fairly effortless) or using V8 snapshots (which can be very difficult). |
|
@m4heshd are there any known ways to obfuscation in Electron.js then checksum each of the files for tamper detection? |
|
@JonathanWoodward My recommendation is |
If this is an issue of bindings not being resolved, you can solve it by using the new |
@m4heshd I may be missing your point with this but should this allow me in my-worker.js just require('better-sqlite3') normally or should I be using the workerScript path to the modules? I've tried a few things and had no success, it's probably my stupidity. |
This is solid but you still would be required to check if the application's in |
Yes it would. For an example, this is the worker source file (unedited) of a project I'm currently shipping. const {parentPort, workerData} = require('worker_threads');
const db = require('better-sqlite3-multiple-ciphers')(workerData.__dbPath, {
fileMustExist: true,
verbose: process.env.VUELECTRO_ENV === 'build' ? null : require(workerData.loggerPath).logDBVerbose
});
if (workerData.isDBEncrypted) db.pragma(`key='${workerData.key}'`);
parentPort.on('message', ({type, sql, parameters}) => {
if (type === 'terminate') {
db.close();
process.exit();
} else {
let result = {
error: null,
data: null
}
try {
result.data = db.prepare(sql).all(...parameters);
} catch (error) {
result.error = error.toString();
}
parentPort.postMessage(result);
}
});This works perfectly fine on all platforms, in all scenarios. One thing I forgot to mention in the previous response, you cannot use the |
|
@m4heshd is it possible to pkg better-sqlite into a worker in a node service separate from electron? `
@GCN0185 MINGW64 /c/git/SquirrelViewData (master) @GCN0185 MINGW64 /c/git/SquirrelViewData (master) SyntaxError: Unexpected identifier |
|
@JonathanWoodward I think you should share a minimal repro of what you're trying to achieve. That would help a lot on debugging this. |
|
@JonathanWoodward Check my PR on your repo. I expected a repro of your Electron approach. I sense that you've given up on trying to fix the issue on Electron's side and trying this hacky method. But I highly suggest not to go this way. You're gonna lose a lot of control doing this and not to mention you might end up corrupting your DBs. What I recommend is to share an absolutely minimal repro of your Electron project and then I'll hopefully be able to assist you with debugging this in a much cleaner way. |
|
@m4heshd No, I have the electron approach working. I am trying to "compile" the code and produce a Windows ".exe" so the source code is not exposed. The package you produced pkg-worker-better-sqlite-1.0.0.tgz is not the same as index-win.exe. |
|
@m4heshd this is the compiled version with your changes.
|
|
@JonathanWoodward I'm pretty sure you didn't run the changes from that PR. If you did, it should output
I even moved it completely away from the project directory. |
@m4heshd apologies I missed a line out. What version of node are you using? I now get the following error which I can't seem to resolve: /c/git/pkg-worker-better-sqlite/dist (main) |
I use Node 14. That's why I mentioned the following on your PR note.
Also if this conversation moves further, it should be moved to your repo's PR. |
|
Very nice, thanks for the help. $ ./pkg-worker-better-sqlite.exe |
Hello,
I use a node.js app in order to benchmark an application server. I use for that a queue combined with multiple threads managed by Node Worker Threads.
better-sqlite3 is excellent for parallel access, so I wanted to use it in my app. The problem is when I include the module in my worker, an error
Module did not self-register.appear.Environments
better-sqlite: v5.4.0
Tested with node:
System: Windows 10 1709
Steps to reproduce
Related sources
I have already trace some related articles:
Regards,
The text was updated successfully, but these errors were encountered: