Skip to content
This repository was archived by the owner on Nov 22, 2022. It is now read-only.

Switch Raspi-IO to using node-ffi #11

Closed
nebrius opened this issue Jun 1, 2015 · 18 comments
Closed

Switch Raspi-IO to using node-ffi #11

nebrius opened this issue Jun 1, 2015 · 18 comments

Comments

@nebrius
Copy link
Contributor

nebrius commented Jun 1, 2015

Now that we know there is an async version of node-ffi from #9 (thanks @TooTallNate!), let's get it in production to see how well it works for robots.

I'll convert Raspi-IO first, since it's a native module I oversee and isn't nearly as popular or depended on as node-serialport, but still has a following.

Once this is done, we can wait a while and see if there are any issues that crop up and then we can look at converting others, if all goes well.

@voodootikigod
Copy link

Can you keep notes on the process so we can port node-sp in the same
fashion? <3z

Chris Williams

@voodootikigod http://twitter.com/voodootikigod | GitHub
http://github.com/voodootikigod

The things I make that you should check out:
SaferAging http://www.saferaging.com/ | JSConf http://jsconf.com/ |
RobotsConf http://robotsconf.com/ | RobotsWeekly
http://robotsweekly.com/

Help me end the negativity on the internet, share this
http://jsconf.eu/2011/an_end_to_negativity.html.

On Mon, Jun 1, 2015 at 1:49 PM, Bryan Hughes [email protected]
wrote:

Now that we know there is an async version of node-ffi from #9
#9 (thanks @TooTallNate
https://github.com/TooTallNate!), let's get it in production to see how
well it works for robots.

I'll convert Raspi-IO first, since it's a native module I oversee and
isn't nearly as popular or depended on as node-serialport, but still has a
following.

Once this is done, we can wait a while and see if there are any issues
that crop up and then we can look at converting others, if all goes well.


Reply to this email directly or view it on GitHub
#11.

@nebrius
Copy link
Contributor Author

nebrius commented Jun 1, 2015

@voodootikigod will do!

@nebrius
Copy link
Contributor Author

nebrius commented Jun 2, 2015

I have some preliminary results. Raspi-IO is composed of a bag of modules I wrote, and I just finished converting one of them over: raspi-gpio.

This one only had synchronous calls, e.g. doing all the work in NAN_METHOD(foo) {} directly as opposed to instantiating a NanAsyncWorker. This was way easy, as it turns out.

I basically copied the C++ code inside the NAN_METHOD calls into JavaScript, undid the parameter marshalling, and hand-transpiled the rest. It took me about 20 minutes to do, but then again this is a small, simple module. You can see the diff here: nebrius/raspi-gpio@92b4d95

@nebrius
Copy link
Contributor Author

nebrius commented Jun 2, 2015

I'm going to tackle the raspi module next, which does make use of a NanAsyncWorker

@fivdi
Copy link

fivdi commented Jun 2, 2015

@bryan-m-hughes have you got any information about the performance of raspi-gpio before and after the modification? For example, the number of calls per second possible for DigitalInput#read and DigitalOutput#write?

@voodootikigod
Copy link

Maybe I am not understanding this , but isn't node-ffi ALSO a native module
and therefore aren't we just consolidating pain (not a bad thing, just a
question)?

Chris Williams

@voodootikigod http://twitter.com/voodootikigod | GitHub
http://github.com/voodootikigod

The things I make that you should check out:
SaferAging http://www.saferaging.com/ | JSConf http://jsconf.com/ |
RobotsConf http://robotsconf.com/ | RobotsWeekly
http://robotsweekly.com/

Help me end the negativity on the internet, share this
http://jsconf.eu/2011/an_end_to_negativity.html.

On Tue, Jun 2, 2015 at 8:33 AM, Brian Cooke [email protected]
wrote:

@bryan-m-hughes https://github.com/bryan-m-hughes have you got any
information about the performance of raspi-gpio before and after the
modification? For example, the number of calls per second possible for
DigitalInput#read and DigitalOutput#read?


Reply to this email directly or view it on GitHub
#11 (comment).

@fivdi
Copy link

fivdi commented Jun 2, 2015

At the moment, yes, but node-ffi may (will?) end up in core. See here.

@nebrius
Copy link
Contributor Author

nebrius commented Jun 2, 2015

@fivdi I don't know performance yet, that's a great question (and idea)! I'll write up a benchmark and compare before and after. Once I get raspi ported, I'll try and come up with something similar for async calls too.

@voodootikigod I was also thinking that we might be able to make node-ffi a priviledged module that is, perhaps, built alongside maybe node.js or npm in their build systems, making node-pre-gyp a more viable solution. This way, we can cover older versions of node as well even on more esoteric platforms like the Raspberry Pi, making that install process less painful too. @TooTallNate, what do you think?

@nebrius
Copy link
Contributor Author

nebrius commented Jun 2, 2015

There's a bigger performance gap than I expected :(

Here's my test code:

var raspi = require('raspi');
//var gpio = require('raspi-gpio.native');
var gpio = require('raspi-gpio.ffi');

var ITERATIONS = 1000;

