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

Michael Stemle themanchicken at gmail.com
Sun Nov 6 05:40:13 UTC 2016


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




src/codecs/coding.cpp (line 151)
<https://git.reviewboard.kde.org/r/129186/#comment67543>

    NIT-PICKY: This looks like you commented something out and forgot to remove it.



src/codecs/coding.cpp (line 154)
<https://git.reviewboard.kde.org/r/129186/#comment67545>

    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?



src/codecs/coding.cpp (line 169)
<https://git.reviewboard.kde.org/r/129186/#comment67544>

    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.


- Michael Stemle


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/6b59b578/attachment-0001.html>


More information about the Kde-frameworks-devel mailing list