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

List:       kde-commits
Subject:    [kiconthemes] /: Greatly improve the performace of makeCacheKey, as it is a critical code path in ic
From:       Mark Gaiser <markg85 () gmail ! com>
Date:       2016-08-07 13:46:18
Message-ID: E1bWOPC-0002LW-Cb () code ! kde ! org
[Download RAW message or body]

Git commit f7adeb71823a7eaff3096f2ee05f99965444e0fc by Mark Gaiser.
Committed on 07/08/2016 at 13:44.
Pushed by markg into branch 'master'.

Greatly improve the performace of makeCacheKey, as it is a critical code path in icon \
lookup.

Before:
********* Start testing of KIconLoader_Benchmark *********
Config: Using QtTest library 5.7.0, Qt 5.7.0 (x86_64-little_endian-lp64 shared \
(dynamic) release build; by GCC 6.1.1 20160602) PASS   : \
KIconLoader_Benchmark::initTestCase() PASS   : \
KIconLoader_Benchmark::benchmarkExistingIcons() RESULT : \
KIconLoader_Benchmark::benchmarkExistingIcons():  0.13 msecs per iteration (total: \
68, iterations: 512) PASS   : \
KIconLoader_Benchmark::benchmarkNonExistingIcon_notCached() RESULT : \
KIconLoader_Benchmark::benchmarkNonExistingIcon_notCached():  7.1 msecs per iteration \
(total: 57, iterations: 8) PASS   : \
KIconLoader_Benchmark::benchmarkNonExistingIcon_cached() RESULT : \
KIconLoader_Benchmark::benchmarkNonExistingIcon_cached():  0.0033 msecs per iteration \
(total: 55, iterations: 16384) PASS   : KIconLoader_Benchmark::cleanupTestCase()
Totals: 5 passed, 0 failed, 0 skipped, 0 blacklisted, 789ms
********* Finished testing of KIconLoader_Benchmark *********

After:
********* Start testing of KIconLoader_Benchmark *********
Config: Using QtTest library 5.7.0, Qt 5.7.0 (x86_64-little_endian-lp64 shared \
(dynamic) release build; by GCC 6.1.1 20160602) PASS   : \
KIconLoader_Benchmark::initTestCase() PASS   : \
KIconLoader_Benchmark::benchmarkExistingIcons() RESULT : \
KIconLoader_Benchmark::benchmarkExistingIcons():  0.028 msecs per iteration (total: \
58, iterations: 2048) PASS   : \
KIconLoader_Benchmark::benchmarkNonExistingIcon_notCached() RESULT : \
KIconLoader_Benchmark::benchmarkNonExistingIcon_notCached():  7.1 msecs per iteration \
(total: 57, iterations: 8) PASS   : \
KIconLoader_Benchmark::benchmarkNonExistingIcon_cached() RESULT : \
KIconLoader_Benchmark::benchmarkNonExistingIcon_cached():  0.00096 msecs per \
iteration (total: 63, iterations: 65536) PASS   : \
                KIconLoader_Benchmark::cleanupTestCase()
Totals: 5 passed, 0 failed, 0 skipped, 0 blacklisted, 797ms
********* Finished testing of KIconLoader_Benchmark *********

The palletteId function also just returns the hex string representation of the 3 \
colors it's made up of. This is 24 characters (as opposed to 32 for a hash) and as a \
                bonus is readable as well.
REVIEW: 128619

M  +22   -0    autotests/kiconloader_unittest.cpp
M  +27   -3    src/kiconloader.cpp

http://commits.kde.org/kiconthemes/f7adeb71823a7eaff3096f2ee05f99965444e0fc

diff --git a/autotests/kiconloader_unittest.cpp b/autotests/kiconloader_unittest.cpp
index fbc4175..ee24533 100644
--- a/autotests/kiconloader_unittest.cpp
+++ b/autotests/kiconloader_unittest.cpp
@@ -30,6 +30,8 @@
 #include <KSharedConfig>
 #include <KConfigGroup>
 
+extern KICONTHEMES_EXPORT void uintToHex(uint32_t colorData, QChar *buffer);
+
 class KIconLoader_UnitTest : public QObject
 {
     Q_OBJECT
@@ -497,6 +499,26 @@ private Q_SLOTS:
         //that is the color we wrote in kdeglobals as text color?
         QCOMPARE(img.pixel(0,0), (uint)4294901760);
     }
+
+    void testUintToHex()
+    {
+        // HEX (ARGB format without the #): ff6496c8
+        QColor testColorNoAlpha(100, 150, 200);
+
+        // The ARGB string in which the composed hex value is stored.
+        QString argbHex(8, Qt::Uninitialized);
+
+        // Verify the ARGB hex (ff6496c8)
+        uintToHex(testColorNoAlpha.rgba(), argbHex.data());
+        QCOMPARE(argbHex, QString("ff6496c8"));
+
+        // HEX (ARGB format without the #): 7b6496c8
+        QColor testColorWithAlpha(100, 150, 200, 123);
+
+        // Test uintToHex to verify its ARGB output.
+        uintToHex(testColorWithAlpha.rgba(), argbHex.data());
+        QCOMPARE(argbHex, QString("7b6496c8"));
+    }
 };
 
 QTEST_MAIN(KIconLoader_UnitTest)
diff --git a/src/kiconloader.cpp b/src/kiconloader.cpp
index a89ba3c..5315ea4 100644
--- a/src/kiconloader.cpp
+++ b/src/kiconloader.cpp
@@ -115,11 +115,35 @@ struct PixmapWithPath {
     QString path;
 };
 
+/**
+ * Function to convert an uint32_t to AARRGGBB hex values.
+ *
+ * W A R N I N G !
+ * This function is for internal use!
+ */
+KICONTHEMES_EXPORT void uintToHex(uint32_t colorData, QChar *buffer)
+{
+    static const char hexLookup[] = "0123456789abcdef";
+    buffer += 7;
+    uchar *colorFields = reinterpret_cast<uchar*>(&colorData);
+
+    for (int i = 0; i < 4; i++) {
+        *buffer-- = hexLookup[*colorFields & 0xf];
+        *buffer-- = hexLookup[*colorFields >> 4];
+        colorFields++;
+    }
+}
+
 static QString paletteId(const QPalette &pal)
 {
-    const QString colorsString = pal.text().color().name() + \
                pal.highlight().color().name() + \
                pal.highlightedText().color().name();
-    //use md5 as speed is needed, not cryptographic security
-    return QCryptographicHash::hash(colorsString.toUtf8(), QCryptographicHash::Md5);
+    // 8 per color. We want 3 colors thus 8*3=24.
+    QString buffer(24, Qt::Uninitialized);
+
+    uintToHex(pal.text().color().rgba(), buffer.data());
+    uintToHex(pal.highlight().color().rgba(), buffer.data() + 8);
+    uintToHex(pal.highlightedText().color().rgba(), buffer.data() + 16);
+
+    return buffer;
 }
 
 /*** KIconThemeNode: A node in the icon theme dependancy tree. ***/


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