Review Request 114847: Remove call of KFileItemModel::index() in KFileItemModelRolesUpdater::applyResolvedRoles()

[Frank Reininghaus]

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

Review request for Dolphin.


Repository: kde-baseapps


Description
-------

KFileItemModel allows to find out the index of a KFileItem with its index(const KFileItem&) method. Calling this method is not extremely expensive, but it's also not free (it looks up the URL of the KFileItem in a QHash, i.e., it has to call qHash(QString) for the full URL), so it should not be called if it's not really necessary IMHO.

In KFileItemModelRolesUpdater, the member applyResolvedRoles(const KFileItem& item, ResolveHint hint) calls KFileItemModel::index(const KFileItem&), even though every caller of applyResolvedRoles() knows the index already. I propose to modify applyResolvedRoles() and let it take the index as a parameter directly.

(I will investigate ways to prevent calls of KFileItemModel::index(const KFileItem&) also in other places. My longer-term idea is that we could consider not to initialize the model's m_items hash at all unless it is really needed. There are a few situations where we cannot really circumvent this, like when the KDirLister reports that a KFileItem is refreshed or deleted, but most of the time, when entering a new folder, we could avoid the initialization of the hash, which would save both memory and time.)


Diffs
-----

  dolphin/src/kitemviews/kfileitemmodelrolesupdater.h fced44a 
  dolphin/src/kitemviews/kfileitemmodelrolesupdater.cpp d6445c9 

Diff: https://git.reviewboard.kde.org/r/114847/diff/


Testing
-------

Loading icons and previews still works correctly for me.


Thanks,

Frank Reininghaus

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