----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118587/#review60482 -----------------------------------------------------------
Ship it! Ship It! - Oswald Buddenhagen On June 18, 2014, 11:18 a.m., Milian Wolff wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/118587/ > ----------------------------------------------------------- > > (Updated June 18, 2014, 11:18 a.m.) > > > Review request for kdelibs, David Faure, Matthew Dawson, and Oswald > Buddenhagen. > > > Repository: kdelibs > > > Description > ------- > > Optimize KConfigIniBackend::parseConfig by reducing allocations. > > Yet another awesome application of the Qt implicit sharing trick. > Since config files often contain only few different keys and even > value strings, we can share them. This reduces memory consumption > and also speeds up parsing, as we do not have to allocate the > duplicated strings, but can simply reuse the previous values. > > The most extreme case for this of my knowledge, is KatePart: > katesyntaxhighlightingrc has more then 20k lines which triggered > about 30k allocations on startup. With this patch applied, this > value goes down dramatically. I added a simple static counter for > the cache hit/miss ratio, which resulted in 5442 cache misses compared > to 43624 cache hits across all KConfig files parsed by kwrite on startup. > > > Diffs > ----- > > kdecore/config/bufferfragment_p.h 5a753ad > kdecore/config/kconfigini.cpp 8ec43c5 > kdecore/config/kconfigini_p.h d7aa43e > > Diff: https://git.reviewboard.kde.org/r/118587/diff/ > > > Testing > ------- > > Unit tests all pass. My malloc tracer shows that the allocations are all gone. > > My malloc tracer showed before: > > 17421 allocations at: > 0x7fee73692b97 QByteArray::QByteArray(char const*, int) > /usr/lib/libQtCore.so.4 > 0x7fee73bb7cee ? /usr/lib/libkdecore.so.5 > 0x7fee73bb7fc4 ? /usr/lib/libkdecore.so.5 > 0x7fee73ba1320 ? /usr/lib/libkdecore.so.5 > 0x7fee73ba1731 KConfig::KConfig(QString const&, QFlags<KConfig::OpenFlag>, > char const*) /usr/lib/libkdecore.so.5 > 0x7fee64830c06 KateHlManager::KateHlManager() in > /ssd/milian/projects/kde4/kate/part/syntax/katesyntaxmanager.cpp:76 > /ssd/milian/projects/compiled/kde4/lib/libkatepartinterfaces.so.4 > > 12699 allocations at: > 0x7fee73692b97 QByteArray::QByteArray(char const*, int) > /usr/lib/libQtCore.so.4 > 0x7fee73bb7cd7 ? /usr/lib/libkdecore.so.5 > 0x7fee73bb7fc4 ? /usr/lib/libkdecore.so.5 > 0x7fee73ba1320 ? /usr/lib/libkdecore.so.5 > 0x7fee73ba1731 KConfig::KConfig(QString const&, QFlags<KConfig::OpenFlag>, > char const*) /usr/lib/libkdecore.so.5 > 0x7fee64830c06 KateHlManager::KateHlManager() in > /ssd/milian/projects/kde4/kate/part/syntax/katesyntaxmanager.cpp:76 > /ssd/milian/projects/compiled/kde4/lib/libkatepartinterfaces.so.4 > > These where the allocation hotspots number #1 and #3 respectively. With the > patch applied, the two locations are not under the top 10 anymore. > > > Thanks, > > Milian Wolff > >