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

[Frank Reininghaus]

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

(Updated Sept. 16, 2013, 11:02 p.m.)


Review request for Dolphin.


Changes
-------

Oops, it turns out that I forgot two places in the code which still assumed that the QHash is always filled with data. That caused a crash when grouping by "Name" in some situations. Fixing that is straightforward, but I guess I'll better test it a bit more and probably add a unit test to make sure that I don't mess anything up before pushing to master.


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 (updated)
-----

  dolphin/src/kitemviews/kfileitemmodel.h 8410161 
  dolphin/src/kitemviews/kfileitemmodel.cpp 51ff142 

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/20130916/c3e576dc/attachment.htm>