Bug 151874 - KIconLoader fails to cache some images
Summary: KIconLoader fails to cache some images
Status: RESOLVED FIXED
Alias: None
Product: kdelibs
Classification: Frameworks and Libraries
Component: general (show other bugs)
Version: unspecified
Platform: Compiled Sources Linux
: NOR normal
Target Milestone: ---
Assignee: kdelibs bugs
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2007-11-05 01:24 UTC by Hamish Rodda
Modified: 2007-11-07 06:20 UTC (History)
0 users

See Also:
Latest Commit:
Version Fixed In:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Hamish Rodda 2007-11-05 01:24:00 UTC
Version:            (using KDE Devel)
Installed from:    Compiled sources
OS:                Linux

KDevelop installs some images into its app directory, for use as icons in the program. (ie. PREFIX/share/apps/kdevelop/pics/*.png)  They are correctly found and loaded, but each time the icon is to be drawn, KIconLoader takes a long time to return the image.  This means that for a tree view with 50 images, each redraw takes several seconds on a modern pc.  Shouldn't the results of these loads be placed in its cache?
Comment 1 Aaron J. Seigo 2007-11-05 03:48:28 UTC
are you sure it isn't being put into the cache, and that a generally slow path is being taken in KIconLoader::loadIcon? it might be useful to see exactly what path is being taken in KIconLoader::loadIcon in this case. what is the exact call you are making to loadIcon?

there's a good amount of optimization that can be done n loadIcon, but without some time spent profiling that method it's hard to know what would have the most useful effect.

i'd also question why the icon is being loaded via the icon loader on each repaint in the first place versus simply holding onto a [QK]Icon somewhere. no matter how fast KIconLoader is, that would seem to be a better way to go.

in any case, i have a small icon load timer tester, but it's probably not using the same paths you are...

in it i have 3 loops that iterate each 1000 times. the first one loads 4 different toolbar icons, the second one loads 82 different toolbar icons, the third one loads 6 different User icons..

time for each loop: ~160ms, ~4.5s, ~200ms .. after the cash is populated, the second loop drops to ~3.7s and the User group loop drops to ~120ms.

which says to me that you're must be pushing something like 21k requests to kiconloader or else take a much slower path than my little app is testing. so, more information would be great here.
Comment 2 Aaron J. Seigo 2007-11-05 04:38:29 UTC
as an aside, callgrinding iconLoader shows a huge amount of time spent in KIconLoaderPrivate::removeIcon mostly due to the kDebug statement in there. i really don't think it is particularly useful debug at this point and so will be removing it. that and some other little optimizations drop the speed on the big loop down to the ~1050ms range. at that point we're doing ~77k icon loads per second across a set of 82 unique icons repeated 1000x.

i'm callgrinding again to see where we are now ...
Comment 3 Aaron J. Seigo 2007-11-05 05:51:05 UTC
i'm now up to 118k icon loads per second. patch going to k-c-d.

i love valgrind. i love kcachegrind. i love lamp.
Comment 4 Aaron J. Seigo 2007-11-05 19:06:38 UTC
SVN commit 733164 by aseigo:

improve the speed of KIconLoader::loadIcon and iconPath dramatically.
for cached icons, this takes us from 22k icons/s to somewhere around 118k.

windows people: you still have the bane of QDir in this hot path, but i don't know what to replace it with that will work on windows. QDir is just too expensive to use in this hot path just to check for a relative or absolute path. if you know of a clever way to do this without resorting to QDir, please make the change in the appropriate ifdef areas. otherwise you'll only get about half the benefit here.

bug 151874: i still don't know if you're having issues with the caching of icons. please test with this rev and let me know if this helps at all.

CCMAIL:kde-window@kde.org
BUGS:151874


 M  +34 -36    kiconloader.cpp  


WebSVN link: http://websvn.kde.org/?view=rev&revision=733164
Comment 5 Aaron J. Seigo 2007-11-05 19:14:38 UTC
woops.. did BUGS: instead of CCBUGS:
Comment 6 Hamish Rodda 2007-11-06 09:55:20 UTC
Callgrinding the case I am using, 80% of the time is spent in KIconThemeDir::iconPath(), and 45% is in QFile::encodeName (inside iconPath(), line 679).  Callgrinds available on request.
Comment 7 Hamish Rodda 2007-11-07 06:20:13 UTC
Fixed by commit r733705