[kde-freebsd] QFilesSystemWatcher instead of polling/FAM

[Raphael Kubo da Costa]

On Friday 06 August 2010 09:39:29 Tijl Coosemans wrote:
> On Tuesday 03
August 2010 20:21:48 Max Brazhnikov wrote:
> > On Mon, 2 Aug 2010 22:10:52
+0200, Tijl Coosemans wrote:
> >> On Monday 19 July 2010 14:07:56 Rusty
Nejdl wrote:
> >>> On Mon, 19 Jul 2010 12:20:35 +0200, Jaap Boender wrote:
>
>>>>>>>> I've attached a patch that makes KDE use QFileSystemWatcher on
>
>>>>>>>> FreeBSD which uses kqueue instead of stat. I've been using it for
>
>>>>>>>> the past few days and it seems to work rather well.
> >>>> 
> >>>>
I've been using the patch (the July 8 version), and it works well
> >>>> for
the most part; the only problem is that plasma-desktop runs up
> >>>> to
100% CPU usage every once in a while (and becomes unresponsive),
> >>>> so
that I've got to kill and restart.
> >>>> 
> >>>> Looking at the logs, there
doesn't seem to be any particular error
> >>>> message, and it doesn't seem
to be triggered by any specific action.
> >>>> If you need me to do anything
specific for debugging purposes, or
> >>>> need more information, let me
know.
> >>> 
> >>> I can completely attest to this and mine was so bad I had
to remove
> >>> the patch.  Plasma would die but everything else would work
after
> >>> that.
> >> 
> >> It turns out that Qt uses a continue statement
in a do-while loop
> >> thereby creating an infinite loop. The attached
patch should solve it.
> >> You have to put it in devel/qt4-corelib/files/
and rebuild that port.
> >> 
> >> I'm still trying to fix some more issues,
but this should already
> >> help a lot, so please give it a try.
> > 
> >
Still crashes with this patch and your latest patch for kdelibs:
> Here's an
updated patch for devel/qt4-corelib. It turns out that kded4
> monitors a
lot of files (2000+ in my case). That means the process
> (such as other
parts of Qt) can no longer use select(2) on descriptors
> opened after those
2000+ because that's limited to 1024 (FD_SETSIZE).
> Qt doesn't do a bounds
check when it tries to use such a descriptor
> with select and essentially a
buffer overflow occurs that can cause
> a crash later on.
> 
> This patch
includes a number of other changes. Below are the commit
> logs for those
interested.
> 
> I'm also still working on a more extensive patch for
kdirwatch.
> 
> 
> 
> QKqueueFileSystemWatcherEngine: Use EV_CLEAR instead
of EV_ONESHOT.
> 
>   Using EV_ONESHOT and re-enabling the kevent after
emitting the signal
>   allows for a window in which file system changes can
go undetected. By
>   using EV_CLEAR instead the kevent can stay enabled.
>

> QKqueueFileSystemWatcherEngine: Deleting kevent is handled by close().
>

> QKqueueFileSystemWatcherEngine: Fix infinite loop cases.
> 
>   While the
application thread is removing the watch on a path, it is
>   still possible
for the worker thread to process events for that path on
>   another CPU. In
the case where the worker thread detects that the event
>   is no longer
valid, there's a continue statement inside a do-while loop
>   causing an
infinite loop, because the next event is never retrieved and
>   the same
event is found invalid over and over.
> 
>   The do-while loop can simply be
removed. This has the added benefit that
>   the lock is released between
processing events, shortening the critical
>   section and allowing the
application thread to add and remove watches
>   more easily.
> 
>
QKqueueFileSystemWatcherEngine: Handle kevent(2) returning EINTR.
> 
>   The
worker thread exits whenever the kevent call returns an error, but
>   in
the case of EINTR (interrupted by signal) it should just call kevent
>  
again. Otherwise for instance, attaching a debugger to the process
>  
causes the worker thread to exit because of the SIGSTOP it receives.
> 
>
QKqueueFileSystemWatcherEngine: Minor tweaks to locking.
> 
>   - Calls to
write(2) potentially block, so make sure the application
> thread unlocks
the mutex before it writes to the pipe between itself and
> the worker
thread, so the latter can continue to process events and
> eventually
unblock the write call (if needed) by emptying the pipe.
> 
>   - In the
worker thread there's no need to lock the mutex when processing
> a pipe
event. Generally the worker thread should hamper the application
> thread as
little as possible, so only lock the mutex where needed.
> 
>
QKqueueFileSystemWatcherEngine: Use higher file descriptors.
> 
>   A file
descriptor is used for every path to be monitored, but descriptors
>   below
FD_SETSIZE (typically 1024) are precious, for use with select(2).
> To allow
the application (and other parts of Qt) to use select(2), try to
> duplicate
the descriptor returned by open(2) above FD_SETSIZE and close(2)
> the
original. However, only do so when the descriptor table is already
> fairly
large (FD_SETSIZE / 2). This keeps the descriptor table small for
>
applications that use only a few descriptors.
> 
>   While here, also set
the close-on-exec flag on the (new) descriptor.

Hey Tijl,

Are you planning
on sending these patches upstream?