D11828: Simplify orPostingIterator and make it faster

Igor Poboiko noreply at phabricator.kde.org
Mon Oct 8 20:20:21 BST 2018


poboiko added a comment.


  Looks fine to me. But do we really need to optimize it? I mean, I didn't see it running more than ~20 ms, and with this patch for small queries it runs like ~16 ms. Worst case is when user types something in KRunner, but again, the lag is negligible there.

INLINE COMMENTS

> orpostingiterator.cpp:73
>  
> -        // First or last element
> -        if (iter->docId() == 0 && iter->next() == 0) {
> -            delete iter;
> -            *it = nullptr;
> -            continue;
> +        auto docId = iter->docId();
> +        if (docId <= m_docId) {

Usage of `auto` keyword (at least for docId) somehow made it a bit harder to read for me. Looks like Qt Coding Conventions <https://wiki.qt.io/Coding_Conventions#auto_Keyword> agrees 
(it's fine for iterators, whose definition has a long-long-name and it's clear from the line that it's an iterator. But for ordinary `docId` I've began to think it's not `quint64`, but some weird class with large definition or something).

> orpostingiterator.cpp:79
> +                delete iter;
> +                *it = nullptr;
> +                it = m_iterators.erase(it);

I'm not really sure, do we actually need to set it to `nullptr`? It becomes inaccessible after next line

REPOSITORY
  R293 Baloo

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

To: bruns, #baloo, #frameworks, poboiko
Cc: fvogt, kde-frameworks-devel, #frameworks, ashaposhnikov, michaelh, astippich, spoorun, ngraham, bruns, abrahams
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20181008/ec12fa45/attachment-0001.html>


More information about the Kde-frameworks-devel mailing list