Prefer QThreadPool over QtConcurrent where we don't care about result

QtConcurrent::run()'s return type is marked with [[nodiscard]], which is
not nice to code that just needs to move some tasks to a worker thread.

On the other hand, QThreadPool satisfies our needs too and Qt doesn't
need to construct futures that we won't use.

One remark about the screenshot plugin: the task lambda is mutable so it
cannot be represented using a std::function.
This commit is contained in:
Vlad Zahorodnii 2023-03-24 23:46:46 +02:00
parent a2ef5cb1a7
commit 983a0bc599
7 changed files with 91 additions and 84 deletions

View file

@ -18,7 +18,6 @@ target_link_libraries(screenshot PRIVATE
KF6::Service KF6::Service
KF6::I18n KF6::I18n
Qt::Concurrent
Qt::DBus Qt::DBus
) )

View file

@ -16,7 +16,7 @@
#include <QDBusConnection> #include <QDBusConnection>
#include <QDBusConnectionInterface> #include <QDBusConnectionInterface>
#include <QtConcurrent> #include <QThreadPool>
#include <errno.h> #include <errno.h>
#include <fcntl.h> #include <fcntl.h>
@ -27,6 +27,77 @@
namespace KWin namespace KWin
{ {
class ScreenShotWriter2 : public QRunnable
{
public:
ScreenShotWriter2(FileDescriptor &&fileDescriptor, const QImage &image)
: m_fileDescriptor(std::move(fileDescriptor))
, m_image(image)
{
}
void run() override
{
const int flags = fcntl(m_fileDescriptor.get(), F_GETFL, 0);
if (flags == -1) {
qCWarning(KWIN_SCREENSHOT) << "failed to get screenshot fd flags:" << strerror(errno);
return;
}
if (!(flags & O_NONBLOCK)) {
if (fcntl(m_fileDescriptor.get(), F_SETFL, flags | O_NONBLOCK) == -1) {
qCWarning(KWIN_SCREENSHOT) << "failed to make screenshot fd non blocking:" << strerror(errno);
return;
}
}
QFile file;
if (!file.open(m_fileDescriptor.get(), QIODevice::WriteOnly)) {
qCWarning(KWIN_SCREENSHOT) << Q_FUNC_INFO << "failed to open pipe:" << file.errorString();
return;
}
const QByteArrayView buffer(m_image.constBits(), m_image.sizeInBytes());
qint64 remainingSize = buffer.size();
pollfd pfds[1];
pfds[0].fd = m_fileDescriptor.get();
pfds[0].events = POLLOUT;
while (true) {
const int ready = poll(pfds, 1, 60000);
if (ready < 0) {
if (errno != EINTR) {
qCWarning(KWIN_SCREENSHOT) << Q_FUNC_INFO << "poll() failed:" << strerror(errno);
return;
}
} else if (ready == 0) {
qCWarning(KWIN_SCREENSHOT) << Q_FUNC_INFO << "timed out writing to pipe";
return;
} else if (!(pfds[0].revents & POLLOUT)) {
qCWarning(KWIN_SCREENSHOT) << Q_FUNC_INFO << "pipe is broken";
return;
} else {
const char *chunk = buffer.constData() + (buffer.size() - remainingSize);
const qint64 writtenCount = file.write(chunk, remainingSize);
if (writtenCount < 0) {
qCWarning(KWIN_SCREENSHOT) << Q_FUNC_INFO << "write() failed:" << file.errorString();
return;
}
remainingSize -= writtenCount;
if (writtenCount == 0 || remainingSize == 0) {
return;
}
}
}
}
protected:
FileDescriptor m_fileDescriptor;
QImage m_image;
};
static ScreenShotFlags screenShotFlagsFromOptions(const QVariantMap &options) static ScreenShotFlags screenShotFlagsFromOptions(const QVariantMap &options)
{ {
ScreenShotFlags flags = ScreenShotFlags(); ScreenShotFlags flags = ScreenShotFlags();
@ -49,62 +120,6 @@ static ScreenShotFlags screenShotFlagsFromOptions(const QVariantMap &options)
return flags; return flags;
} }
static void writeBufferToPipe(FileDescriptor fileDescriptor, const QByteArray &buffer)
{
const int flags = fcntl(fileDescriptor.get(), F_GETFL, 0);
if (flags == -1) {
qCWarning(KWIN_SCREENSHOT) << "failed to get screenshot fd flags:" << strerror(errno);
return;
}
if (!(flags & O_NONBLOCK)) {
if (fcntl(fileDescriptor.get(), F_SETFL, flags | O_NONBLOCK) == -1) {
qCWarning(KWIN_SCREENSHOT) << "failed to make screenshot fd non blocking:" << strerror(errno);
return;
}
}
QFile file;
if (!file.open(fileDescriptor.get(), QIODevice::WriteOnly)) {
qCWarning(KWIN_SCREENSHOT) << Q_FUNC_INFO << "failed to open pipe:" << file.errorString();
return;
}
qint64 remainingSize = buffer.size();
pollfd pfds[1];
pfds[0].fd = fileDescriptor.get();
pfds[0].events = POLLOUT;
while (true) {
const int ready = poll(pfds, 1, 60000);
if (ready < 0) {
if (errno != EINTR) {
qCWarning(KWIN_SCREENSHOT) << Q_FUNC_INFO << "poll() failed:" << strerror(errno);
return;
}
} else if (ready == 0) {
qCWarning(KWIN_SCREENSHOT) << Q_FUNC_INFO << "timed out writing to pipe";
return;
} else if (!(pfds[0].revents & POLLOUT)) {
qCWarning(KWIN_SCREENSHOT) << Q_FUNC_INFO << "pipe is broken";
return;
} else {
const char *chunk = buffer.constData() + (buffer.size() - remainingSize);
const qint64 writtenCount = file.write(chunk, remainingSize);
if (writtenCount < 0) {
qCWarning(KWIN_SCREENSHOT) << Q_FUNC_INFO << "write() failed:" << file.errorString();
return;
}
remainingSize -= writtenCount;
if (writtenCount == 0 || remainingSize == 0) {
return;
}
}
}
}
static const QString s_dbusServiceName = QStringLiteral("org.kde.KWin.ScreenShot2"); static const QString s_dbusServiceName = QStringLiteral("org.kde.KWin.ScreenShot2");
static const QString s_dbusInterface = QStringLiteral("org.kde.KWin.ScreenShot2"); static const QString s_dbusInterface = QStringLiteral("org.kde.KWin.ScreenShot2");
static const QString s_dbusObjectPath = QStringLiteral("/org/kde/KWin/ScreenShot2"); static const QString s_dbusObjectPath = QStringLiteral("/org/kde/KWin/ScreenShot2");
@ -287,11 +302,9 @@ void ScreenShotSinkPipe2::flush(const QImage &image, const QVariantMap &attribut
results.insert(QStringLiteral("scale"), double(image.devicePixelRatio())); results.insert(QStringLiteral("scale"), double(image.devicePixelRatio()));
QDBusConnection::sessionBus().send(m_replyMessage.createReply(results)); QDBusConnection::sessionBus().send(m_replyMessage.createReply(results));
QtConcurrent::run([fileDescriptor = std::move(m_fileDescriptor), image]() mutable { auto writer = new ScreenShotWriter2(std::move(m_fileDescriptor), image);
const QByteArray buffer(reinterpret_cast<const char *>(image.constBits()), writer->setAutoDelete(true);
image.sizeInBytes()); QThreadPool::globalInstance()->start(writer);
writeBufferToPipe(std::move(fileDescriptor), buffer);
});
} }
ScreenShotDBusInterface2::ScreenShotDBusInterface2(ScreenShotEffect *effect) ScreenShotDBusInterface2::ScreenShotDBusInterface2(ScreenShotEffect *effect)

View file

@ -48,7 +48,6 @@
#include <QAction> #include <QAction>
#include <QCheckBox> #include <QCheckBox>
#include <QPushButton> #include <QPushButton>
#include <QtConcurrentRun>
#include <KGlobalAccel> #include <KGlobalAccel>
#include <KLazyLocalizedString> #include <KLazyLocalizedString>
@ -186,9 +185,7 @@ void UserActionsMenu::helperDialog(const QString &message, Window *window)
if (window) { if (window) {
args << QStringLiteral("--embed") << QString::number(window->window()); args << QStringLiteral("--embed") << QString::number(window->window());
} }
QtConcurrent::run([args]() { KProcess::startDetached(QStringLiteral("kdialog"), args);
KProcess::startDetached(QStringLiteral("kdialog"), args);
});
} }
QStringList configModules(bool controlCenter) QStringList configModules(bool controlCenter)

