----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118587/#review59406 -----------------------------------------------------------
kdecore/config/bufferfragment_p.h <https://git.reviewboard.kde.org/r/118587/#comment41361> Why? Now it could clash with an application class BufferFragment, on a system without "hidden visibility". Or it might confuse gdb, even with hidden-visibility? not sure how that works. kdecore/config/kconfigini.cpp <https://git.reviewboard.kde.org/r/118587/#comment41362> This isn't always the case though. kmail does that, yes, but e.g. kdeglobals doesn't. Nor most desktop files, nor konquerorrc (except that some video player added per-file groups into mine!). What do we lose if the config file has no repeated keys? Memory (the hash) and CPU (inserting and looking up into the hash), right? I have a hard time being sure that the tradeoff is always in our favour. - David Faure On June 6, 2014, 9:31 a.m., Milian Wolff wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/118587/ > ----------------------------------------------------------- > > (Updated June 6, 2014, 9:31 a.m.) > > > Review request for kdelibs and David Faure. > > > 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 > >
