> On Sept. 19, 2016, 9:08 p.m., Alex Richardson wrote: > > src/lib/plugin/desktopfileparser.cpp, line 478 > > <https://git.reviewboard.kde.org/r/128944/diff/1/?file=477140#file477140line478> > > > > If I read this correctly we no longer handle leading/trailing spaces > > properly. Does the test still pass? Or maybe I forgot to add tests for this > > case. > > trimmed() will add another allocation, maybe we can change the string > > in place? > > > > There is `QStringRef::trimmed()` so `line.midRef(0).trimmed()` should > > work without any extra allocations, right?
`leftRef(-1)` is slightly more efficient so rather use that I guess - Alex ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/128944/#review99278 ----------------------------------------------------------- On Sept. 19, 2016, 4:20 p.m., Aleix Pol Gonzalez wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/128944/ > ----------------------------------------------------------- > > (Updated Sept. 19, 2016, 4:20 p.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 > >
