Web lists-archives.com

Re: Review Request 129186: [baloo] Speedup Positioncodec::encode()




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

On November 6th, 2016, 5:40 a.m. UTC, Michael Stemle wrote:

src/codecs/coding.cpp (Diff revision 1)
154
#ifdef USE_DEFAULT_CODING

QUESTION: I don't see anything here that would really need us to keep the original code around. Are we just keeping it around in case we find something wrong in this code later?

It kinda seems like we should remove it. If we do decide to keep this conditional, should we name it something more obviously related to this change?

I agree about not keeping dead code around, it's just confusing. One can use git to find it later on if needed.


On November 6th, 2016, 5:40 a.m. UTC, Michael Stemle wrote:

src/codecs/coding.cpp (Diff revision 1)
169
    baTmp.resize((values.size() + 1) * 5);

PEDANTIC QUESTION: Why do we resize() rather than just using QByteArray((values.size() + 1) * 5, '\0')? I get that it's effectively the same thing, but it seems like it would be more concise to use the constructor.

Resize is faster because it doesn't fill the contents with zeroes.


- David


On November 5th, 2016, 11:39 p.m. UTC, Christian Ehrlicher wrote:

Review request for Baloo, KDE Frameworks and Vishesh Handa.
By Christian Ehrlicher.

Updated Nov. 5, 2016, 11:39 p.m.

Repository: baloo

Description

This patch speeds up PostingCodec::encode() by a factor of ~4 by not adding every single encoded int32 to the resulting bytearray which results in a lot of small memcpy operations. The idea is to use a preallocated QByteArray and directly encode the integers into this buffer. This makes the code a little bit more complex but the speedup should be gain enough for this.

Ping! No interest in this patch? Should I discard it?

Testing

positoncodectest calculates the same md5sum as before.

Diffs

  • src/codecs/coding.cpp (5961077)

View Diff