> On April 28, 2016, 9:22 a.m., Milian Wolff wrote: > > could you also have a look at it with heaptrack? it should instantly give > > you a total number of allocations after finish, which should already give > > us an indication whether this brings lots to the table. Your callgrind > > numbers seem to indicate so though. I just feel a bit uneasy with > > lazy-constructing the string without caching it. If that is happening a > > lot, you may want to cache the result > > Aleix Pol Gonzalez wrote: > I'll upload the heaptrack files, TBH I don't see a big improvement there > (using dolphin, on plasmashell it will definitely show, I guess I should have > tested that one :D). > > Please note that while you're right, you're also wrong. The string wasn't > being used as is, on ::iconPath it needed to have the file appended as well, > so the construction at runtime happened as well. The only difference is that > we're constructing the path from 3 sources rather than 2. > > Aleix Pol Gonzalez wrote: > It went from ~500k allocations to ~450k.
I can't download the files for some reason, I get a server error :-/ But this sounds like a good improvement already and funnily enough maps directly to the 10% you measured with valgrind :) - Milian ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/127763/#review94948 ----------------------------------------------------------- On April 28, 2016, 12:39 p.m., Aleix Pol Gonzalez wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/127763/ > ----------------------------------------------------------- > > (Updated April 28, 2016, 12:39 p.m.) > > > Review request for KDE Frameworks and Christoph Feck. > > > Repository: kiconthemes > > > Description > ------- > > Leverages the implicitly shared nature of QString by only constructing the > paths when it's strictly necessary. It souldn't show a big performance impact > since the generated paths were only intermediate in most cases after all. > > This is especially important now that plasma has several KIconLoader > instances at the same time, but it should have an impact on every application > using kiconthemes (or under the plasma-integration effects). > > > Diffs > ----- > > src/kicontheme.cpp 735ecf7 > > Diff: https://git.reviewboard.kde.org/r/127763/diff/ > > > Testing > ------- > > Tests still pass, apps still work. > > Starting and closing dolphin under callgrind showed 1640M instructions, now > 1466M(10% improvement). > > > File Attachments > ---------------- > > after > > https://git.reviewboard.kde.org/media/uploaded/files/2016/04/28/3f9b64d7-58e0-454b-9774-6ec86aa7f0c3__heaptrack.dolphin.16572-after.gz > before > > https://git.reviewboard.kde.org/media/uploaded/files/2016/04/28/6229961c-bf00-4959-9fc1-9effa63b2b34__heaptrack.dolphin.16861-before.gz > > > Thanks, > > Aleix Pol Gonzalez > >
_______________________________________________ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