raspi.init(function() {
  var input = new gpio.DigitalInput('GPIO4');
  var output = new gpio.DigitalOutput('GPIO17');
  var start, end;

  start = Date.now();
  for (i = 0; i < ITERATIONS; i++) {
    input.read();
  }
  end = Date.now();
  console.log('Read took ' + (end - start) + ' ms');
});

The FFI version of raspi-gpio (v1.3.0) averaged 360ms, whereas the native version of raspi-gpio (1.2.1) averaged 5ms. This was averaged across 5 runs each.

@TooTallNate
Copy link

I was also thinking that we might be able to make node-ffi a priviledged module that is, perhaps, built alongside maybe node.js

I'm trying to make this happen. See: https://github.com/nodejs/io.js/labels/ffi

@voodootikigod
Copy link

i think you were missing the native version time :)

Chris Williams

@voodootikigod http://twitter.com/voodootikigod | GitHub
http://github.com/voodootikigod

The things I make that you should check out:
SaferAging http://www.saferaging.com/ | JSConf http://jsconf.com/ |
RobotsConf http://robotsconf.com/ | RobotsWeekly
http://robotsweekly.com/

Help me end the negativity on the internet, share this
http://jsconf.eu/2011/an_end_to_negativity.html.

On Tue, Jun 2, 2015 at 12:38 PM, Bryan Hughes [email protected]
wrote:

There's a bigger performance gap than I expected :(

Here's my test code:

var raspi = require('raspi');//var gpio = require('raspi-gpio.native');var gpio = require('raspi-gpio.ffi');
var ITERATIONS = 1000;

raspi.init(function() {
var input = new gpio.DigitalInput('GPIO4');
var output = new gpio.DigitalOutput('GPIO17');
var start, end;

start = Date.now();
for (i = 0; i < ITERATIONS; i++) {
input.read();
}
end = Date.now();
console.log('Read took ' + (end - start) + ' ms');
});

The FFI version of raspi-gpio (v1.3.0) averaged 360ms, whereas the native
version of raspi-gpio (1.2.1). This was averaged across 5 runs each.


Reply to this email directly or view it on GitHub
#11 (comment).

@nebrius
Copy link
Contributor Author

nebrius commented Jun 2, 2015

@voodootikigod I think my brain kinda wondered off a bit there while posting :). I edited the comment, but of course there is no email notifications.

5ms for native, compared with 360ms for FFI, so about two orders of magnitude. These are synthetic tests of course, designed to measure raw performance, so the performance drop may not matter, depending on the circumstances.

@fivdi
Copy link

fivdi commented Jun 2, 2015

Using process.hrtime() rather than Date.now() should give more accurate measurements, but the trend isn't going to change. @bryan-m-hughes are you going to go ahead and switch everything in raspi-io over to ffi? I guess it's a tough decision.

@nebrius
Copy link
Contributor Author

nebrius commented Jun 2, 2015

@fivdi right, I always forget about process.hrtime() :(

I'm leaning towards switching raspi-gpio back, but I'm not going to formally make a decision until later this week. I kinda want to let info this marinate a while.

Fortunately, raspi-io's version locking saves the day yet again, it's still on 1.2.1 of raspi-gpio. Seriously, that was the best decision I think I've made for raspi-io, even though I didn't know most of the reasons why at the time.

@reconbot
Copy link

reconbot commented Jun 2, 2015

Why it's slower is a big question, maybe it can improved upon?
On Jun 2, 2015 3:48 PM, "Bryan Hughes" [email protected] wrote:

@fivdi https://github.com/fivdi right, I always forget about
process.hrtime() :(

I'm leaning towards switching raspi-gpio back, but I'm not going to
formally make a decision until later this week. I kinda want to let info
this marinate a while.

Fortunately, raspi-io's version locking saves the day yet again, it's
still on 1.2.1 of raspi-gpio
https://github.com/bryan-m-hughes/raspi-io/blob/master/package.json#L20.
Seriously, that was the best decision I think I've made for raspi-io, even
though I didn't know most of the reasons why at the time.


Reply to this email directly or view it on GitHub
#11 (comment).

@nebrius
Copy link
Contributor Author

nebrius commented Jun 2, 2015

@reconbot I'll admit to not being very familiar with FFI and performance, @TooTallNate, perhaps you could weigh in if you have a free moment?

I do remember from a previous life not in JavaScript that these kinds of approaches are typically slower. IIRC, reflection in Java is usually about two orders of magnitude slower, and reflection in C# starts out about one order of magnitude slower and gets worse the more arguments are passed to the method.

@fivdi
Copy link

fivdi commented Jun 2, 2015

The performance figures observed correspond with what the Call Overhead section of the ffi readme says.

@nebrius
Copy link
Contributor Author

nebrius commented Jul 16, 2015

After some consideration, I've decided not to switch the raspi-io ecosystem to FFI. The performance overhead is too great given that node.js at idle takes a high amount of resources as it is on the single core, ~600MHz Raspberry Pi 1.

EDIT: just for clarification, I reverted the changes in raspi-gpio and published v1.4.0 with these changes

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

No branches or pull requests

5 participants