Review Request 111789: Improve KFileItemList memory usage and performance

[Commit Hook]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/111789/#review37222
-----------------------------------------------------------


This review has been submitted with commit 0c01e89b329a846718e67dc59d216cdf6440e3f3 by Frank Reininghaus to branch frameworks.

- Commit Hook


On July 29, 2013, 10:39 p.m., Frank Reininghaus wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/111789/
> -----------------------------------------------------------
> 
> (Updated July 29, 2013, 10:39 p.m.)
> 
> 
> Review request for KDE Frameworks and David Faure.
> 
> 
> Description
> -------
> 
> The other day, I noticed that KFileItem is not declared as a Q_MOVABLE_TYPE. Therefore, QList does not store KFileItems directly, but only pointers to them, and allocates memory for every single KFileItem separately. This wastes quite a bit of memory.
> 
> It looks like now might be a good moment to fix this because we can break binary compatibility with KDE 4.x.
> 
> 
> Diffs
> -----
> 
>   staging/kio/src/core/kfileitem.h 2c33f3c 
> 
> Diff: http://git.reviewboard.kde.org/r/111789/diff/
> 
> 
> Testing
> -------
> 
> My poor man's aproach to test the memory usage and performace is here: http://paste.kde.org/p46abc91f/ (the reason for the 10 second delay is that I needed some time to take a KSysGuard screen shot).
> 
> It creates a KFileItemList with 1 million empty KFileItems. The memory usage change is shown in the pictures. I used both massif/massif-visualizer and KSysGuard to measure how much memory it uses. The KSysGuard measurement shows a far bigger difference - this is because massif only measures the net memory consumption and fails to consider the overhead which is caused by the memory allocator itself. The latter is quite considerable when many small memory allocations are made.
> 
> Moreover, I also measured the runtime of the test (without massif).
> 
> 5 measurements without patch required between 171 ms and 189 ms.
> 
> 5 measurements with patch required between 98 ms and 106 ms.
> 
> 
> File Attachments
> ----------------
> 
> Memory usage WITHOUT patch
>   http://git.reviewboard.kde.org/media/uploaded/files/2013/07/29/before.png
> Memory usage WITH patch
>   http://git.reviewboard.kde.org/media/uploaded/files/2013/07/29/after.png
> 
> 
> Thanks,
> 
> Frank Reininghaus
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20130806/9f41e618/attachment.html>