Re: Introducing a kqueue backend for GIO (GSoC)

[Dmitry Matveev <me dmitrymatveev co uk>]

Hello Dan,

Thanks a lot for your comments!

On Tue, 21 Jun 2011 19:42:51 -0400, Dan Winship <danw gnome org> wrote:
> The configure check is setting kqueue_support to true before
> checking that kqueue() and kevent() exist.

Fixed.

> I assume this is supposed to work on all BSDs? What about OS X?
> (If not, it probably needs additional checks to avoid building
> on platforms where it won't work.)

It should. OS X also has kqueue/kevent, so in theory it should
work even there. I will try to test it on OS X to make sure.

> Re: kqueue-thread: Are you sure it's not possible add the kqueue
> fd to the main loop and just call kevent() whenever poll() says
> it's readable?

The problem is that the watched file descriptors are passed to
the kernel and notifications are returned from it within a single
call to kevent(). So I do not see any other solutions :).

> (Longer term, there is a bug in bugzilla about moving the main
> loop to use epoll() instead of poll(), and if that ever happens
> it would probably make it pretty easy to switch the BSDs to using
> kqueue() instead of poll() too.)

Yes, that would be fine. I have thought about it during my
GMainLoop implementation analysis, and, since the Windows port
already uses HANDLEs and [Msg]WaitForMultipleObjects(), I even
considered to do it during the summer, but then decided that
it would have a huge impact and harder to integrate.

> "initialized" in _kh_startup() needs to be volatile or it's not
> guaranteed to behave the way you want...

Thanks for pointing it out for me!

> There was a comment somewhere about using memory pools; in glib
> that would be gslice, and yes, it's good to use that if you're
> allocating lots of objects of the same size.

kqueue_subs are now allocated with Glib's Memory Slices.

> The write() call at the end of _kqueue_thread_func() needs to
> deal with the possibility of EINTR, I think. Or else you could
> just use some other communication mechanism, such as GAsyncQueue.
> (You might want to use that for the pick_up/removed fds lists
> too.)

Added EINTR handling.

I have tried to use GAsyncQueue to transfer fds to the kqueue thread,
but there was a regression - the monitor has missed a lot of events
on an intensive fs activity. Probably it was my bug, anyway I will not
merge it into the kqueue/master until I find a root cause.

> Minor code style problems:

Corrected.

> If you're going to do function documentation in a "structured"
> way, use gtk-doc conventions.

Done.

> The use of a "g_" prefix on static/global variables seems very
> wrong to me (especially in files where nothing else is
> g-prefixed).

Renamed.

Again, thanks for such a detailed review!

Dmitry