[prev in list] [next in list] [prev in thread] [next in thread] 

List:       kde-core-devel
Subject:    [PATCH]: KIconLoader::loadIcon() speed ups
From:       "Aaron J. Seigo" <aseigo () kde ! org>
Date:       2007-11-05 5:01:31
Message-ID: 200711042201.31586.aseigo () kde ! org
[Download RAW message or body]

[Attachment #2 (multipart/mixed)]


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.

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

["kiconloader.diff" (text/x-diff)]

Index: kiconloader.cpp
===================================================================
--- kiconloader.cpp	(revision 731865)
+++ kiconloader.cpp	(working copy)
@@ -597,30 +597,12 @@
 
 QString KIconLoaderPrivate::removeIconExtension(const QString &name) const
 {
-    int extensionLength=0;
-
-    QString ext = name.right(4);
-
-    if (ext == ".png" || ext == ".xpm" )
-      extensionLength=4;
-    else
-    {
-        static const QString &svgz_ext = KGlobal::staticQString(".svgz");
-        if (name.endsWith(svgz_ext))
-            extensionLength=5;
-        else if (ext == ".svg")
-            extensionLength=4;
+    if (name.endsWith(".png") || name.endsWith(".xpm") || name.endsWith(".svg")) {
+        return name.left(name.length() - 4);
+    } else if (name.endsWith(".svgz")) {
+        return name.left(name.length() - 5);
     }
 
-    if ( extensionLength > 0 )
-    {
-#ifndef NDEBUG
-        kDebug(264) << "Application " << KGlobal::mainComponent().componentName()
-                     << " loads icon " << name << " with extension." << endl;
-#endif
-
-        return name.left(name.length() - extensionLength);
-    }
     return name;
 }
 
@@ -854,8 +836,8 @@
     QString name = _name;
     QString path;
     QPixmap pix;
-    QString key;
-    bool absolutePath=false, favIconOverlay=false;
+    bool absolutePath = false;
+    bool favIconOverlay = false;
 
     // Special case for absolute path icons.
     if (name.startsWith("favicons/"))
@@ -863,16 +845,20 @@
        favIconOverlay = true;
        name = KStandardDirs::locateLocal("cache", name+".png");
     }
-    if (!QDir::isRelativePath(name)) absolutePath=true;
 
+    if (!name.isEmpty() && name[0] == '/') {
+        absolutePath = true;
+    }
+
     static const QString &str_unknown = KGlobal::staticQString("unknown");
 
     // Special case for "User" icons.
     if (group == KIconLoader::User)
     {
-        key = "$kicou_";
-        key += QString::number(size); key += '_';
-        key += name;
+        QString key;
+        key.reserve(200);
+        key.append("$kicou_");
+        key.append(name).append('_').append(QString::number(size));
         key.append(overlays.join("_")); // krazy:exclude=doublequote_chars
 
         if (d->mIconCache->find(key, pix, path_store)) {
@@ -948,20 +934,22 @@
 
     // Generate a unique cache key for the icon.
 
-    key = "$kico_";
-    key += name; key += '_';
-    key += QString::number(size); key += '_';
+    QString key;
+    key.reserve(100);
+    key.append("$kico_");
+    key.append(name).append('_').append(QString::number(size));
 
     QString overlayKey = overlays.join("_"); // krazy:exclude=doublequote_chars
     QString noEffectKey = key + overlayKey;
 
     if (group >= 0)
     {
-        key += d->mpEffect.fingerprint(group, state);
+        key.append(d->mpEffect.fingerprint(group, state));
         if (d->mpGroups[group].dblPixels)
-            key += QLatin1String(":dblsize");
-    } else
-        key += QLatin1String("noeffect");
+            key.append(QLatin1String(":dblsize"));
+    } else {
+        key.append(QLatin1String("noeffect"));
+    }
     key.append(overlayKey);
 
     // Is the icon in the cache?

[Attachment #6 (application/pgp-signature)]

[prev in list] [next in list] [prev in thread] [next in thread]