Avoid constantly increasing Qt event queue in KIO slaves
ClosedPublic

Authored by davidedmundson on Nov 27 2018, 6:05 PM.

Details

Summary

Connection is used in two places SlaveBase (file.so etc) and
SlaveInterface (dolphin etc).

In SlaveInterface it operates in a normal event-driven Qt way with
signals when data is ready. If data is ready and a client reads one
line, it emitted dataReady again next event loop to tell the client to
read the next line.

SlaveBase has a custom event loop. We're either polling the task queue
or blocking for a more low level signal, we don't ever process the Qt
event queue.
The one exception is QCoreApplication::sendPostedEvents(nullptr,
QEvent::DeferredDelete); which we called manually after we've dispatched
each task.

If we're copying a lot of files, SlaveBase reads the first command
there's still many commands left so Connection in the Slave process posts an event,
which won't get used, into the queue.

This is a problem as now when we call sendPostedEvents(DefferedDelete)
qApp itterates through the list of pending events in linear time,
without clearing anything. After each command we're itterating through a
bigger and bigger list until we're spending all our CPU time here.

This patch splits Connection to have distinct modes for use by SlaveBase
and SlaveInterface as SlaveBase wasn't going to ever run those queued
events anyway.

Test Plan

Created a test set of million files 10 bytes files.
Copying using kioclient was /considerably/ quicker:
Before: 14 hours
After: 10 minutes

Unit tests still pass.

Diff Detail

Repository
R241 KIO
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
davidedmundson created this revision.Nov 27 2018, 6:05 PM
Restricted Application added a project: Frameworks. · View Herald TranscriptNov 27 2018, 6:05 PM
Restricted Application added a subscriber: kde-frameworks-devel. · View Herald Transcript
davidedmundson requested review of this revision.Nov 27 2018, 6:05 PM
davidedmundson edited the summary of this revision. (Show Details)Nov 27 2018, 6:06 PM
davidedmundson edited the test plan for this revision. (Show Details)
davidedmundson added a reviewer: dfaure.

Wow, awesome!

Might this huge improvement in small file copy speed be enough to call it a definitive fix for https://bugs.kde.org/show_bug.cgi?id=342056?

davidedmundson edited the summary of this revision. (Show Details)Nov 27 2018, 6:31 PM
dfaure accepted this revision.Nov 30 2018, 12:20 PM

Very nice catch.

Just in case, please wait for after the tagging this weekend before pushing; an old performance issue is better than a possible major regression.

This revision is now accepted and ready to land.Nov 30 2018, 12:20 PM
This revision was automatically updated to reflect the committed changes.