search

socketry / nio4r

Cross-platform asynchronous I/O primitives for scalable network clients and servers.
Other
965 stars 86 forks source link

Prefer kqueue when on OSX >= v10.12.2 #268

Closed jcmfernandes closed 3 years ago

jcmfernandes commented 3 years ago

Description

This a regression I introduced in 2.5.5 (sorry!). I got my hands on a Mac to confirm it (and test this fix).

I'm also taking the chance to address https://github.com/socketry/nio4r/issues/266#issuecomment-782430078 as given that we check for linux/io_uring.h in extconf.rb, and that header was only introduced in Linux 5.1, my change is safe. In fact, while libev was looking for Linux >= 4.14 to compile safely, the backend is only activated on systems with kernel >= 5.6.1.

Types of Changes

Testing

ioquatix commented 3 years ago

Should I release v2.5.6 with these changes?

jcmfernandes commented 3 years ago

Thanks for the quick reply @ioquatix!

I don't fully grasp the performance impact of using poll instead of kqueue on OSX. But given that probably no one uses OSX in production, and therefore this only impacts development environments, IMHO it's not an urgent fix that requires a release. Proof is that no one reported it; I noticed it while reading the code.

At the same time, if releasing isn't much of an hassle, it becomes a "why not?".

I would love to get more information on https://github.com/socketry/nio4r/issues/266 but it seems like the reporter went missing.

ioquatix commented 3 years ago

kqueue is the event driven notification system which has efficient registration. poll is just a slightly more generic interface than select but performance is comparable (bad).

tarcieri commented 3 years ago

The reason kqueue wasn't the default for macOS in the past was due to alleged bugs:

http://pod.tst.eu/http://cvs.schmorp.de/libev/ev.pod#OS_X_AND_DARWIN_BUGS

See also #125

ioquatix commented 3 years ago

Yeah, it's notoriously unreliable, and I'm not sure if that's still the case, but when I did enable it a year or so ago, I felt it was stable enough (all tests in async were passing).

smridge commented 3 years ago

I'm also taking the chance to address #266 (comment) as given that we check for linux/io_uring.h in extconf.rb, and that header was only introduced in Linux 5.1, my change is safe. In fact, while libev was looking for Linux >= 4.14 to compile safely, the backend is only activated on systems with kernel >= 5.6.1.

I can confirm this was resolved! Many thanks!