Revision babed59...

Go back to digest for 2nd June 2013

Optimization in Graphics

Jan Kundrát committed changes in [kphotoalbum] /:

Use cached QImage instead of invoking KIcon::pixmap on broken files

KIcon is a cool class which can e.g. find the best available pixmap size based
on each request. However, its operations are rather expensive -- to the extent
that when I have most of my 63k images not available due to an unmounted NFS
share, KPA eats all CPU on this machine. This is how a backtrace from one thread
looked like:

#0 QByteArray::resize (this=0x7fffffffbdf0, size=<optimized out>) at tools/qbytearray.cpp:1433
#1 0x00007ffff326780a in QUtf8::convertFromUnicode (uc=<optimized out>, len=<optimized out>, state=0x0) at codecs/qutfcodec.cpp:143
#2 0x00007ffff3267985 in QUtf8Codec::convertFromUnicode (this=<optimized out>, uc=<optimized out>, len=<optimized out>, state=<optimized out>) at codecs/qutfcodec.cpp:522
#3 0x00007ffff326287b in QTextCodec::fromUnicode (this=<optimized out>, str=...) at codecs/qtextcodec.cpp:1375
#4 0x00007ffff3151b49 in QString::toLocal8Bit (this=0x7fffffffbe10) at tools/qstring.cpp:3767
#5 0x00007ffff319b71d in locale_encode (f=...) at io/qfile.cpp:72
#6 0x00007ffff319bd0e in QFile::encodeName (fileName=...) at io/qfile.cpp:515
#7 0x00007ffff4fbf4d1 in access (mode=4, path=...) at /var/tmp/portage/kde-base/kdelibs-4.9.5/work/kdelibs-4.9.5/kdecore/util/kde_file.h:181
#8 KIconThemeDir::iconPath (this=<optimized out>, name=...) at /var/tmp/portage/kde-base/kdelibs-4.9.5/work/kdelibs-4.9.5/kdeui/icons/kicontheme.cpp:707
#9 0x00007ffff4fbf6f2 in KIconTheme::iconPath (this=0x555555b3f7a0, name=..., size=112, match=KIconLoader::MatchBest)
at /var/tmp/portage/kde-base/kdelibs-4.9.5/work/kdelibs-4.9.5/kdeui/icons/kicontheme.cpp:492
#10 0x00007ffff4fb8fc8 in KIconLoaderPrivate::findMatchingIcon (this=<optimized out>, name=..., size=112)
at /var/tmp/portage/kde-base/kdelibs-4.9.5/work/kdelibs-4.9.5/kdeui/icons/kiconloader.cpp:1032
#11 0x00007ffff4fba458 in KIconLoaderPrivate::findMatchingIconWithGenericFallbacks (this=0x555555add130, name=..., size=112)
at /var/tmp/portage/kde-base/kdelibs-4.9.5/work/kdelibs-4.9.5/kdeui/icons/kiconloader.cpp:899
#12 0x00007ffff4fbb7ae in KIconLoader::loadIcon (this=0x555555b9d6f0, _name=..., group=KIconLoader::Desktop, size=112, state=0, overlays=..., path_store=0x0, canReturnNull=false)
at /var/tmp/portage/kde-base/kdelibs-4.9.5/work/kdelibs-4.9.5/kdeui/icons/kiconloader.cpp:1253
#13 0x00007ffff4fb22c0 in KIconEngine::pixmap (this=<optimized out>, size=..., mode=<optimized out>, state=<optimized out>)
at /var/tmp/portage/kde-base/kdelibs-4.9.5/work/kdelibs-4.9.5/kdeui/icons/kiconengine.cpp:104
#14 0x00007ffff4269ad9 in QIcon::pixmap (this=<optimized out>, size=..., mode=<optimized out>, state=<optimized out>) at image/qicon.cpp:684
#15 0x00005555556b2f8c in ImageManager::AsyncLoader::customEvent (this=0x55555cae9830, ev=0x7fffcc00bcd0) at /home/jkt/work/prog/kde/kphotoalbum/ImageManager/AsyncLoader.cpp:161

I have not profiled the application using callgrind this time -- the backtrace
looked suspicious enough. Instead, this patch simply prepares a single QImage in
advance and reuses it whenever a request for a broken image is made.

The updated version (v2) of the patch checks whether the previously used image
has a correct size, and if not, goes the slow KIcon path again. To me, this
smells like little bit too much work with a lock being held, but so be it.

REVIEW: 110651

File Changes

Modified 2 files
  •   ImageManager/AsyncLoader.cpp
  •   ImageManager/AsyncLoader.h
2 files changed in total