Review Request 128944: Reduce temporary allocations in the DesktopFileParser

[Milian Wolff]

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



personally, I don't think it's a good idea to use QString instead of QByteArray here. If that is really just for QStringRef in the function, wouldn't a simple view class on the byte array be the better choice? It's not much code to write, afaik you could even copy the code that is on gerrit somewhere for QByteArrayView. That way you will reduce the allocations but keep the data in UTF-8 instead of converting it to UTF-16. Also, your patch will probably use more memory now, or?


src/lib/plugin/desktopfileparser.cpp (line 99)
<https://git.reviewboard.kde.org/r/128944/#comment66896>

    famous last words ;-)


- Milian Wolff


On Sept. 20, 2016, 11:19 a.m., Aleix Pol Gonzalez wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/128944/
> -----------------------------------------------------------
> 
> (Updated Sept. 20, 2016, 11:19 a.m.)
> 
> 
> Review request for KDE Frameworks.
> 
> 
> Repository: kcoreaddons
> 
> 
> Description
> -------
> 
> While analising plasmashell under heaptrack, one of the sore spots is temporary allocations within DesktopFileParser. This improves the situation by:
> 
> * Only converting to QString/utf8 once.
> * Using QStringRef instead of fully splitting QString to parse them.
> 
> 
> Diffs
> -----
> 
>   src/lib/plugin/desktopfileparser.cpp 2eb198d 
>   src/lib/plugin/desktopfileparser_p.h c61b297 
> 
> Diff: https://git.reviewboard.kde.org/r/128944/diff/
> 
> 
> Testing
> -------
> 
> tests still pass, plasma still works normally.
> 
> heaptrack plasmashell:
> 
> after:
>         allocations:            4169312
>         leaked allocations:     83225
>         temporary allocations:  606902
> 
> before:
>         allocations:            4680691
>         leaked allocations:     84825
>         temporary allocations:  819292
> 
> 
> Thanks,
> 
> Aleix Pol Gonzalez
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20160921/e0917130/attachment.html>