charto / nbind

:sparkles: Magical headers that make your C++ library accessible from JavaScript :rocket:
MIT License
1.98k stars 119 forks source link

Persistent callbacks macros #51

Closed kenOfYugen closed 7 years ago

kenOfYugen commented 7 years ago

I have been having trouble implementing asynchronous callbacks. After checking out past issues, such as #15 and #23, I am still not confident enough.

After studying libui-node's source, I found out that it now uses preprocessor definitions for DEFINE_EVENT and IMPLEMENT_EVENT.

Are there any plans for nbind to have such an abstraction? Or maybe get a minimal working example in the examples as a solid reference?

arcanis commented 7 years ago

Ideally, it would be nice to be able to avoid working with nbind::cbCallback altogether and just use std::function, but I don't think it's ready atm (I think I saw an issue about this: #38). In the meantime, cbCallback is a good enough replacement, and doesn't require much work to get to work (the one catch I've found is #52, which requires you to use pointers to store your callbacks).

You can find an example here: https://github.com/arcanis/text-layout/blob/d1ec26a5193c16256165feda3420bad95c0e9cd1/sources/TextLayout.hh#L35-L45 https://github.com/arcanis/text-layout/blob/d1ec26a5193c16256165feda3420bad95c0e9cd1/sources/TextLayout.cc#L84-L96 https://github.com/arcanis/text-layout/blob/d1ec26a5193c16256165feda3420bad95c0e9cd1/sources/TextLayout.cc#L477-L480

kenOfYugen commented 7 years ago

@arcanis thanks for the response, will check your code!


Edit: that's how I've been doing it myself, storing the *cb and doing a cb->call. Maybe it's working while I think it doesn't.. will investigate further and update.

jjrv commented 7 years ago

Using only normal pointers for persistent callbacks isn't really a good idea, since the callback may get garbage collected before using it through the pointer. Also making a copy using new is bad style nowadays. std::make_shared and std::make_unique should be fine.

kenOfYugen commented 7 years ago

I am confused. How is it possible for code to be asynchronous without leveraging nan's AsyncWorker, or something else equivalent?

For example I compared the behavior of setTimeout and nbind.init:

var nbind = require('nbind');

console.time('test timeout outer');
console.time('test timeout inner');
setTimeout((function() {
  console.log('timeout fired');
  console.timeEnd('test timeout inner');
}), 1000);
console.timeEnd('test timeout outer');

console.time('test nbind outer');
console.time('test nbind inner');
nbind.init(function(err, binding) {
  console.log('nbind native module loaded');
  var lib = binding.lib;
  // Use the library.
  console.timeEnd('test nbind inner');
});
console.timeEnd('test nbind outer');

Outputs:

test timeout outer: 6.589ms
nbind native module loaded
test nbind inner: 28.489ms
test nbind outer: 29.176ms
timeout fired
test timeout inner: 1008.726ms

Shouldn't test nbind outer be firing before test nbind inner and log less ms? nbind.init appears to be blocking. But the callback seems to fire okish. Are my assumptions wrong?

jjrv commented 7 years ago

nbind.init currently always calls the callback synchronously on Node.js and asynchronously inside web browsers... The point is to allow using CommonJS require() to load the addon and immediately start using it. However on browsers the asm.js code may decide to load a binary with initial heap contents, forcing it to work asynchronously.

Currently nbind doesn't really work asynchronously, because you cannot do the equivalent of setTimeout from the C++ side. Basically this becomes a problem if you'd want to create a thread in C++ code, since there's no API for another thread to send a message to the JavaScript thread.

kenOfYugen commented 7 years ago

Ok, now it makes sense why I was thinking my code wasn't working properly. I will close this, thanks for the swift clarification.