Review Request 117975: Change libmaildir listNew and listCurrent functions to explicetely not sort QDir.entryList
Kevin Krammer
krammer at kde.org
Mon May 5 12:54:31 BST 2014
> On May 4, 2014, 12:54 p.m., Kevin Krammer wrote:
> > What if you pass NoSort to entryList() itself?
>
> Sergio Luis Martins wrote:
> QStringList entryList( Filters filters = NoFilter, SortFlags sort = NoSort ) const;
>
> NoSort is already the default, so no point on passing it.
>
> What happens is that entryList() does:
> if (sort == NoSort)
> sort = d->sort;
>
> and QDirPrivate::sort is initialized with QDir::Name | QDir::IgnoreCase
>
> Buggy Qt IMHO
>
> Kevin Krammer wrote:
> Wow, buggy indeed.
> Even still wrong in Qt5.
>
> How about initializing the default to NoSort, e.g.
> QDir d( ...., QString(), NoSort );
>
> Sergio Luis Martins wrote:
> What would be the gain ? It's also less readable, as it introduces one more parameter "QString()" that people must lookup on the documentation to know what it means.
>
> Saves us one "d->sort = sort" assignment in the whole maildir import run though :)
>
> Martin Steigerwald wrote:
> Kevin, okay with you to ship? Any objections left? I like that the change makes it explicit that no sorting is required direct at the place where it matters. Its an additional call for each folder synchronrization, but I donĀ“t think that matters. I could add a comment, but then I documentated it all in the git changelog.
Yes, the difference is jsut a matter of taste anyway
- Kevin
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/117975/#review57245
-----------------------------------------------------------
On May 4, 2014, 8:58 a.m., Martin Steigerwald wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/117975/
> -----------------------------------------------------------
>
> (Updated May 4, 2014, 8:58 a.m.)
>
>
> Review request for KDEPIM, KDEPIM-Libraries, Andras Mantia, David Faure, and Sergio Luis Martins.
>
>
> Bugs: 334218
> http://bugs.kde.org/show_bug.cgi?id=334218
>
>
> Repository: kdepim-runtime
>
>
> Description
> -------
>
> On trying to find the cause for Akonadi maildir resource hogging up a Sandybridge core for minutes without any notable MySQL or disk activity, Sergio recommended doing a callgrind run. This revealed a bottleneck with QDir.entryList and there specifically in QAlgorihmsPrivate::qSortHelper[1].
>
> Calling setSorting(QDir::NoSort) on the QDir object gets rid of the sorting. This I did twice in maildir.ccp and twice in keycache.cpp for listCurrent and listNew functions there.
>
> It is not yet clear whether this change is safe, but on my tests it appears to do. Moving and filtering mails still work okay. I will CC Andras on this as Sergio tried to ping him about this change.
>
> I think it is fair to say that this change has a *huge* impact on Akonadi maildir performance with large maildir and I recommend to backport this for stable branch as well once changing to no sorting is considered to be safe.
>
> Thanks to Sergio and David for pointing out the sorting issue, telling how to change to no sorting and general help.
>
> See also:
>
> Bug 334218 - synchronizations of large folders with filesystem contents hogs a Sandybridge core for minutes
> https://bugs.kde.org/334218
>
> [1] https://bugs.kde.org/show_bug.cgi?id=334218#c4
>
> [2] Bug 334206 - While maildir resources synchronizes a folder KMail blocks on switching to a different folder:
> https://bugs.kde.org/334206
>
>
> Diffs
> -----
>
> resources/maildir/libmaildir/keycache.cpp f0af9c47cef63dac45ae68dd94fa17a040dc1593
> resources/maildir/libmaildir/maildir.cpp 9bd380201a1a250ecfe354dd99946e8dae0ab668
>
> Diff: https://git.reviewboard.kde.org/r/117975/diff/
>
>
> Testing
> -------
>
> In my tests with KMail SC 4.12.4 Debian unstable packages and Akonadi and kdepimlibs from Git it works well. Well basically Akonadi maildir resource changed from being a CPU hog for minutes to hardly every appearing in a 10 second measurement average snapshot of atop anymore. KMail hardly blocks on folder changes anymore, which I still believe to be a different issue tough[2].
>
> Mail filtering and moving mails to a different folder still works ago. And KMail is subjectively much faster. It completely changes my KMail experience from being unbearable at times to quite pleasant.
>
>
> Thanks,
>
> Martin Steigerwald
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-pim/attachments/20140505/92d6612a/attachment.html>
More information about the kde-pim
mailing list