View file

@ -14,9 +14,9 @@
#include <QIcon> #include <QIcon>
#include <QList> #include <QList>
#include <QRect> #include <QRect>
#include <QThreadPool>
#include <QUuid> #include <QUuid>
#include <QVector> #include <QVector>
#include <QtConcurrentRun>
#include <qwayland-server-plasma-window-management.h> #include <qwayland-server-plasma-window-management.h>
@ -466,15 +466,13 @@ void PlasmaWindowInterfacePrivate::setResourceName(const QString &resourceName)
void PlasmaWindowInterfacePrivate::org_kde_plasma_window_get_icon(Resource *resource, int32_t fd) void PlasmaWindowInterfacePrivate::org_kde_plasma_window_get_icon(Resource *resource, int32_t fd)
{ {
QtConcurrent::run( QThreadPool::globalInstance()->start([fd, icon = m_icon]() {
[fd](const QIcon &icon) { QFile file;
QFile file; file.open(fd, QIODevice::WriteOnly, QFileDevice::AutoCloseHandle);
file.open(fd, QIODevice::WriteOnly, QFileDevice::AutoCloseHandle); QDataStream ds(&file);
QDataStream ds(&file); ds << icon;
ds << icon; file.close();
file.close(); });
},
m_icon);
} }
void PlasmaWindowInterfacePrivate::org_kde_plasma_window_request_enter_virtual_desktop(Resource *resource, const QString &id) void PlasmaWindowInterfacePrivate::org_kde_plasma_window_request_enter_virtual_desktop(Resource *resource, const QString &id)

