search

redis / hiredis-rb

Ruby wrapper for hiredis
BSD 3-Clause "New" or "Revised" License
320 stars 90 forks source link

rb_wait_for_single_fd() should probably be used instead of rb_thread_fd_select() #90

Open eregon opened 2 years ago

eregon commented 2 years ago

I've noticed this gem is using rb_thread_fd_select() in https://github.com/redis/hiredis-rb/blob/d62cb77c72fe292e4f35f385467006096b7d9e3b/ext/hiredis_ext/connection.c#L94 And it's used by https://github.com/redis/hiredis-rb/blob/d62cb77c72fe292e4f35f385467006096b7d9e3b/ext/hiredis_ext/connection.c#L126 and https://github.com/redis/hiredis-rb/blob/d62cb77c72fe292e4f35f385467006096b7d9e3b/ext/hiredis_ext/connection.c#L156 to wait for a single file descriptor, either read or write.

rb_thread_fd_select() uses select(2) internally which is rather inefficient, especially with a large number of open file descriptors. So int rb_wait_for_single_fd(int fd, int events, struct timeval *tv); seems a better fit there, and that uses poll() internally which is much more efficient.

As Ruby APIs there is also IO#wait_readable and IO#wait_writable. That's probably less easy to use from C as it takes a Float and not a struct timeval for the timeout.

eregon commented 2 years ago

Also select() seems limited to 1024 descriptors on Linux, so if fd happens to be higher it might just not work.