----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118586/#review59497 -----------------------------------------------------------
Ship it! fwiw, this could be optimized much more reasonably if the entry map was an actual tree. thomas braxton sent me a preliminary version of that years ago, but never finished it. are you interested in picking this up? - Oswald Buddenhagen On June 6, 2014, 11:22 a.m., Milian Wolff wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/118586/ > ----------------------------------------------------------- > > (Updated June 6, 2014, 11:22 a.m.) > > > Review request for kdelibs, David Faure, Matthew Dawson, and Oswald > Buddenhagen. > > > Repository: kdelibs > > > Description > ------- > > Optimize KConfigGroup::exists and similar operations. > > Before, these kind of read-only operations did a lot of allocations: > > 1) allocate a list of all sub groups > 2) for the above, also allocate a sub-group match key > 3) iterate over sub groups, allocate a list of all keys in there > and then finally check whether that list is non-empty > > All of the above is now done without a single allocation, by simply > iterating over the list of entries. > > Note: The whole list was iterated even before in allSubGroups. Now > we still do that, but check for non-empty keys in the group or > sub group directly. Much more efficient. > > Note2: While at it, allSubGroups is also optimized to not require the > allocation of the subgroup match key. > > > Diffs > ----- > > kdecore/config/kconfig.cpp b941983 > > Diff: https://git.reviewboard.kde.org/r/118586/diff/ > > > Testing > ------- > > The unit tests all run just fine. I broke it while implementing this patch, > so I'm confident this functionality is actually properly tested and covered :) > > The saved allocations are plenty. This is e.g. from the startup of kwrite: > > 2844 allocations at: > 0x7ff7fbc490e9 QHashData::allocateNode(int) /usr/lib/libQtCore.so.4 > 0x7ff7fc152f8e QHash<QString, QHashDummyValue>::createNode(unsigned int, > QString const&, QHashDummyValue const&, QHashNode<QString, QHashDumm > yValue>**) in /usr/include/qt4/QtCore/qhash.h:543 > /ssd/milian/projects/compiled/kde4/lib/libkdecore.so.5 > 0x7ff7fc152472 QHash<QString, QHashDummyValue>::insert(QString const&, > QHashDummyValue const&) in /usr/include/qt4/QtCore/qhash.h:763 /ssd/mi > lian/projects/compiled/kde4/lib/libkdecore.so.5 > 0x7ff7fc1515f9 QSet<QString>::insert(QString const&) in > /usr/include/qt4/QtCore/qset.h:181 > /ssd/milian/projects/compiled/kde4/lib/libkdecore. > so.5 > 0x7ff7fc1501ab QSet<QString>::operator<<(QString const&) in > /usr/include/qt4/QtCore/qset.h:201 > /ssd/milian/projects/compiled/kde4/lib/libkdec > ore.so.5 > 0x7ff7fc14b4a7 KConfigPrivate::keyListImpl(QByteArray const&) const in > /ssd/milian/projects/kde4/kdelibs/kdecore/config/kconfig.cpp:367 /ssd/ > milian/projects/compiled/kde4/lib/libkdecore.so.5 > 0x7ff7fc14b25d KConfigPrivate::hasNonDeletedEntries(QByteArray const&) const > in /ssd/milian/projects/kde4/kdelibs/kdecore/config/kconfig.cpp: > 347 /ssd/milian/projects/compiled/kde4/lib/libkdecore.so.5 > 0x7ff7fc14dfeb KConfig::hasGroupImpl(QByteArray const&) const in > /ssd/milian/projects/kde4/kdelibs/kdecore/config/kconfig.cpp:856 /ssd/milian > /projects/compiled/kde4/lib/libkdecore.so.5 > 0x7ff7fc1536b4 KConfigBase::hasGroup(QByteArray const&) const in > /ssd/milian/projects/kde4/kdelibs/kdecore/config/kconfigbase.cpp:42 /ssd/mil > ian/projects/compiled/kde4/lib/libkdecore.so.5 > 0x7ff7fc158fcd KConfigGroup::exists() const in > /ssd/milian/projects/kde4/kdelibs/kdecore/config/kconfiggroup.cpp:592 > /ssd/milian/projects/com > piled/kde4/lib/libkdecore.so.5 > > 2844 allocations at: > 0x7ff7fbc71426 QString::QString(int, Qt::Initialization) > /usr/lib/libQtCore.so.4 > 0x7ff7fbd66820 ? /usr/lib/libQtCore.so.4 > 0x7ff7fbc737db QString::fromUtf8(char const*, int) /usr/lib/libQtCore.so.4 > 0x7ff7fc14b494 KConfigPrivate::keyListImpl(QByteArray const&) const in > /ssd/milian/projects/kde4/kdelibs/kdecore/config/kconfig.cpp:367 > /ssd/milian/projects/compiled/kde4/lib/libkdecore.so.5 > 0x7ff7fc14b25d KConfigPrivate::hasNonDeletedEntries(QByteArray const&) const > in /ssd/milian/projects/kde4/kdelibs/kdecore/config/kconfig.cpp:347 > /ssd/milian/projects/compiled/kde4/lib/libkdecore.so.5 > 0x7ff7fc14dfeb KConfig::hasGroupImpl(QByteArray const&) const in > /ssd/milian/projects/kde4/kdelibs/kdecore/config/kconfig.cpp:856 > /ssd/milian/projects/compiled/kde4/lib/libkdecore.so.5 > 0x7ff7fc1536b4 KConfigBase::hasGroup(QByteArray const&) const in > /ssd/milian/projects/kde4/kdelibs/kdecore/config/kconfigbase.cpp:42 > /ssd/milian/projects/compiled/kde4/lib/libkdecore.so.5 > 0x7ff7fc158fcd KConfigGroup::exists() const in > /ssd/milian/projects/kde4/kdelibs/kdecore/config/kconfiggroup.cpp:592 > /ssd/milian/projects/compiled/kde4/lib/libkdecore.so.5 > > All of these are gone with the patch. > > > Thanks, > > Milian Wolff > >
