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

Review Request #110651 - Created May 26, 2013 and submitted

Jan Kundrát
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.



  • 0
  • 0
  • 1
  • 1
Description From Last Updated
Johannes Zarl-Zierl
Jan Kundrát
Johannes Zarl-Zierl
Commit Hook
This review has been submitted with commit babed59a4ab1c869776e2ad8362d86980927a0e6 by Jan Kundrát to branch master.
Jan Kundrát
Review request changed

Status: Closed (submitted)