xcb: utilize thread-safety of QAbstractEventDispatcher::wakeUp

QAbstractEventDispatcher::wakeUp is a thread-safe method, using
a queued connection to invoke it is wasteful. This type of connection
involves allocating temporary QMetaCallEvent on a heap and locking of
destination thread's post-event queue. In most use cases this is ok,
and really convenient when target method is not thread-safe. But in
this case the existing solution was suboptimal, especially because
the events we are reading can be high frequency.

The solution that we use here is lock-free. There can be only one
time when it might need to wait for the lock, which is upon exiting
the application. If we have entered the critical section in
QXcbEventReader::run(), then the registered post routine (qAddPostRoutine)
will block the QCoreApplication's dtor (this is where dispatcher is
set to 0) until we exit the critical section. We also know when not
to enter the critical section, in case dtor is already running.

With this approach we might need to compete for the lock at most
once, instead of whole application lifetime, which was the case
with the existing code.

Change-Id: If6737329c972347b0050d67658e28dbaa6f552e8
Reviewed-by: Thiago Macieira <thiago.macieira@intel.com>
This commit is contained in:
Gatis Paeglis 2018-09-19 09:50:05 +02:00
parent 80b1cce0cf
commit 4aa86d38ef
3 changed files with 22 additions and 14 deletions

View File

@ -40,11 +40,16 @@
#include "qxcbconnection.h" #include "qxcbconnection.h"
#include <QtCore/QObject> #include <QtCore/QObject>
#include <QtCore/QCoreApplication>
#include <QtCore/QAbstractEventDispatcher> #include <QtCore/QAbstractEventDispatcher>
#include <QtCore/QMutex>
#include <QtCore/QDebug> #include <QtCore/QDebug>
QT_BEGIN_NAMESPACE QT_BEGIN_NAMESPACE
static QBasicMutex qAppExiting;
static bool dispatcherOwnerDestructing = false;
/*! /*!
\class QXcbEventQueue \class QXcbEventQueue
\internal \internal
@ -78,6 +83,15 @@ QXcbEventQueue::QXcbEventQueue(QXcbConnection *connection)
{ {
connect(this, &QXcbEventQueue::finished, m_connection, &QXcbConnection::processXcbEvents); connect(this, &QXcbEventQueue::finished, m_connection, &QXcbConnection::processXcbEvents);
// When running test cases in auto tests, static variables are preserved
// between test function runs, even if Q*Application object is destroyed.
// Reset to default value to account for this.
dispatcherOwnerDestructing = false;
qAddPostRoutine([]() {
QMutexLocker locker(&qAppExiting);
dispatcherOwnerDestructing = true;
});
// Lets init the list with one node, so we don't have to check for // Lets init the list with one node, so we don't have to check for
// this special case in various places. // this special case in various places.
m_head = m_flushedTail = qXcbEventNodeFactory(nullptr); m_head = m_flushedTail = qXcbEventNodeFactory(nullptr);
@ -102,11 +116,6 @@ QXcbEventQueue::~QXcbEventQueue()
qCDebug(lcQpaEventReader) << "nodes on heap:" << m_nodesOnHeap; qCDebug(lcQpaEventReader) << "nodes on heap:" << m_nodesOnHeap;
} }
void QXcbEventQueue::registerEventDispatcher(QAbstractEventDispatcher *dispatcher)
{
connect(this, &QXcbEventQueue::eventsPending, dispatcher, &QAbstractEventDispatcher::wakeUp, Qt::QueuedConnection);
}
xcb_generic_event_t *QXcbEventQueue::takeFirst() xcb_generic_event_t *QXcbEventQueue::takeFirst()
{ {
if (isEmpty()) if (isEmpty())
@ -191,7 +200,13 @@ void QXcbEventQueue::run()
while (!m_closeConnectionDetected && (event = xcb_poll_for_queued_event(connection))) while (!m_closeConnectionDetected && (event = xcb_poll_for_queued_event(connection)))
enqueueEvent(event); enqueueEvent(event);
emit eventsPending(); QMutexLocker locker(&qAppExiting);
if (!dispatcherOwnerDestructing) {
// This thread can run before a dispatcher has been created,
// so check if it is ready.
if (QCoreApplication::eventDispatcher())
QCoreApplication::eventDispatcher()->wakeUp();
}
} }
} }

View File

@ -80,8 +80,6 @@ public:
void run() override; void run() override;
void registerEventDispatcher(QAbstractEventDispatcher *dispatcher);
bool isEmpty() const { return m_head == m_flushedTail && !m_head->event; } bool isEmpty() const { return m_head == m_flushedTail && !m_head->event; }
xcb_generic_event_t *takeFirst(); xcb_generic_event_t *takeFirst();
void flushBufferedEvents(); void flushBufferedEvents();
@ -101,8 +99,6 @@ public:
using PeekerCallback = bool (*)(xcb_generic_event_t *event, void *peekerData); using PeekerCallback = bool (*)(xcb_generic_event_t *event, void *peekerData);
bool peekEventQueue(PeekerCallback peeker, void *peekerData = nullptr, bool peekEventQueue(PeekerCallback peeker, void *peekerData = nullptr,
PeekOptions option = PeekDefault, qint32 peekerId = -1); PeekOptions option = PeekDefault, qint32 peekerId = -1);
signals:
void eventsPending();
private: private:
QXcbEventNode *qXcbEventNodeFactory(xcb_generic_event_t *event); QXcbEventNode *qXcbEventNodeFactory(xcb_generic_event_t *event);

View File

@ -343,10 +343,7 @@ bool QXcbIntegration::hasCapability(QPlatformIntegration::Capability cap) const
QAbstractEventDispatcher *QXcbIntegration::createEventDispatcher() const QAbstractEventDispatcher *QXcbIntegration::createEventDispatcher() const
{ {
QAbstractEventDispatcher *dispatcher = QXcbEventDispatcher::createEventDispatcher(defaultConnection()); return QXcbEventDispatcher::createEventDispatcher(defaultConnection());
for (int i = 0; i < m_connections.size(); i++)
m_connections[i]->eventQueue()->registerEventDispatcher(dispatcher);
return dispatcher;
} }
void QXcbIntegration::initialize() void QXcbIntegration::initialize()