[PATCH]: KIconLoader::loadIcon() speed ups

[Dan Meltzer]

On 11/5/07, Aaron J. Seigo <aseigo at kde.org> wrote:
> hi all..
>
> attached is a patch that rather dramatically improves the speed of
> KIconLoader::loadIcon().
>
> i felt like playing with code tonight, but on something other than plasma (in
> an attempt to stave off getting burnt out dealing with QGV madnesses[1]) ...
> so when i saw a bug report from Hamish on KIconLoader performance[2] i fired
> up my text editor and got kcachegrind ready.
>
> the main culprits were KIconLoaderPrivate::removeIconExtension which was doing
> this a little suboptimally but more importantly had a debug statement that's
> probably superfluous at this stage in devel. off it went, and that revealed
> the next horror shows.
>
> it was using QDir::isRelativePath exactly once and that was now accounting for
> fully 33.46% of execution time. that's because it creaes a QFileInfo which
> init's a file engine ...... yeah. way too much for a hot path like loadIcon.
> i figure we don't -really- care if it's an absolute or relative path and we
> can fudge it by asking if the first character is '/'.
>
> i'm going to out on a limb and guess that this breaks on windows. if so, i can
> wrap it in an ifdef because the win is just way too big to ignore.

how about something like this to make them win folks happy?
..Completely untested of course, and I don't know what the exact
specifications on win drive names are.. but I think they are char + :
+ \ ? is / possible for the third character? if not the check can be
simplified more.

#ifdef Q_WS_WIN
absolutePath = !name.isEmpty() && name[1] == '1' && (name[2] == '\' ||
name[2] == '/');
#else
absolutePath = !name.isEmpty() && name[0] == '/';
#endif


>
> next was to preallocate enough space in the qstring used for the key to avoid
> expensive repeated reallocs, which knocked ~6% more execution speed. between
> this and the QDir removal, that was nearly 40% of execution gone.
>
> now it's down to QPixmapCache::find() taking the most amount of time (~10%)
> mostly due to QHash it seems.
>
> with this patch we go from ~22kis (kilo icons per second ;) to 118kis. rock
> on.
>
> should this go in? =)
>
> [1] live items that accept hover events causing full screen repaints or items
> affecting the geometries of other items on the scene for no good reason
>
> [2] http://bugs.kde.org/show_bug.cgi?id=151874
>
> --
> Aaron J. Seigo
> humru othro a kohnu se
> GPG Fingerprint: 8B8B 2209 0C6F 7C47 B1EA  EE75 D6B7 2EB1 A7F1 DB43
>
> KDE core developer sponsored by Trolltech
>
>