Remove thread from QFileSystemWatcherEngine implementations.

These threads are actually counterproductive, as generally speaking, processing
watches is not that expensive an operation, so instead, they process at full
speed and can (in the case of slow processing in the thread processing the
events) stack up and consume resources for no good reason.

Threads also have an additional resource consumption per engine (some ~8mb of
thread stack on Linux), so doing away with them is nice.

A side effect of this change is that events are now effectively rate-limited by
the eventloop speed of the thread they run in, so if your thread runs too slow,
and you recieve a lot of events, on some platforms, events may be dropped now
where in the past, they would be read by the monitor thread and turned into Qt
signals (thus not visibly showing as a problem, apart from invisibly bloating
memory usage).

Task-number: QTBUG-20028
Change-Id: I345a56a8c709f6f778ca9a0b55b57c05229ba477
Reviewed-by: João Abecasis <joao.abecasis@nokia.com>
Reviewed-by: Bradley T. Hughes <bradley.hughes@nokia.com>

1 files changed, 41 insertions, 105 deletions

diff --git a/src/corelib/io/qfilesystemwatcher_kqueue.cpp b/src/corelib/io/qfilesystemwatcher_kqueue.cpp
index 7379c69d66..2f62176cd4 100644
--- a/src/corelib/io/qfilesystemwatcher_kqueue.cpp
+++ b/src/corelib/io/qfilesystemwatcher_kqueue.cpp
@@ -77,39 +77,16 @@ QKqueueFileSystemWatcherEngine *QKqueueFileSystemWatcherEngine::create()
 
 QKqueueFileSystemWatcherEngine::QKqueueFileSystemWatcherEngine(int kqfd)
     : kqfd(kqfd)
+    , notifier(kqfd, QSocketNotifier::Read, this)
 {
-    fcntl(kqfd, F_SETFD, FD_CLOEXEC);
+    connect(&notifier, SIGNAL(activated(int)), SLOT(readFromKqueue()));
 
-    if (pipe(kqpipe) == -1) {
-        perror("QKqueueFileSystemWatcherEngine: cannot create pipe");
-        kqpipe[0] = kqpipe[1] = -1;
-        return;
-    }
-    fcntl(kqpipe[0], F_SETFD, FD_CLOEXEC);
-    fcntl(kqpipe[1], F_SETFD, FD_CLOEXEC);
-
-    struct kevent kev;
-    EV_SET(&kev,
-           kqpipe[0],
-           EVFILT_READ,
-           EV_ADD | EV_ENABLE,
-           0,
-           0,
-           0);
-    if (kevent(kqfd, &kev, 1, 0, 0, 0) == -1) {
-        perror("QKqueueFileSystemWatcherEngine: cannot watch pipe, kevent returned");
-        return;
-    }
+    fcntl(kqfd, F_SETFD, FD_CLOEXEC);
 }
 
 QKqueueFileSystemWatcherEngine::~QKqueueFileSystemWatcherEngine()
 {
-    stop();
-    wait();
-
     close(kqfd);
-    close(kqpipe[0]);
-    close(kqpipe[1]);
 
     foreach (int id, pathToID)
         ::close(id < 0 ? -id : id);
@@ -192,11 +169,6 @@ QStringList QKqueueFileSystemWatcherEngine::addPaths(const QStringList &paths,
         }
     }
 
-    if (!isRunning())
-        start();
-    else
-        write(kqpipe[1], "@", 1);
-
     return p;
 }
 
@@ -230,102 +202,66 @@ QStringList QKqueueFileSystemWatcherEngine::removePaths(const QStringList &paths
         isEmpty = pathToID.isEmpty();
     }
 
-    if (isEmpty) {
-        stop();
-        wait();
-    } else {
-        write(kqpipe[1], "@", 1);
-    }
-
     return p;
 }
 
