[Spice-commits] client/x11

Hans de Goede jwrdegoede at kemper.freedesktop.org
Tue Oct 12 04:42:29 PDT 2010


 client/x11/event_sources_p.cpp |  295 ++++++++++++-----------------------------
 client/x11/event_sources_p.h   |   20 --
 2 files changed, 97 insertions(+), 218 deletions(-)

New commits:
commit da35f9acd6fd735734a32f3d77f154c11337053b
Author: Alexander Larsson <alexl at redhat.com>
Date:   Tue Oct 12 13:43:44 2010 +0200

    Replace epoll with select in X client
    
    The use of epoll in the client is totally overkill in terms of
    scalability, and its a problem for portability. The OSX port converts
    this to use select, but keeps some of the old complexities in the code.
    This new patch makes it simpler and look much more like the windows
    code.

diff --git a/client/x11/event_sources_p.cpp b/client/x11/event_sources_p.cpp
index d59bffd..9a69255 100644
--- a/client/x11/event_sources_p.cpp
+++ b/client/x11/event_sources_p.cpp
@@ -15,189 +15,144 @@
    License along with this library; if not, see <http://www.gnu.org/licenses/>.
 */
 
-#include <sys/epoll.h>
+#include <sys/select.h>
 #include <sys/fcntl.h>
 
 #include "event_sources.h"
 #include "debug.h"
 #include "utils.h"
 