View file

@ -17,7 +17,7 @@ if (TARGET Qt::Widgets)
fakeoutput.cpp fakeoutput.cpp
) )
add_executable(testRenderingServer ${testRenderingServer_SRCS}) add_executable(testRenderingServer ${testRenderingServer_SRCS})
target_link_libraries(testRenderingServer kwin Qt::Concurrent Qt::Widgets) target_link_libraries(testRenderingServer kwin Qt::Core Qt::Widgets)
ecm_mark_as_test(testRenderingServer) ecm_mark_as_test(testRenderingServer)
endif() endif()
@ -26,7 +26,7 @@ target_link_libraries(copyClient KF6::WaylandClient)
ecm_mark_as_test(copyClient) ecm_mark_as_test(copyClient)
add_executable(pasteClient pasteclient.cpp) add_executable(pasteClient pasteclient.cpp)
target_link_libraries(pasteClient Qt::Concurrent KF6::WaylandClient) target_link_libraries(pasteClient Qt::Core KF6::WaylandClient)
ecm_mark_as_test(pasteClient) ecm_mark_as_test(pasteClient)
add_executable(touchClientTest touchclienttest.cpp) add_executable(touchClientTest touchclienttest.cpp)

View file

@ -22,8 +22,7 @@
#include <QFile> #include <QFile>
#include <QImage> #include <QImage>
#include <QMimeType> #include <QMimeType>
#include <QThread> #include <QThreadPool>
#include <QtConcurrentRun>
// system // system
#include <unistd.h> #include <unistd.h>
@ -137,7 +136,7 @@ void PasteClient::setupRegistry(Registry *registry)
} }
dataOffer->receive((*it).name(), pipeFds[1]); dataOffer->receive((*it).name(), pipeFds[1]);
close(pipeFds[1]); close(pipeFds[1]);
QtConcurrent::run([pipeFds] { QThreadPool::globalInstance()->start([pipeFds] {
QFile readPipe; QFile readPipe;
if (readPipe.open(pipeFds[0], QIODevice::ReadOnly)) { if (readPipe.open(pipeFds[0], QIODevice::ReadOnly)) {
qDebug() << "Pasted: " << readPipe.readLine(); qDebug() << "Pasted: " << readPipe.readLine();

View file

@ -18,11 +18,12 @@
#include <QApplication> #include <QApplication>
#include <QCommandLineParser> #include <QCommandLineParser>
#include <QDateTime> #include <QDateTime>
#include <QFile>
#include <QKeyEvent> #include <QKeyEvent>
#include <QMouseEvent> #include <QMouseEvent>
#include <QPainter> #include <QPainter>
#include <QThreadPool>
#include <QWidget> #include <QWidget>
#include <QtConcurrent>
#include <iostream> #include <iostream>
#include <unistd.h> #include <unistd.h>
@ -274,7 +275,7 @@ int main(int argc, char **argv)
exit(1); exit(1);
} }
QtConcurrent::run([pipe] { QThreadPool::globalInstance()->start([pipe] {
readDisplayFromPipe(pipe); readDisplayFromPipe(pipe);
}); });
} }