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

Faster read/write via native bindings? (discuss) #26

Open
tjanson opened this issue Oct 27, 2014 · 14 comments
Open

Faster read/write via native bindings? (discuss) #26

tjanson opened this issue Oct 27, 2014 · 14 comments

Comments

@tjanson
Copy link
Collaborator

tjanson commented Oct 27, 2014

I’m thinking about rewriting read and write to use bindings to the native Wiring Pi library. Why?

  • On one hand I have a slightly irrational aversion against the filesystem read/writes that are currently being used. (It’s unlikely that that sys interface would change, but I’d prefer to stay above that level.) Using an existing library (or some other abstraction) just seems cleaner.
  • More importantly, I think it would be a significant speed improvement (but I haven’t actually tested this). (I’ve used eugeneware/wiring-pi before; writes in under 1ms. I don’t know how fast the fs calls are.)

What do you think: Do we even need faster r/w, or is it plenty fast already? Do you prefer the native library approach over fs writes?
(relates to #25)

@rakeshpai
Copy link
Owner

I'm on-board with this change. If we can get a perf advantage, even if theoretical, we shouldn't stop ourselves. (Of course, actual data will help, but I can understand that this might be hard to get.)

A minor concern is that since the bindings will require compilation, it significantly complicates the npm install process. There will be issues, there will be support tickets, and there will be drop-offs. I might be over-thinking this - gyp is actually very good - but I thought I'd just put this out there.

If we can ensure that things Just Work, I'm all for this change. +1 for native, rather than fs calls.

@rakeshpai
Copy link
Owner

Thought I'd add: I understand that the compilation will be the responsibility of the wiring-pi bindings lib we use (say by eugeneware). That said, I'd rather provide a developer experience where npm install just works. If it doesn't work, I'd look at that failure as ours, rather than directing blame elsewhere.

I might be overthinking all of this. ;)

@tjanson
Copy link
Collaborator Author

tjanson commented Oct 27, 2014

No, I agree 100%. But my experience with the bindings has been good, i.e., the compilation always was automatic and smooth. (Though it seems it didn’t work out of the box for this person.)

I’ll just implement it (shouldn’t be much work at all) and we can see how well it works.

@tjanson
Copy link
Collaborator Author

tjanson commented Oct 29, 2014

I just realized we might as well write native bindings for the gpio utility too, while we’re at it, and get rid of the exec calls alltogether …

One step at a time though. I’ve been a bit busy in the past days but I’m on it. :)

@jfriend00
Copy link

You can't write native bindings that will run in node unless you're going to require your app run at root level access. The whole reason for doing an exec on the separate app is so that it can be given root privileges, but your node app does not have to be.

@tjanson
Copy link
Collaborator Author

tjanson commented Oct 29, 2014

Ah, of course, I didn’t think of that … But we could at least strive to do as much natively as we can, so that only the bare minimum is done as execs.

@jfriend00
Copy link

What do you think could be improved in that regard? There's one exec() call for .open(), one for .close() and everything else is just node.js file I/O.

@tjanson
Copy link
Collaborator Author

tjanson commented Oct 29, 2014

I have rewritten pi-gpio to use the Wiring Pi gpio utility rather than gpio-admin (see #25, code is here) – so I was mostly talking about my own code, which I just wrote using exec calls to the gpio util.

But I recognize that that’s unacceptably inefficient – and given the choice of

  • the current way: fs calls,
  • my (lazy, provisional) way: exec calls, or
  • native bindings,
    I’d say we go for the bindings – that’s what I meant in my original post of this issue.

That is:

  • export (and SPI, if that ever happens) via exec,
  • r/w and everything else via native bindings.

I’m not sure whether / how much of a speed improvement this would be over fs calls – I haven’t tested and can’t speculate (okay, actually, I do: I think it’ll be a lot faster).

What do you think? Are you against changing to bindings (it does have its drawbacks: compilation)?

@jfriend00
Copy link

More native bindings and thus more things to compile per platform is a significant complication and place for things to go wrong. Thus, it should be justified with a proven and important benefit that is useful to most people in order to be worth it for the main line of code. In my application, a performance benefit for reading a GPIO port (I'm using digital temperature sensors) would not be of any value to me so thus I wouldn't be interested in added complication or potential problems. I recognize some applications may have different needs. FYI, I have my own modified pi-gpio https://github.com/jfriend00/pi-gpio/blob/master/pi-gpio.js to solve different issues I encountered (open throws an error if the port is already open, there's no synchronous way to close the port upon app exit, etc...). I just solved my own problems in an expeditious way in my modified version - I did not spend time thinking about a good general purpose way that would be good for all.

@tjanson
Copy link
Collaborator Author

tjanson commented Oct 29, 2014

I would argue that relying on native bindings of the Wiring Pi library is a cleaner approach than writing to the fs device. Yes, it means we need to compile WPi and the node bindings, but I personally haven’t had any issues with that.

I agree that we should only include it if there’s consensus that it’s benefitial. (I personally would like this for my small PiSwitch project (433MHz power socket remote utility), which needs fast writes for the datagram transmission. But if it ends up being troublesome and unnecessary for most users, I definitely don’t want to force it on anyone.)

@jfriend00
Copy link

Then I would think some performance tests with native bindings for read and write are in order to see how much of a difference it makes compared to the file system access. It's only speculation until there are meaningful test cases with measured performance tests so the benefits can be quantified and discussed.

@tjanson
Copy link
Collaborator Author

tjanson commented Oct 30, 2014

I wrote a benchmark that says it’s about 50% faster, which is significant, but not necessarily noticable. If that’s it, I would agree with you that the extra hassle is just not worth it.

I’m not sure how much I trust the benchmark though (either way). Maybe I’m not using it correctly (feel free to have a look). ~250 operations per second seems quite slow, because, again, I’m successfully using wiring-pi to send datagrams with sub-millisecond pulse length.

@tjanson
Copy link
Collaborator Author

tjanson commented Oct 31, 2014

Okay, another pretty important argument (that I should have mentioned much earlier): Wiring Pi comes with a ton of features that we might want to use. For example, it would get us a lot closer to interrupts, I2C, SPI (see #1 and #4).
It would still need to be implemented, of course, but the underlying functions are there.

Talk is cheap, so I’ll come back when I’ve written some code. :)

@rakeshpai
Copy link
Owner

I agree with @tjanson. I think, if we decide to go the route of native bindings, we should simply use the bindings for everything. The complexity/failure-possibility lies in the compile process, but if we decide to take that hit, we could simply use the bindings for everything. I don't think there'd be any advantage to use other things (exec, fs) for stuff that the bindings provide already - in fact if anything that increases complexity.

This essentially means that we'd be a easier-to-use wrapper on top of the wiring-pi bindings, and I'm cool with that. In fact, we should then focus on making this module's API easy to use, and just use the bindings for everything underneath the API layer.

My 2c.

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

No branches or pull requests

3 participants