Review Request 111922: KFileItemModel::retrieveData(): do not store default values in the hash

[Frank Reininghaus]

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


Thanks for testing! I guess I'll push it to KDE/4.11 and merge to master soon then. Even if we have all missed a problem, it will get a month of testing from our brave master users then.

About the profiling output: I should have pointed out that I'm going to remove that before pushing to git.kde.org. I just included it in the patch to make it more transparent how I got those numbers. Putting a QElapsedTimer (even inside an #ifdef) in every single function does not exactly make the code easier to read ;-)

- Frank Reininghaus


On Aug. 6, 2013, 11:07 p.m., Frank Reininghaus wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/111922/
> -----------------------------------------------------------
> 
> (Updated Aug. 6, 2013, 11:07 p.m.)
> 
> 
> Review request for Dolphin.
> 
> 
> Description
> -------
> 
> I noticed that there is still some memory usage and performance improvement possible in KFileItemModel without intrusive code changes. The idea is to only store data in the QHash for each item if these data are different from the default value. If a key is not found in the hash, QHash::value() will return the default value anyway. This saves a bit of memory and makes loading directories a bit faster.
> 
> I've added a timer to the patch to make the savings visible.
> 
> The patch is quite straightforward, I think, and if we don't find any regressions, maybe we can push it to 4.11.1 or 4.11.2.
> 
> 
> Diffs
> -----
> 
>   dolphin/src/kitemviews/kfileitemmodel.cpp c7e1c86 
>   dolphin/src/kitemviews/kstandarditemlistwidget.cpp b429211 
> 
> Diff: http://git.reviewboard.kde.org/r/111922/diff/
> 
> 
> Testing
> -------
> 
> Loaded a directory with 100,000 files in Details View.
> 
> The patch reduced the memory usage (as reported by KSysGuard) from about 245 MB to about 227 MB.
> 
> Typical profiling output from the patch is as follows on my system (release build, note that the function is called multiple times when entering a huge folder):
> 
> Without patch:
> 
> 0.00887137 ms per item. Total time: 288
> 0.00881481 ms per item. Total time: 357
> 0.00884007 ms per item. Total time: 239
> 
> With patch:
> 
> 0.00713453 ms per item. Total time: 263
> 0.00708436 ms per item. Total time: 250
> 0.00707412 ms per item. Total time: 197
> 
> This means that the total time spent in createItemDataList() is reduced from 884 ms to 710 ms.
> 
> The savings are not huge (I hope that implementing lazy loading for the hashes will have a much bigger impact in 4.12), but every little helps :-)
> 
> 
> Thanks,
> 
> Frank Reininghaus
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.kde.org/mailman/private/kfm-devel/attachments/20130808/7ccb52c6/attachment.htm>