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
