andywer / threads.js

๐Ÿงต Make web workers & worker threads as simple as a function call.
https://threads.js.org/
MIT License
3.04k stars 161 forks source link

[Question] How to use a class method as spawn method #57

Closed WoodySlum closed 6 years ago

WoodySlum commented 7 years ago

Hi,

I would like to thread a class method. Is it possible ? Here is my non working code :

const threads = require("threads");

class A {
    constructor() {
        let thread = threads.spawn(this.asyncProcess);
        thread.send();
    }

    asyncProcess(input, done) {
        // Do stuff
        done();
    }
}

let a = new A();

And the following error appears :

evalmachine.<anonymous>:1
module.exports = asyncProcess(input, done) {
                                           ^
SyntaxError: Unexpected token {
    at Object.exports.runInNewContext (vm.js:71:16)
    at runAsSandboxedModule (/Users/smizrahi/Documents/hautomation/core-node/node_modules/threads/lib/worker.node/slave.js:35:6)
    at process.<anonymous> (/Users/smizrahi/Documents/hautomation/core-node/node_modules/threads/lib/worker.node/slave.js:72:22)
    at emitTwo (events.js:106:13)
    at process.emit (events.js:191:7)
    at process.nextTick (internal/child_process.js:719:12)
    at _combinedTickCallback (internal/process/next_tick.js:67:7)
    at process._tickCallback (internal/process/next_tick.js:98:9)

Is it possible to use a class method of an instantiated object in spawn ?

Thanks

andywer commented 7 years ago

Hey @WoodySlum

Yeah, this bug is a fun one... See, the method you pass to spawn() is stringified (const stringified = method.toString()) and then run as a sandboxed module (runInSandbox('module.exports = ' + stringified))

Now this works for plain old functions and arrow functions. But when you pass an ES6 class method or object method it seems to be stringified as such and then cannot be used outside of a class/function without yielding a syntax error ๐Ÿ™ˆ๐Ÿ™ƒ

So this is why. Let's see if we can find an easy solution for this queer issue ๐Ÿ˜…

andywer commented 7 years ago

On the other hand... It might not be the best idea to feed spawn() with a class method anyway, since in its code you cannot access any of the identifiers declared outside the method (and no this).

Writing it as a class method kind-of invites you to use this which won't work anyway.

WoodySlum commented 7 years ago

Hey, thanks for the problem description. In fact I would like to run a code part in a threaded way. My use case : I have a service class. This class is extended by services. In parent constructor, you can specify which kind of service you're running : classic (nothing particular, in the subclass you implement the start method and do stuff), external (run an external application, in any kind of language), and threaded (code provided in run subclass method is in js language).

The fact that threaded code is contained in a class doesn't matter ; having an access to this would be great but I understand that IPC doesn't allow this kind of access.

As I understand the problem, it should work with a static method of a class, no ?

   const threads = require("threads");

class A {
    constructor() {
        let thread = threads.spawn(A.asyncProcess);
        thread.send();
    }

    static asyncProcess(input, done) {
        // Do stuff
        done();
    }
}

let a = new A();

However this doesn't work also.

Other question, can I pass the object instance through input (e.g. : thread.send({instance : this});) ?

andywer commented 7 years ago

Nahh, that's still the same problem... What you could do as a quick fix is writing the static method as

A.asyncProcess = (input, done) => { ... }

This way .toString() will serialize it as an arrow function and it will work.

You could try to send the instance reference via .send(), but I don't know if that is a good idea. The instance that arrives on the other side will be a clone of the original, not the original itself.

andywer commented 7 years ago

Maybe the classical OOP way is not your best bet here. In the last years I came to the personal conclusion that class-based OOP esp. in JS has always been a bit hacky and will probably continue to feel quirky.

But fortunately the languange also offers neat ways to go a rather functional approach. That might cause less headaches in the long run ๐Ÿ˜‰ I wrote the lib before I came to this conclusion, so... ^^ But I plan to make it a bit more functional in version 1.0. Can't promise when that will happen, though.

WoodySlum commented 7 years ago

You could try to send the instance reference via .send(), but I don't know if that is a good idea. The instance that arrives on the other side will be a clone of the original, not the original itself.

Good to know

I usually use Objective-C as main langage, and GCD provides powerful object communication between threads.

I have to do tricky things to accomplish what I want to do :)

I can confirm that sending this through send is not working as well.

WoodySlum commented 7 years ago

Hey, come back to the issue. I found a way to make this working, even if it's a little bit tricky :) The goal is to modify the function prototype. ie instead of run(input, done) {done();} we want (input, done) => {done();}. Here is how I do this :

    const regex = /(\()(.*)(\))([^]{0,1})({)([^]+)(\})/mg;
    let regexResults = regex.exec(this.run.toString());
    if (regexResults.length === 8) {
        let prototype = "(" + regexResults[2] + ") => {" + regexResults[6] + "}";

        let thread  = threads.spawn(eval(prototype));
        thread.send();
    }
andywer commented 7 years ago

Sorry, I had a lot of other stuff going on.

Yeah, I thought about something similar. Might even be worth creating a small stand-alone package for that purpose.

The only problem I see here is that it's so damn hard to read that regex and it's probably impossible to create an accurate solution using regexs. Finding a regex for the function arguments is nowadays de facto impossible, consider foo (param1 = null, { y: z } = {}) { /* ... */ }

Not sure how to solve this issue in a reliable way :-S

andywer commented 6 years ago

I don't see a way to fix this at the moment. Closing for now, but feel free to re-open if you have an idea or the issue still relevant.