-void QKqueueFileSystemWatcherEngine::stop()
-{
-    write(kqpipe[1], "q", 1);
-}
-
-void QKqueueFileSystemWatcherEngine::run()
+void QKqueueFileSystemWatcherEngine::readFromKqueue()
 {
     forever {
+        DEBUG() << "QKqueueFileSystemWatcherEngine: polling for changes";
         int r;
         struct kevent kev;
-        DEBUG() << "QKqueueFileSystemWatcherEngine: waiting for kevents...";
-        EINTR_LOOP(r, kevent(kqfd, 0, 0, &kev, 1, 0));
+        struct timespec ts = { 0, 0 }; // 0 ts, because we want to poll
+        EINTR_LOOP(r, kevent(kqfd, 0, 0, &kev, 1, &ts));
         if (r < 0) {
             perror("QKqueueFileSystemWatcherEngine: error during kevent wait");
             return;
+        } else if (r == 0) {
+            // polling returned no events, so stop
+            break;
         } else {
             int fd = kev.ident;
 
             DEBUG() << "QKqueueFileSystemWatcherEngine: processing kevent" << kev.ident << kev.filter;
-            if (fd == kqpipe[0]) {
-                // read all pending data from the pipe
-                QByteArray ba;
-                ba.resize(kev.data);
-                if (read(kqpipe[0], ba.data(), ba.size()) != ba.size()) {
-                    perror("QKqueueFileSystemWatcherEngine: error reading from pipe");
-                    return;
-                }
-                // read the command from the buffer (but break and return on 'q')
-                char cmd = 0;
-                for (int i = 0; i < ba.size(); ++i) {
-                    cmd = ba.constData()[i];
-                    if (cmd == 'q')
-                        break;
-                }
-                // handle the command
-                switch (cmd) {
-                case 'q':
-                    DEBUG() << "QKqueueFileSystemWatcherEngine: thread received 'q', exiting...";
-                    return;
-                case '@':
-                    DEBUG() << "QKqueueFileSystemWatcherEngine: thread received '@', continuing...";
-                    break;
-                default:
-                    DEBUG() << "QKqueueFileSystemWatcherEngine: thread received unknow message" << cmd;
-                    break;
-                }
-            } else {
-                QMutexLocker locker(&mutex);
-
-                int id = fd;
-                QString path = idToPath.value(id);
+            QMutexLocker locker(&mutex);
+
+            int id = fd;
+            QString path = idToPath.value(id);
+            if (path.isEmpty()) {
+                // perhaps a directory?
+                id = -id;
+                path = idToPath.value(id);
                 if (path.isEmpty()) {
-                    // perhaps a directory?
-                    id = -id;
-                    path = idToPath.value(id);
-                    if (path.isEmpty()) {
-                        DEBUG() << "QKqueueFileSystemWatcherEngine: received a kevent for a file we're not watching";
-                        continue;
-                    }
-                }
-                if (kev.filter != EVFILT_VNODE) {
-                    DEBUG() << "QKqueueFileSystemWatcherEngine: received a kevent with the wrong filter";
+                    DEBUG() << "QKqueueFileSystemWatcherEngine: received a kevent for a file we're not watching";
                     continue;
                 }
+            }
+            if (kev.filter != EVFILT_VNODE) {
+                DEBUG() << "QKqueueFileSystemWatcherEngine: received a kevent with the wrong filter";
+                continue;
+            }
 
-                if ((kev.fflags & (NOTE_DELETE | NOTE_REVOKE | NOTE_RENAME)) != 0) {
-                    DEBUG() << path << "removed, removing watch also";
+            if ((kev.fflags & (NOTE_DELETE | NOTE_REVOKE | NOTE_RENAME)) != 0) {
+                DEBUG() << path << "removed, removing watch also";
 
-                    pathToID.remove(path);
-                    idToPath.remove(id);
-                    ::close(fd);
+                pathToID.remove(path);
+                idToPath.remove(id);
+                ::close(fd);
 
-                    if (id < 0)
-                        emit directoryChanged(path, true);
-                    else
-                        emit fileChanged(path, true);
-                } else {
-                    DEBUG() << path << "changed";
-
-                    if (id < 0)
-                        emit directoryChanged(path, false);
-                    else
-                        emit fileChanged(path, false);
-                }
+                if (id < 0)
+                    emit directoryChanged(path, true);
+                else
+                    emit fileChanged(path, true);
+            } else {
+                DEBUG() << path << "changed";
+
+                if (id < 0)
+                    emit directoryChanged(path, false);
+                else
+                    emit fileChanged(path, false);
             }
         }
+
     }
 }