Review Request 112725: Save memory and time by lazy-loading the item data if possible

[Commit Hook]

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


This review has been submitted with commit d7446760f7ba6b70bc13f0d250213ccffa164c4f by Frank Reininghaus to branch master.

- Commit Hook


On Sept. 26, 2013, 8:36 p.m., Frank Reininghaus wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/112725/
> -----------------------------------------------------------
> 
> Review request for Dolphin.
> 
> 
> Repository: kde-baseapps
> 
> 
> Description
> -------
> 
> This patch tries to prevent calling KFileItemModel::retrieveData() when possible in order to save memory and time when loading huge directories.
> 
> 
> There are changes in the following members of KFileItemModel:
> 
> * data() and setData() are modified such that the QHash "values" for an item is initialized with retrieveData() if that has not happened already.
> 
> * indexForKeyboardSearch() uses the "text" from the KFileItem itself, rather than looking than calling data() (which can be expensive if called for many items because data() might call retrieveData()).
> 
> * isExpandable() calls data(), rather than accessing the QHash "values" directly. If this hash was not initialized it, isExpandable() would always return the default value "false".
> 
> * createItemDataList() does not call retrieveData() for each item any more. However, if the sort role is such that sortRoleCompare() would try to access the QHash "values" when sorting, retrieveData() is called to ensure that sorting works correctly.
> 
> 
> Things that could still be improved:
> 
> * Currently, the layouting in Compact View calls KFileItemModel::data() for every item. In the case that only the "Name" is shown, this could be prevented (similar to the approach that is used in Icons View, see https://git.reviewboard.kde.org/r/112253/
> 
> * When changing the roles (e.g., by adding or removing columns in Details View) or changing the sort role, retrieveData() is currently called for all items. Moreover, a changedItems signal is issued for all items, such that KFileItemModelRolesUpdater tries to load icons/previews for all items (like in Dolphin < 4.11).
> 
> 
> Diffs
> -----
> 
>   dolphin/src/kitemviews/kfileitemmodel.h 8410161 
>   dolphin/src/kitemviews/kfileitemmodel.cpp c06f87e 
>   dolphin/src/tests/kfileitemmodeltest.cpp ff8dcd2 
> 
> Diff: http://git.reviewboard.kde.org/r/112725/diff/
> 
> 
> Testing
> -------
> 
> I opened a directory with 100,000 files in Icons View and Details View with a release build, and measured the memory usage with KSysGuard. Moreover, I measured the time required to load the directory using the following patch for DolphinView: http://paste.kde.org/pae0a64c1/
> 
> The time required for the initial loading is not very meaningful because it varies strongly. We can get a better idea of how much time is spent inside Dolphin for loading if all items are cached, so I re-loaded the folder 8 of times by pressing Alt+Up and then Alt+Left.
> 
> In Icons View, the memory consumption is reduced from 198.2 MB to 164.6 MB (-17%), and the average time from 2898 ms to 2814 ms (-2.9%).
> 
> In Details View, the memory consumption is reduced from 225.1 MB to 166.6 MB (-26%), and the average time from 1539 ms to 1076 ms (-30%).
> 
> The savings are larger in Details View because retrieveData() stores more information about the items in the QHash "values" (like the "expanded parents count").
> 
> I could not see any significant differences in Compace View (as expected).
> 
> 
> Thanks,
> 
> Frank Reininghaus
> 
>

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