-#ifdef USING_EVENT_FD
-#include <sys/eventfd.h>
-#endif
-
-#define NUM_EPOLL_EVENTS 10
-
-#ifdef USING_EVENT_FD
-#define WRITE_FD _event_fd
-#define EVENT_DATA_TYPE eventfd_t
-#else
-#define WRITE_FD _event_write_fd
-#define EVENT_DATA_TYPE uint8_t
-#endif
-
-class EventWrapper {
-public:
-    EventWrapper(EventSources& owner, EventSource& event)
-        : _owner (owner)
-        , _event (&event)
-        , _refs (1)
-    {
-    }
-
-    EventWrapper* ref()
-    {
-        _refs++;
-        return this;
+static void set_non_blocking(int fd)
+{
+    int flags;
+    if ((flags = ::fcntl(fd, F_GETFL)) == -1) {
+        THROW("failed to set socket non block: %s", strerror(errno));
     }
 
-    void unref()
-    {
-        if (!--_refs) {
-            _owner.remove_wrapper(this);
-            delete this;
-        }
+    if (::fcntl(fd, F_SETFL, flags | O_NONBLOCK) == -1) {
+        THROW("failed to set socket non block: %s", strerror(errno));
     }
+}
 
-    EventSource* get_event()
-    {
-        return _event;
+static void set_blocking(int fd)
+{
+    int flags;
+    if ((flags = ::fcntl(fd, F_GETFL)) == -1) {
+        THROW("failed to clear socket non block: %s", strerror(errno));
     }
 
-    void invalidate()
-    {
-        _event = NULL;
+    if (::fcntl(fd, F_SETFL, flags & ~O_NONBLOCK) == -1) {
+        THROW("failed to clear socket non block: %s", strerror(errno));
     }
-
-private:
-    EventSources& _owner;
-    EventSource* _event;
-    int _refs;
-};
+}
 
 EventSources::EventSources()
 {
-    _epoll = epoll_create(NUM_EPOLL_EVENTS);
-    if (_epoll == -1) {
-        THROW("create epool failed");
-    }
 }
 
 EventSources::~EventSources()
 {
-    Events::iterator iter = _events.begin();
-    for (; iter != _events.end(); iter++) {
-        delete *iter;
-    }
-    close(_epoll);
 }
 
-bool EventSources::wait_events(int timeout_ms)
+void EventSources_p::add_event(int fd, EventSource* source)
 {
-    struct epoll_event events[NUM_EPOLL_EVENTS];
-    int num_events = epoll_wait(_epoll, events, NUM_EPOLL_EVENTS, timeout_ms);
+    int size = _events.size();
+    _events.resize(size + 1);
+    _fds.resize(size + 1);
+    _events[size] = source;
+    _fds[size] = fd;
+}
 
-    if (num_events == -1) {
-        if (errno == EINTR) {
-            return false;
+void EventSources_p::remove_event(EventSource* source)
+{
+    int size = _events.size();
+    for (int i = 0; i < size; i++) {
+        if (_events[i] == source) {
+            for (i++; i < size; i++) {
+                _events[i - 1] = _events[i];
+                _fds[i - 1] = _fds[i];
+            }
+            _events.resize(size - 1);
+            _fds.resize(size - 1);
+            return;
         }
-        THROW("wait error eventfd failed");
+    }
+    THROW("event not found");
+}
+
+bool EventSources::wait_events(int timeout_msec)
+{
+    int maxfd = 0;
+    fd_set rfds;
+    struct timeval tv;
+    struct timeval *tvp;
+    int ready;
+
+    FD_ZERO(&rfds);
+
+    int size = _events.size();
+    for (int i = 0; i < size; i++) {
+        maxfd = MAX(maxfd, _fds[i]);
+        FD_SET(_fds[i], &rfds);
     }
 
-    for (int i = 0; i < num_events; i++) {
-        ((EventWrapper*)events[i].data.ptr)->ref();
+    if (timeout_msec == INFINITE) {
+        tvp = NULL;
+    } else {
+        tv.tv_sec = timeout_msec / 1000;
+        tv.tv_usec = (timeout_msec % 1000) * 1000;
+        tvp = &tv;
     }
 
-    for (int i = 0; i < num_events; i++) {
-        EventWrapper* wrapper;
-        EventSource* event;
+    /* Right now we only use read polling in spice */
+    ready = ::select(maxfd+1, &rfds, NULL, NULL, tvp);
 
-        wrapper = (EventWrapper *)events[i].data.ptr;
-        if ((event = wrapper->get_event())) {
-            event->action();
+    if (ready == -1) {
+        if (errno == EINTR) {
+            return false;
         }
-        wrapper->unref();
+        THROW("wait error select failed");
+    } else if (ready == 0) {
+        return false;
     }
-    return false;
-}
 
-void EventSources::add_trigger(Trigger& trigger)
-{
-    int fd = trigger.get_fd();
-    EventWrapper* wrapper = new EventWrapper(*this, trigger);
-    struct epoll_event event;
-    event.data.ptr = wrapper;
-    event.events = EPOLLIN;
-    if (epoll_ctl(_epoll, EPOLL_CTL_ADD, fd, &event) == -1) {
-        THROW("epoll add failed");
+    for (int i = 0; i < _events.size(); i++) {
+        if (FD_ISSET(_fds[i], &rfds)) {
+            _events[i]->action();
+            /* The action may have removed / added event sources changing
+               our array, so leave the loop and handle other events the next
+               time we are called */
+            return false;
+        }
     }
-    _events.push_back(wrapper);
+    return false;
 }
 
-void EventSources_p::remove_wrapper(EventWrapper* wrapper)
+void EventSources::add_trigger(Trigger& trigger)
 {
-    Events::iterator iter = _events.begin();
-    for (;; iter++) {
-        if (iter == _events.end()) {
-            THROW("wrapper not found");
-        }
-        if ((*iter) == wrapper) {
-            _events.erase(iter);
-            return;
-        }
-    }
+    add_event(trigger.get_fd(), &trigger);
 }
 
 void EventSources::remove_trigger(Trigger& trigger)
 {
-    Events::iterator iter = _events.begin();
-    for (;; iter++) {
-        if (iter == _events.end()) {
-            THROW("trigger not found");
-        }
-        if ((*iter)->get_event() == &trigger) {
-            (*iter)->invalidate();
-            (*iter)->unref();
-            break;
-        }
-    }
-    int fd = trigger.get_fd();
-    if (epoll_ctl(_epoll, EPOLL_CTL_DEL, fd, NULL) == -1) {
-        THROW("epoll remove failed");
-    }
+    remove_event(&trigger);
 }
 
 EventSources::Trigger::Trigger()
 {
-#ifdef USING_EVENT_FD
-    _event_fd = eventfd(0, 0);
-    if (_event_fd == -1) {
-        THROW("create eventfd failed");
-    }
-#else
     int fd[2];
-    if (pipe(fd) == -1) {
+    if (::pipe(fd) == -1) {
         THROW("create pipe failed");
     }
     _event_fd = fd[0];
     _event_write_fd = fd[1];
-#endif
-    int flags;
-    if ((flags = fcntl(_event_fd, F_GETFL)) == -1) {
-        THROW("failed to set eventfd non block: %s", strerror(errno));
-    }
-
-    if (fcntl(_event_fd, F_SETFL, flags | O_NONBLOCK) == -1) {
-        THROW("failed to set eventfd non block: %s", strerror(errno));
-    }
+    set_non_blocking(_event_fd);
 }
 
 EventSources::Trigger::~Trigger()
 {
     close(_event_fd);
-#ifndef USING_EVENT_FD
     close(_event_write_fd);
-#endif
 }
 
 void EventSources::Trigger::trigger()
@@ -207,8 +162,8 @@ void EventSources::Trigger::trigger()
         return;
     }
     _pending_int = true;
-    static const EVENT_DATA_TYPE val = 1;
-    if (::write(WRITE_FD, &val, sizeof(val)) != sizeof(val)) {
+    const uint8_t val = 1;
+    if (::write(_event_write_fd, &val, sizeof(val)) != sizeof(val)) {
         THROW("write event failed");
     }
 }
@@ -219,8 +174,8 @@ bool Trigger_p::reset_event()
     if (!_pending_int) {
         return false;
     }
-    EVENT_DATA_TYPE val;
-    if (read(_event_fd, &val, sizeof(val)) != sizeof(val)) {
+    uint8_t val;
+    if (::read(_event_fd, &val, sizeof(val)) != sizeof(val)) {
         THROW("event read error");
     }
     _pending_int = false;
@@ -239,95 +194,27 @@ void EventSources::Trigger::action()
     }
 }
 
-static void set_non_blocking(int fd)
-{
-    int flags;
-    if ((flags = fcntl(fd, F_GETFL)) == -1) {
-        THROW("failed to set socket non block: %s", strerror(errno));
-    }
-
-    if (fcntl(fd, F_SETFL, flags | O_NONBLOCK) == -1) {
-        THROW("failed to set socket non block: %s", strerror(errno));
-    }
-}
-
-static void set_blocking(int fd)
-{
-    int flags;
-    if ((flags = fcntl(fd, F_GETFL)) == -1) {
-        THROW("failed to clear socket non block: %s", strerror(errno));
-    }
-
-    if (fcntl(fd, F_SETFL, flags & ~O_NONBLOCK) == -1) {
-        THROW("failed to clear socket non block: %s", strerror(errno));
-    }
-}
-
-static void add_to_poll(int fd, int epoll, EventWrapper* wrapper)
-{
-    struct epoll_event event;
-    event.data.ptr = wrapper;
-    event.events = EPOLLIN | EPOLLOUT | EPOLLET;
-    if (epoll_ctl(epoll, EPOLL_CTL_ADD, fd, &event) == -1) {
-        THROW("epoll add failed");
-    }
-}
-
 void EventSources::add_socket(Socket& socket)
 {
-    int fd = socket.get_socket();
-    set_non_blocking(fd);
-    EventWrapper* wrapper = new EventWrapper(*this, socket);
-    add_to_poll(fd, _epoll, wrapper);
-    _events.push_back(wrapper);
-}
-
-static bool remove_event(EventSources_p::Events& events, EventSource& event)
-{
-    EventSources_p::Events::iterator iter = events.begin();
-    for (;; iter++) {
-        if (iter == events.end()) {
-            return false;
-        }
-        if ((*iter)->get_event() == &event) {
-            (*iter)->invalidate();
-            (*iter)->unref();
-            return true;
-        }
-    }
+    add_event(socket.get_socket(), &socket);
+    set_non_blocking(socket.get_socket());
 }
 
 void EventSources::remove_socket(Socket& socket)
 {
-    if (!remove_event(_events, socket)) {
-        THROW("socket not found");
-    }
+    remove_event(&socket);
     int fd = socket.get_socket();
-    if (epoll_ctl(_epoll, EPOLL_CTL_DEL, fd, NULL) == -1) {
-        THROW("epoll remove failed");
-    }
     set_blocking(fd);
 }
 
 void EventSources::add_file(File& file)
 {
-    int fd = file.get_fd();
-    set_non_blocking(fd);
-    EventWrapper* wrapper = new EventWrapper(*this, file);
-    add_to_poll(fd, _epoll, wrapper);
-    _events.push_back(wrapper);
+    add_event(file.get_fd(), &file);
 }
 
 void EventSources::remove_file(File& file)
 {
-    if (!remove_event(_events, file)) {
-        THROW("file not found");
-    }
-    int fd = file.get_fd();
-    if (epoll_ctl(_epoll, EPOLL_CTL_DEL, fd, NULL) == -1) {
-        THROW("epoll remove failed");
-    }
-    set_blocking(fd);
+    remove_event(&file);
 }
 
 void EventSources::add_handle(Handle& file)
diff --git a/client/x11/event_sources_p.h b/client/x11/event_sources_p.h
index 09703c0..959460c 100644
--- a/client/x11/event_sources_p.h
+++ b/client/x11/event_sources_p.h
@@ -21,24 +21,18 @@
 #include "common.h"
 #include "threads.h"
 
-#if __GLIBC__ > 2 || (__GLIBC__ == 2 && __GLIBC_MINOR__ >= 8)
-#define USING_EVENT_FD
-#endif
-
 #define INFINITE -1
 
-class EventWrapper;
+class EventSource;
 
 class EventSources_p {
-public:
-    void remove_wrapper(EventWrapper*);
+protected:
+    void add_event(int fd, EventSource* source);
+    void remove_event(EventSource* source);
 
 public:
-    int _epoll;
-    typedef std::list<EventWrapper*> Events;
-    Events _events;
-
-    friend class EventWrapper;
+    std::vector<EventSource*> _events;
+    std::vector<int> _fds;
 };
 
 class Trigger_p {
@@ -49,9 +43,7 @@ public:
 
 public:
     int _event_fd;
-#ifndef USING_EVENT_FD
     int _event_write_fd;
-#endif
     bool _pending_int;
     Mutex _lock;
 };


More information about the Spice-commits mailing list