Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Getting a failure under node #23

Closed
csterritt opened this issue Feb 18, 2017 · 13 comments
Closed

Getting a failure under node #23

csterritt opened this issue Feb 18, 2017 · 13 comments
Labels

Comments

@csterritt
Copy link

Hello,

Trying the fourth test function under node.

However, I'm compiling from ES6, so I've switched the functions to arrow functions:

import * as hamsters from 'hamsters.js'

//4 threads and let's aggregate our individual thread results into one final output
const pfunc = () => {
  const params = { 'array': [0, 1, 2, 3, 4, 5, 6, 7, 8, 9] }
  hamsters.run(params, () => {
      const arr = params.array
      arr.forEach((item) => {
        rtn.data.push((item * 120) / 10)
      })
    },
    (output) => output,
    4, true)
}
const res = pfunc()
console.log("hamster res:", res)

This fails with the following error:

hamster res: undefined
/Volumes/Second/Chris/myprogram/node_modules/hamsters.js/src/es5/hamsters.js:653
      self.rtn = {
      ^

ReferenceError: self is not defined
    at null._onTimeout (/Volumes/Second/Chris/myprogram/node_modules/hamsters.js/src/es5/hamsters.js:653:7)
    at Timer.listOnTimeout (timers.js:92:15)

real    0m0.082s
user    0m0.064s
sys     0m0.014s

This is using node v4.3.2 (for compatibility reasons).

Any clues? Thanks!

@austinksmith
Copy link
Owner

Interesting, I'll have to take a look at it here soon. The issue appears to be that Node doesn't like using self as a shortcut to the global scope, typically self.blah would be the same as global.blah or window.blah so node is doing something weird.

I'd suggest trying to use a 3rd party web worker implementation as well, something like this might help you get started before I can address this as its an issue with the legacy fallback mode that doesn't actually use threads. https://www.npmjs.com/package/webworker-threads

Node doesn't ship with a native web worker implementation out of the box so the library is defaulting to the fallback mode in your case. If you make use of a third party implementation be sure to import that BEFORE you import hamsters.

@csterritt
Copy link
Author

csterritt commented Feb 19, 2017 via email

@austinksmith
Copy link
Owner

@csterritt Try using the latest from dev branch now, just pushed this commit which should fix the issue. I haven't tested it yet but it looks like it'll work.

17b18ab

@austinksmith
Copy link
Owner

@csterritt Please let me know if that resolved your issue if you get the chance, thanks

@csterritt
Copy link
Author

csterritt commented Feb 24, 2017 via email

@austinksmith
Copy link
Owner

@csterritt No problem, I'm going to go ahead and close this issue out. I went ahead and published v3.9.8 which includes this fix. If you have anymore issues feel free to open another ticket, thanks!

https://github.com/austinksmith/Hamsters.js/releases/tag/v3.9.8

@csterritt
Copy link
Author

Sorry to be 'that guy', but I'm getting a new problem with v3.9.8. It fails the same way under node v4.3.2 and v7.7.0:

/Volumes/Second/Chris/myprogram/node_modules/hamsters.js/src/es5/hamsters.js:312
          hamsters.wheel.hamsters.push(new Worker('src/common/wheel.min.js'));
                                           ^

ReferenceError: Worker is not defined
    at spawnHamsters (/Volumes/Second/Chris/myprogram/node_modules/hamsters.js/src/es5/hamsters.js:312:44)
    at /Volumes/Second/Chris/myprogram/node_modules/hamsters.js/src/es5/hamsters.js:963:7
    at setupHamstersEnvironment (/Volumes/Second/Chris/myprogram/node_modules/hamsters.js/src/es5/hamsters.js:112:5)
    at /Volumes/Second/Chris/myprogram/node_modules/hamsters.js/src/es5/hamsters.js:907:3
    at Object.<anonymous> (/Volumes/Second/Chris/myprogram/node_modules/hamsters.js/src/es5/hamsters.js:966:3)
    at Module._compile (module.js:571:32)
    at Object.Module._extensions..js (module.js:580:10)
    at Module.load (module.js:488:32)
    at tryModuleLoad (module.js:447:12)
    at Function.Module._load (module.js:439:3)

@austinksmith
Copy link
Owner

No problem, that's why I asked you to follow up haha. So it looks like I mixed up the order during the environment setup so currently the v3.9.8 will only work with a 3rd party web worker implementation (which is what you'll want for real threading). I'll push a fix today that will restore the legacy fallback mode which will allow you to write your logic and take advantage of the async nature of the library but will not give you the performance benefits you're probably looking for with extra threads.

@austinksmith
Copy link
Owner

austinksmith commented Mar 2, 2017

As promised, this should resolve your issues. Runkit on npm hasn't updated yet but i think this will work.

https://github.com/austinksmith/Hamsters.js/releases/tag/v4.0.0

@csterritt
Copy link
Author

Alas:

/Volumes/Second/Chris/myprogram/node_modules/hamsters.js/src/es5/hamsters.js:102
      setupNodeSupport();
      ^

ReferenceError: setupNodeSupport is not defined
    at setupHamstersEnvironment (/Volumes/Second/Chris/myprogram/node_modules/hamsters.js/src/es5/hamsters.js:102:7)
    at /Volumes/Second/Chris/myprogram/node_modules/hamsters.js/src/es5/hamsters.js:909:3
    at Object.<anonymous> (/Volumes/Second/Chris/myprogram/node_modules/hamsters.js/src/es5/hamsters.js:968:3)
    at Module._compile (module.js:409:26)
    at Object.Module._extensions..js (module.js:416:10)
    at Module.load (module.js:343:32)
    at Function.Module._load (module.js:300:12)
    at Module.require (module.js:353:17)
    at require (internal/module.js:12:17)
    at Object.<anonymous> (/Volumes/Second/Chris/myprogram/build/try-parallel.js:3:17)

Also, which 3rd party web worker implementation do you recommend?

@austinksmith austinksmith reopened this Mar 7, 2017
@austinksmith
Copy link
Owner

Hmm, so I actually published version 4.0 which resolves the issue you're talking about already. I see on the npmjs.com page that it shows version 4.0 is what you should see. This however looks like version 3.9.9.

@austinksmith
Copy link
Owner

austinksmith commented Mar 7, 2017

I put in a support ticket with them to get the website to update the package, sorry about that. Regarding web worker implementations, this one looks fairly good. I will warn however that their example documentation isn't what you probably want here. You'll probably need to do something like this.

require('webworker-threads');
require('hamsters.js');

Then you can just use Hamsters like you usually would. If you get an error that Worker doesn't exist..then do what their documentation says and assign a global variable to assign Worker.

global.Worker = require('webworker-threads').Worker;
require('hamsters.js');

@csterritt
Copy link
Author

Hey Austin - glad to report that 4.0 works fine! I haven't gotten it working with webworker-thread yet, but I'm sure that will come.

Thanks again for your help!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants