broulik added a comment.
I think you should cache the X props, otherwise every `data` access causes a
round trip to the X server.
INLINE COMMENTS
> xwindowtasksmodel.cpp:472
>
> +static const QByteArray s_x11AppMenuServiceNamePropertyName =
> QByteArrayLiteral("_KDE_NET_WM_APPMENU_SERVICE_NAME");
> +static const QByteArray s_x11AppMenuObjectPathPropertyName =
> QByteArrayLiteral("_KDE_NET_WM_APPMENU_OBJECT_PATH");
We generally want to avoid global statics in library code. In the applet it's
fine because it's a plugin that is loaded on demand
> xwindowtasksmodel.cpp:475
> +
> +#if HAVE_X11
> +static QHash<QByteArray, xcb_atom_t> s_atoms;
Is this check needed? I would think `xwindowtasksmodel` is guarded in its
entirety both compile-time and run-time.
> xwindowtasksmodel.cpp:486
> + QByteArray value;
> + if (!s_atoms.contains(name)) {
> + const xcb_intern_atom_cookie_t atomCookie = xcb_intern_atom(c,
> false, name.length(), name.constData());
Avoid double look-up, use iterator
> xwindowtasksmodel.cpp:494
> + s_atoms[name] = atomReply->atom;
> + if (s_atoms[name] == XCB_ATOM_NONE) {
> + return value;
Avoid double hash look-up. Store atom in variable and check it
> xwindowtasksmodel.cpp:523
> + }
> + return qMakePair(QStringLiteral(""), QStringLiteral(""));
> +}
Use null `QString()`
REPOSITORY
R120 Plasma Workspace
REVISION DETAIL
https://phabricator.kde.org/D27959
To: cblack, #plasma
Cc: broulik, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack,
jraleigh, zachus, fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, himcesjf,
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart
