cpjreynolds / rustty

A terminal UI library
https://docs.rs/rustty
MIT License
153 stars 14 forks source link

Compilation fails on Mac OS X due to missing epoll. #14

Closed SirVer closed 8 years ago

SirVer commented 9 years ago

No epoll on Mac OS X, but there is kqueue: http://stackoverflow.com/questions/13856413/does-os-x-not-support-epoll-function

$ cargo build
    Updating registry `https://github.com/rust-lang/crates.io-index`
   Compiling rustty v0.1.8 (file:///private/tmp/rustty)
src/core/terminal.rs:17:23: 17:35 error: unresolved import `nix::sys::epoll::epoll_create`. Could not find `epoll` in `nix::sys` [E0432]
src/core/terminal.rs:17 use nix::sys::epoll::{epoll_create, epoll_ctl, epoll_wait};
                                              ^~~~~~~~~~~~
src/core/terminal.rs:17:23: 17:35 help: run `rustc --explain E0432` to see a detailed explanation
src/core/terminal.rs:19:5: 19:20 error: unresolved import `nix::sys::epoll`. There is no `epoll` in `nix::sys` [E0432]
src/core/terminal.rs:19 use nix::sys::epoll;
                            ^~~~~~~~~~~~~~~
src/core/terminal.rs:19:5: 19:20 help: run `rustc --explain E0432` to see a detailed explanation
src/core/terminal.rs:18:44: 18:58 error: unresolved import `nix::sys::epoll::EpollEventKind`. Could not find `epoll` in `nix::sys` [E0432]
src/core/terminal.rs:18 use nix::sys::epoll::{EpollOp, EpollEvent, EpollEventKind};
                                                                   ^~~~~~~~~~~~~~
src/core/terminal.rs:18:44: 18:58 help: run `rustc --explain E0432` to see a detailed explanation
src/core/terminal.rs:18:32: 18:42 error: unresolved import `nix::sys::epoll::EpollEvent`. Could not find `epoll` in `nix::sys` [E0432]
src/core/terminal.rs:18 use nix::sys::epoll::{EpollOp, EpollEvent, EpollEventKind};
                                                       ^~~~~~~~~~
src/core/terminal.rs:18:32: 18:42 help: run `rustc --explain E0432` to see a detailed explanation
src/core/terminal.rs:18:23: 18:30 error: unresolved import `nix::sys::epoll::EpollOp`. Could not find `epoll` in `nix::sys` [E0432]
src/core/terminal.rs:18 use nix::sys::epoll::{EpollOp, EpollEvent, EpollEventKind};
                                              ^~~~~~~
src/core/terminal.rs:18:23: 18:30 help: run `rustc --explain E0432` to see a detailed explanation
src/core/terminal.rs:17:48: 17:58 error: unresolved import `nix::sys::epoll::epoll_wait`. Could not find `epoll` in `nix::sys` [E0432]
src/core/terminal.rs:17 use nix::sys::epoll::{epoll_create, epoll_ctl, epoll_wait};
                                                                       ^~~~~~~~~~
src/core/terminal.rs:17:48: 17:58 help: run `rustc --explain E0432` to see a detailed explanation
src/core/terminal.rs:17:37: 17:46 error: unresolved import `nix::sys::epoll::epoll_ctl`. Could not find `epoll` in `nix::sys` [E0432]
src/core/terminal.rs:17 use nix::sys::epoll::{epoll_create, epoll_ctl, epoll_wait};
                                                            ^~~~~~~~~
src/core/terminal.rs:17:37: 17:46 help: run `rustc --explain E0432` to see a detailed explanation
error: aborting due to 7 previous errors
Could not compile `rustty`.

To learn more, run the command again with --verbose.
awelkie commented 9 years ago

Would using mio help with this problem?

SirVer commented 9 years ago

I think so. Mio imho can poll on file descriptors since recently, so it could be used. It does work on Mac OS X for sure.

mio is also an excellently engineered piece of software, so I can recommend it.

ghost commented 9 years ago

I believe that mio uses kqueue on OS X, rather than poll or select. Kqueue on OS X unfortunately doesn't support polling /dev/tty (however it does support file descriptor 0). Therefore I doubt that it will help, because rustty reads input from /dev/tty.

IMHO, the best solution to this problem would be to write rust bindings for either poll or select, which are just as good as epoll/kqueue in this case because only one file descriptor needs to be polled, and use those.

cpjreynolds commented 9 years ago

@orangea is right, kqueue on OS X doesn't support /dev/tty, and I don't believe poll on OS X supports it either. However, an easier solution to this would be to spin up a new thread on initialization which just continually blocks on reading /dev/tty and sends events to the main thread through a channel. That approach would vastly simplify the implementation and take care of a lot of portability issues. I'm going to try and finish implementing that this weekend. Thoughts?

droundy commented 9 years ago

Given the separate thread approach seems likely to be more resource intensive on Linux, would you keep the epoll implementation for Linux?

On Sat, Sep 19, 2015, 1:16 PM Cole Reynolds notifications@github.com wrote:

@orangea https://github.com/orangea is right, kqueue on OS X doesn't support /dev/tty, and I don't believe poll on OS X supports it either. However, an easier solution to this would be to spin up a new thread on initialization which just continually blocks on reading /dev/tty and sends events to the main thread through a channel. That approach would vastly simplify the implementation and take care of a lot of portability issues. I'm going to try and finish implementing that this weekend. Thoughts?

— Reply to this email directly or view it on GitHub https://github.com/cpjreynolds/rustty/issues/14#issuecomment-141690348.

cpjreynolds commented 9 years ago

@droundy, I just finished a rough implementation of the separate thread approach, and it was definitely a mistake. In addition to being more resource intensive, it also ends up reducing the flexibility of the external api and the complexity of the internal workings.

In light of this, a small rust wrapper around platform specific polling apis seems to be the way to go. I'll get to work on implementing that now. When this is done we should have compatibility with most *nix systems.

SirVer commented 9 years ago

@cpjreynolds Do you mind sharing your code? I have a hard time believing that the design that does blocking IO on one thread and forwards the data through a channel should be more complicated than wrapping epoll and dealing with an event loop.

In fact, in https://github.com/swiboe/swiboe/issues/37 I just switched the client IO from a event loop to one read and one write thread (since there is only ever one connection). This improved latency at the cost of one additional thread and made the code much simpler. This is pretty much the same refactoring you considered doing, but of course in another program.

SirVer commented 9 years ago

Also @droundy, why would you consider the non-epoll approach more resource intensive? It will cost one extra thread that will mostly be idle (waiting for IO), and the queue. Therefore the cost is 1 stack frame, a few bytes for the queue and the atomic operations when pushing items in the queue - which should not be noticeable at all.

droundy commented 9 years ago

I agree that it isn't very resource intensive. But spawning a new thread and using atomic operations, waiting while the kernel schedules the thread, etc, is all more costly than a simple epoll. Not a big deal, and there is something to be gained in simplicity and a single implementation. But then, there's also a benefit to having more efficient code. To be honest, I don't know how channels are implemented, or how the kernel decides when to schedule each thread. I would expect there would be latency and CPU overhead introduced in communication between threads. Again, not much when compared with the speed of my typing, but why scrap a superior implementation in favor of an inferior one, when the superior one is already implemented?

On Sun, Sep 20, 2015 at 3:19 PM Holger Rapp notifications@github.com wrote:

Also @droundy https://github.com/droundy, why would you consider the non-epoll approach more resource intensive? It will cost one extra thread that will mostly be idle (waiting for IO), and the queue. Therefore the cost is 1 stack frame, a few bytes for the queue and the atomic operations when pushing items in the queue - which should not be noticeable at all.

— Reply to this email directly or view it on GitHub https://github.com/cpjreynolds/rustty/issues/14#issuecomment-141822349.

SirVer commented 9 years ago

But then, there's also a benefit to having more efficient code.

Well, measure first, premature optimization and everything. There is latency, but it is in order of a few hundred nanoseconds.

but why scrap a superior implementation in favor of an inferior one, when the superior one is already implemented?

I think the definition of superior can always be argued, but I'd say same code that works on more systems is superior to one that runs twice as fast on Linux - though that speed is not noticeable to the user.

If you want to stick with selceting, maybe there is a wrapper for the select syscall for rust somewhere? Since you are only selecting on one file descriptor you could use that for a posix compliant implementation that runs on any Unix.

cpjreynolds commented 9 years ago

@SirVer, I made some improvements to the threading approach and fixed the internal complications, however going with the threading approach means we can no longer poll for events with a timeout as the event api is limited to what mpsc::Receiver provides.

@droundy I also agree that we should take performance into account, I don't want rustty to be the performance bottleneck in a downstream crate. I think the best approach at the moment is to test both and decide based on those results.

I'd like to have some benchmarks of both approaches to see what the overhead is, if any. Any suggestions on how to best test this?

droundy commented 9 years ago

I believe the approach for timeouts with threads is a third thread which sleeps and then sends an empty message through a second channel. By using select on those two channels, you can achieve a timeout. I have done this myself, and can confirm that it works, albeit at the cost of a bit more complexity.

On Mon, Sep 21, 2015 at 5:56 PM Cole Reynolds notifications@github.com wrote:

@SirVer https://github.com/SirVer, I made some improvements to the threading approach and fixed the internal complications, however going with the threading approach means we can no longer poll for events with a timeout as the event api is limited to what mpsc::Receiver provides.

@droundy https://github.com/droundy I also agree that we should take performance into account, I don't want rustty to be the performance bottleneck in a downstream crate. I think the best approach at the moment is to test both and decide based on those results.

I'd like to have some benchmarks of both approaches to see what the overhead is, if any. Any suggestions on how to best test this?

— Reply to this email directly or view it on GitHub https://github.com/cpjreynolds/rustty/issues/14#issuecomment-142121772.

cpjreynolds commented 9 years ago

That would work, however that also costs another thread, and unfortunately select isn't (and from the looks of it won't be anytime soon) stable at the moment.

For the time being, I'm going to go with what you said before and rely on epoll/poll/select for our event system. I've factored the platform-specific code out into a separate crate, I'll get to work on finishing the bindings and then we should be good to go.

ghost commented 8 years ago

Is the source code for the separate crate available? edit: Nevermind, I found it :) https://github.com/cpjreynolds/rev

felipesere commented 8 years ago

What is the state of rustty on OSX? I currently have some nasty custom code to read the size of the current terminal that I'd like to replace with rustty.

cpjreynolds commented 8 years ago

As of #35, rustty uses select for polling /dev/tty, which should solve the portability issues. Closing the issue as (fingers crossed) it has been fixed. Changes will be available on cargo within the next 10 minutes as version 0.1.10.