D11282: less expensive findByUrl in KCoreDirListerCache

[Milian Wolff]

mwolff added a comment.


  I'm not into the code base, just adding some comments to ensure everything is taken into account - maybe your initial attempt was better after all...

INLINE COMMENTS

> kcoredirlister.cpp:852
> +                if (refresh) {
> +                    (*it).refresh();
>                  }

is it OK that you operate on a copy of the item here? the item in the hash won't be modified, is that on purpose?

> kcoredirlister.cpp:1970
>      // Delete all remaining items
> -    QMutableListIterator<KFileItem> it(lstItems);
> +    QMutableHashIterator<QUrl, KFileItem> it(hashItems);
>      while (it.hasNext()) {

O(N) iteration over a large hash is extremely slow, is this done elsewhere? if so, then you may need to find an alternative approach - potentially via multiple containers or by using a sorted vector after all like you proposed initially

> kcoredirlister.cpp:2532
>      // Of course if there is no filter and we can do a range-insertion instead of a loop, that might be good.
> -    QList<KFileItem>::const_iterator kit = items.begin();
> -    const QList<KFileItem>::const_iterator kend = items.end();
> +    auto kit = items.begin();
> +    const auto kend = items.end();

this can be slow, see above

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D11282

To: jtamate, #frameworks, dfaure
Cc: mwolff, michaelh, ngraham
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20180315/e661ffab/attachment-0001.html>