Batch window changes events on XCB

Summary:
In a log from someone talking about high CPU we can see get multiple X
events for the same window as multiple events, but directly next to each
other. This causes the TaskManager to process changes multiple times
instead of just once which is a waste.

An example is just pressing "enter" in konsole, which will pointlessly
update the title.

This causes problems for expensive app lookup and also QML performs text layouts immediately so any text changes cause quite large CPU usage if done more than 60fps; especially a task text resizing
could result in resizing the entire panel.

Something not relevant in kwin that also monitors these rolls.

This class sits between KWindowSystem and XWindowTasksModel
transparently buffering the changes.

CCBUG: 378010
BUG: 365317

Reviewers: #plasma, hein, broulik

Reviewed By: #plasma, hein, broulik

Subscribers: ngraham, cfeck, broulik, hein, graesslin, plasma-devel

Tags: #plasma

Differential Revision: https://phabricator.kde.org/D7481

libtaskmanager/CMakeLists.txt

@@ -24,6 +24,7 @@ set(taskmanager_LIB_SRCS
 if (X11_FOUND)
     set(taskmanager_LIB_SRCS
         ${taskmanager_LIB_SRCS}
+        xwindowsystemeventbatcher.cpp
         xwindowtasksmodel.cpp
     )
 endif()

libtaskmanager/TODO.txt

@@ -2,7 +2,6 @@ Larger outstanding tasks:
 - Implement missing kwayland bits (e.g. transient handling, virtual desktop logic).
 
 Other:
-- Old lib compressed window changes over 200ms before grabbing new data from the server, might need to be added back to avoid a DoS vector.
 - Tests tests tests.
 - Code cleanup and addressing FIXMEs.
 - Reconsider the library exports.

libtaskmanager/xwindowsystemeventbatcher.cpp

@@ -0,0 +1,80 @@
+/********************************************************************
+Copyright 2017  David Edmundson <davidedmundson@kde.org>
+
+This library is free software; you can redistribute it and/or
+modify it under the terms of the GNU Lesser General Public
+License as published by the Free Software Foundation; either
+version 2.1 of the License, or (at your option) version 3, or any
+later version accepted by the membership of KDE e.V. (or its
+successor approved by the membership of KDE e.V.), which shall
+act as a proxy defined in Section 6 of version 3 of the license.
+
+This library is distributed in the hope that it will be useful,
+but WITHOUT ANY WARRANTY; without even the implied warranty of
+MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+Lesser General Public License for more details.
+
+You should have received a copy of the GNU Lesser General Public
+License along with this library.  If not, see <http://www.gnu.org/licenses/>.
+*********************************************************************/
+
+#include "xwindowsystemeventbatcher.h"
+
+#include <KWindowSystem>
+#include <QTimerEvent>
+#include <QDebug>
+
+#define BATCH_TIME 10
+
+static const NET::Properties s_cachableProperties = NET::WMName | NET::WMVisibleName;
+static const NET::Properties2 s_cachableProperties2 = NET::WM2UserTime;
+
+XWindowSystemEventBatcher::XWindowSystemEventBatcher(QObject* parent)
+    : QObject(parent)
+{
+    connect(KWindowSystem::self(), &KWindowSystem::windowAdded, this, &XWindowSystemEventBatcher::windowAdded);
+
+    //remove our cache entries when we lose a window, otherwise we might fire change signals after a window is destroyed which wouldn't make sense
+    connect(KWindowSystem::self(), &KWindowSystem::windowRemoved, this, [this](WId wid) {
+        m_cache.remove(wid);
+        emit windowRemoved(wid);
+    });
+
+    void (KWindowSystem::*myWindowChangeSignal)(WId window,
+        NET::Properties properties, NET::Properties2 properties2) = &KWindowSystem::windowChanged;
+    QObject::connect(KWindowSystem::self(), myWindowChangeSignal, this,
+        [this](WId window, NET::Properties properties, NET::Properties2 properties2) {
+            //if properties contained only cachable flags
+            if ((properties | s_cachableProperties) == s_cachableProperties &&
+                (properties2 | s_cachableProperties2) == s_cachableProperties2) {
+                m_cache[window].properties |= properties;
+                m_cache[window].properties2 |= properties2;
+                if (!m_timerId) {
+                    m_timerId = startTimer(BATCH_TIME);
+                }
+            } else {
+                //submit all caches along with any real updates
+                auto it = m_cache.constFind(window);
+                if (it != m_cache.constEnd()) {
+                    properties |= it->properties;
+                    properties2 |= it->properties2;
+                    m_cache.erase(it);
+                }
+                emit windowChanged(window, properties, properties2);
+            }
+        }
+    );
+}
+
+void XWindowSystemEventBatcher::timerEvent(QTimerEvent* event)
+{
+    if (event->timerId() != m_timerId) {
+        return;
+    }
+    for (auto it = m_cache.constBegin(); it!= m_cache.constEnd(); it++) {
+        emit windowChanged(it.key(), it.value().properties, it.value().properties2);
+    };
+    m_cache.clear();
+    killTimer(m_timerId);
+    m_timerId = 0;
+}

libtaskmanager/xwindowsystemeventbatcher.h

@@ -0,0 +1,51 @@
+/********************************************************************
+Copyright 2017  David Edmundson <davidedmundson@kde.org>
+
+This library is free software; you can redistribute it and/or
+modify it under the terms of the GNU Lesser General Public
+License as published by the Free Software Foundation; either
+version 2.1 of the License, or (at your option) version 3, or any
+later version accepted by the membership of KDE e.V. (or its
+successor approved by the membership of KDE e.V.), which shall
+act as a proxy defined in Section 6 of version 3 of the license.
+
+This library is distributed in the hope that it will be useful,
+but WITHOUT ANY WARRANTY; without even the implied warranty of
+MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+Lesser General Public License for more details.
+
+You should have received a copy of the GNU Lesser General Public
+License along with this library.  If not, see <http://www.gnu.org/licenses/>.
+*********************************************************************/
+
+#ifndef XWIDOWSYSTEMEVENTBATCHER_H
+#define XWIDOWSYSTEMEVENTBATCHER_H
+
+#include <QObject>
+
+#include <KWindowSystem>
+
+/*
+ * Relay class for KWindowSystem events that batches updates
+ */
+class XWindowSystemEventBatcher : public QObject
+{
+    Q_OBJECT
+public:
+    XWindowSystemEventBatcher(QObject *parent);
+Q_SIGNALS:
+    void windowAdded(WId window);
+    void windowRemoved(WId window);
+    void windowChanged(WId window, NET::Properties properties, NET::Properties2 properties2);
+protected:
+    void timerEvent(QTimerEvent *event);
+private:
+    struct AllProps {
+        NET::Properties properties = 0;
+        NET::Properties2 properties2 = 0;
+    };
+    QHash<WId, AllProps> m_cache;
+    int m_timerId = 0;
+};
+
+#endif

libtaskmanager/xwindowtasksmodel.cpp

@@ -21,6 +21,7 @@ License along with this library.  If not, see <http://www.gnu.org/licenses/>.
 
 #include "xwindowtasksmodel.h"
 #include "tasktools.h"
+#include "xwindowsystemeventbatcher.h"
 
 #include <KActivities/ResourceInstance>
 #include <KDesktopFile>
@@ -149,21 +150,21 @@ void XWindowTasksModel::Private::init()
     QObject::connect(configWatcher, &KDirWatch::created, rulesConfigChange);
     QObject::connect(configWatcher, &KDirWatch::deleted, rulesConfigChange);
 
-    QObject::connect(KWindowSystem::self(), &KWindowSystem::windowAdded, q,
+    auto windowSystem = new XWindowSystemEventBatcher(q);
+
+    QObject::connect(windowSystem, &XWindowSystemEventBatcher::windowAdded, q,
         [this](WId window) {
             addWindow(window);
         }
     );
 
-    QObject::connect(KWindowSystem::self(), &KWindowSystem::windowRemoved, q,
+    QObject::connect(windowSystem, &XWindowSystemEventBatcher::windowRemoved, q,
         [this](WId window) {
             removeWindow(window);
         }
     );
 
-    void (KWindowSystem::*myWindowChangeSignal)(WId window,
-        NET::Properties properties, NET::Properties2 properties2) = &KWindowSystem::windowChanged;
-    QObject::connect(KWindowSystem::self(), myWindowChangeSignal, q,
+    QObject::connect(windowSystem, &XWindowSystemEventBatcher::windowChanged, q,
         [this](WId window, NET::Properties properties, NET::Properties2 properties2) {
             windowChanged(window, properties, properties2);
         }