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

[David Faure]

> On Nov. 6, 2016, 5:40 a.m., Michael Stemle wrote:
> > src/codecs/coding.cpp, line 186
> > <https://git.reviewboard.kde.org/r/129186/diff/1/?file=482211#file482211line186>
> >
> >     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.


> On Nov. 6, 2016, 5:40 a.m., Michael Stemle wrote:
> > src/codecs/coding.cpp, line 171
> > <https://git.reviewboard.kde.org/r/129186/diff/1/?file=482211#file482211line171>
> >
> >     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.


- David


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


On Nov. 5, 2016, 11:39 p.m., Christian Ehrlicher wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/129186/
> -----------------------------------------------------------
> 
> (Updated Nov. 5, 2016, 11:39 p.m.)
> 
> 
> Review request for Baloo, KDE Frameworks and Vishesh Handa.
> 
> 
> 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?
> 
> 
> Diffs
> -----
> 
>   src/codecs/coding.cpp 5961077 
> 
> Diff: https://git.reviewboard.kde.org/r/129186/diff/
> 
> 
> Testing
> -------
> 
> positoncodectest calculates the same md5sum as before.
> 
> 
> Thanks,
> 
> Christian Ehrlicher
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20161106/0c806505/attachment-0001.html>