DevHeads.net

Review Request 118587: Optimize KConfigIniBackend::parseConfig by reducing allocations.

Review request for kdelibs.

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: <a href="https://git.reviewboard.kde.org/r/118587/diff/" title="https://git.reviewboard.kde.org/r/118587/diff/">https://git.reviewboard.kde.org/r/118587/diff/</a>

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

Comments

Re: Review Request 118587: Optimize KConfigIniBackend::parseConf

By Milian Wolff at 06/06/2014 - 04:31

(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: <a href="https://git.reviewboard.kde.org/r/118587/diff/" title="https://git.reviewboard.kde.org/r/118587/diff/">https://git.reviewboard.kde.org/r/118587/diff/</a>

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

Re: Review Request 118587: Optimize KConfigIniBackend::parseConf

By Milian Wolff at 06/06/2014 - 06:21

(Updated June 6, 2014, 11:21 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: <a href="https://git.reviewboard.kde.org/r/118587/diff/" title="https://git.reviewboard.kde.org/r/118587/diff/">https://git.reviewboard.kde.org/r/118587/diff/</a>

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

Re: Review Request 118587: Optimize KConfigIniBackend::parseConf

By Milian Wolff at 06/06/2014 - 06:28

(Updated June 6, 2014, 11:28 a.m.)

Review request for kdelibs, David Faure, Matthew Dawson, and Oswald Buddenhagen.

Changes
use friend functions

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 (updated)
kdecore/config/bufferfragment_p.h 5a753ad
kdecore/config/kconfigini.cpp 8ec43c5
kdecore/config/kconfigini_p.h d7aa43e

Diff: <a href="https://git.reviewboard.kde.org/r/118587/diff/" title="https://git.reviewboard.kde.org/r/118587/diff/">https://git.reviewboard.kde.org/r/118587/diff/</a>

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

Re: Review Request 118587: Optimize KConfigIniBackend::parseConf

By Milian Wolff at 06/06/2014 - 06:35

(Updated June 6, 2014, 11:35 a.m.)

Review request for kdelibs, David Faure, Matthew Dawson, and Oswald Buddenhagen.

Changes
Make KConfigIniBackend::BufferFragment a public class.

It's still internal, as KConfigIniBackend is a private class. This makes the code simpler (no friends required) and also allows us to keep the lookup free-function static.

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 (updated)
kdecore/config/bufferfragment_p.h 5a753ad
kdecore/config/kconfigini.cpp 8ec43c5
kdecore/config/kconfigini_p.h d7aa43e

Diff: <a href="https://git.reviewboard.kde.org/r/118587/diff/" title="https://git.reviewboard.kde.org/r/118587/diff/">https://git.reviewboard.kde.org/r/118587/diff/</a>

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

Re: Review Request 118587: Optimize KConfigIniBackend::parseConf

By Milian Wolff at 06/18/2014 - 06:15

(Updated June 18, 2014, 11:15 a.m.)

Review request for kdelibs, David Faure, Matthew Dawson, and Oswald Buddenhagen.

Changes
- reverted unrelated whitespace change
- updated qHash(BufferFragment) to use the Qt5 algorithm (without the fastCRC32 branch though)

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 (updated)
kdecore/config/bufferfragment_p.h 5a753ad
kdecore/config/kconfigini.cpp 8ec43c5
kdecore/config/kconfigini_p.h d7aa43e

Diff: <a href="https://git.reviewboard.kde.org/r/118587/diff/" title="https://git.reviewboard.kde.org/r/118587/diff/">https://git.reviewboard.kde.org/r/118587/diff/</a>

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

Re: Review Request 118587: Optimize KConfigIniBackend::parseConf

By Milian Wolff at 06/18/2014 - 06:18

(Updated June 18, 2014, 11:18 a.m.)

Review request for kdelibs, David Faure, Matthew Dawson, and Oswald Buddenhagen.

Changes
hm apparently uploaded the wrong patch before

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 (updated)
kdecore/config/bufferfragment_p.h 5a753ad
kdecore/config/kconfigini.cpp 8ec43c5
kdecore/config/kconfigini_p.h d7aa43e

Diff: <a href="https://git.reviewboard.kde.org/r/118587/diff/" title="https://git.reviewboard.kde.org/r/118587/diff/">https://git.reviewboard.kde.org/r/118587/diff/</a>

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

Re: Review Request 118587: Optimize KConfigIniBackend::parseConf

By Milian Wolff at 06/19/2014 - 05:04

(Updated June 19, 2014, 10:04 a.m.)

Status
This change has been marked as submitted.

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: <a href="https://git.reviewboard.kde.org/r/118587/diff/" title="https://git.reviewboard.kde.org/r/118587/diff/">https://git.reviewboard.kde.org/r/118587/diff/</a>

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

Re: Review Request 118587: Optimize KConfigIniBackend::parseConf

By Commit Hook at 06/19/2014 - 05:23

This review has been submitted with commit 9aeacb07b1169fa3dc9b7653e8f30070a91569d5 by Milian Wolff to branch master.

- Commit Hook

On June 19, 2014, 10:04 a.m., Milian Wolff wrote:

Re: Review Request 118587: Optimize KConfigIniBackend::parseConf

By Commit Hook at 06/19/2014 - 05:04

This review has been submitted with commit b8aaeff128233cfaecf67899168887572589dde8 by Milian Wolff to branch master.

- Commit Hook

On June 18, 2014, 11:18 a.m., Milian Wolff wrote:

Re: Review Request 118587: Optimize KConfigIniBackend::parseConf

By Oswald Buddenhagen at 06/19/2014 - 05:00

Ship it!

Ship It!

- Oswald Buddenhagen

On June 18, 2014, 11:18 a.m., Milian Wolff wrote:

Re: Review Request 118587: Optimize KConfigIniBackend::parseConf

By Oswald Buddenhagen at 06/07/2014 - 05:01

kdecore/config/bufferfragment_p.h
<https://git.reviewboard.kde.org/r/118587/#comment41440>

unrelated whitespace fix.

kdecore/config/bufferfragment_p.h
<https://git.reviewboard.kde.org/r/118587/#comment41441>

note that qt5 uses a different algo which is reportedly faster.

note that you could make it even more efficient with a more drastic approach: have the entry map hold buffer fragments to start with. the entries would also have variants (or at least byte arrays) as a cache for the already-converted (and overwritten) values. that would mean keeping the raw config file in memory, but that shouldn't be significant compared to everything else.

i have no idea whether keeping a string table of the converted values on top of that would still add value. it could if the many identical values are actually all used (identical keys should not matter, as they are never converted - unless you enumerate them).

- Oswald Buddenhagen

On June 6, 2014, 11:35 a.m., Milian Wolff wrote:

Re: Review Request 118587: Optimize KConfigIniBackend::parseConf

By Milian Wolff at 06/07/2014 - 05:11

On June 7, 2014, 10:01 a.m., Milian Wolff wrote:
Yes, interesting ideas. But it would mean one has to keep the full config file QByteArray in memory. I'm not sure that is beneficial from a memory usage pov. I think I'll eventually try it out in kf5 land. This was easy to get done for kdelibs 4.

I'll fix the issue you pointed out and try out the improved qHash from Qt5 and resubmit next week.

Thanks!

- Milian

On June 6, 2014, 11:35 a.m., Milian Wolff wrote:

Re: Review Request 118587: Optimize KConfigIniBackend::parseConf

By David Faure at 06/06/2014 - 06:12

kdecore/config/bufferfragment_p.h
<https://git.reviewboard.kde.org/r/118587/#comment41376>

You could just declare as a friend the qHash function from within KConfigIniBackend, no?

- David Faure

On June 6, 2014, 9:31 a.m., Milian Wolff wrote:

Re: Review Request 118587: Optimize KConfigIniBackend::parseConf

By Milian Wolff at 06/06/2014 - 06:15

Yes, if you prefer that I can also do that.

- Milian

On June 6, 2014, 9:31 a.m., Milian Wolff wrote:

Re: Review Request 118587: Optimize KConfigIniBackend::parseConf

By David Faure at 06/06/2014 - 05:59

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:

Re: Review Request 118587: Optimize KConfigIniBackend::parseConf

By Milian Wolff at 06/06/2014 - 06:08

Looking at the cache hit/miss ratio in Konqueror:

after startup:
HIT 7629
MISSED 5863

after shutdown:
HIT 11153
MISSED 6485

so even there this is worth it.

- Milian

On June 6, 2014, 9:31 a.m., Milian Wolff wrote:

Re: Review Request 118587: Optimize KConfigIniBackend::parseConf

By Milian Wolff at 06/06/2014 - 06:04

Because I need to access it from free functions (qHash, lookup). I can also make it a public if you prefer to keep the namespace. Is that OK?

Yes, the tradeoff is the memory of the hash-internal structure (which is temporary!).

Looking at my konquerorrc, I still see tons of shared strings: false and true :) Then all the stuff in the toolbar configuration, esp. the keys used there.

Also: konquerorrc and other config or desktop files are small. It might be slowed down by this patch, but not by much. It will not become a hotspot suddenly. But the cases where this already is an issue right now, this patch helps a lot.

- Milian

On June 6, 2014, 9:31 a.m., Milian Wolff wrote: