KDE/kdelibs

David Faure faure at kde.org
Wed Apr 1 22:26:44 CEST 2009


SVN commit 947971 by dfaure:

Performance: Add a hash in ksycoca with the parent mimetypes for each mimetype. Allows KMimeType::is() to skip loading full KMimeTypes.
The hash is loaded on demand to save memory in apps that don't really use mimetypes.
Before: 3.5 msec / 7,000,000 ticks / 5,021,498 instr. loads
After: 0.48 msec / 960,000 ticks / 791,404 instr. loads   => 7 times faster.
(QBENCHMARK is quite nice for measuring this kind of things)
CCMAIL: kde-optimize at kde.org


 M  +44 -55    kdecore/services/kmimetype.cpp  
 M  +0 -2      kdecore/services/kmimetype.h  
 M  +0 -4      kdecore/services/kmimetype_p.h  
 M  +26 -4     kdecore/services/kmimetypefactory.cpp  
 M  +18 -2     kdecore/services/kmimetypefactory.h  
 M  +1 -1      kdecore/sycoca/ksycoca.cpp  
 M  +8 -3      kdecore/tests/kmimetypetest.cpp  
 M  +17 -16    kded/kbuildmimetypefactory.cpp  


--- trunk/KDE/kdelibs/kdecore/services/kmimetype.cpp #947970:947971
@@ -60,11 +60,8 @@
 void KMimeTypePrivate::loadInternal( QDataStream& _str )
 {
     QString oldParentMimeTypeString;
-    _str >> m_lstPatterns >> oldParentMimeTypeString >> m_parentMimeTypes >> m_iconName;
-
-    // kde-4.0 compatibility. Remove in kde5.
-    if (!oldParentMimeTypeString.isEmpty() && m_parentMimeTypes.isEmpty())
-        m_parentMimeTypes.append(oldParentMimeTypeString);
+    QStringList oldParentMimeTypesList;
+    _str >> m_lstPatterns >> oldParentMimeTypeString >> oldParentMimeTypesList >> m_iconName;
 }
 
 /**
@@ -488,9 +485,9 @@
 void KMimeTypePrivate::save( QDataStream& _str )
 {
     KServiceTypePrivate::save( _str );
-    // Warning adding/removing fields here involves a binary incompatible change - update version
-    // number in ksycoca.h
-    _str << m_lstPatterns << QString() << m_parentMimeTypes << m_iconName;
+    // Warning adding fields here involves a binary incompatible change - update version
+    // number in ksycoca.h. Never remove fields.
+    _str << m_lstPatterns << QString() << QStringList() << m_iconName;
 }
 
 QVariant KMimeTypePrivate::property( const QString& _name ) const
@@ -570,42 +567,54 @@
     return d->comment(url);
 }
 
-QString KMimeType::parentMimeType() const
+static QString fallbackParent(const QString& mimeTypeName)
 {
-    Q_D(const KMimeType);
-
-    if (!d->m_parentMimeTypes.isEmpty())
-        return d->m_parentMimeTypes.first();
-    return d->fallbackParent();
-}
-
-QString KMimeTypePrivate::fallbackParent() const
-{
-    const QString myGroup = m_strName.left(m_strName.indexOf('/'));
+    const QString myGroup = mimeTypeName.left(mimeTypeName.indexOf('/'));
     // All text/* types are subclasses of text/plain.
-    if (myGroup == "text" && m_strName != "text/plain")
+    if (myGroup == "text" && mimeTypeName != "text/plain")
         return "text/plain";
     // All real-file mimetypes implicitly derive from application/octet-stream
     if (myGroup != "inode" &&
         // kde extensions
         myGroup != "all" && myGroup != "fonts" && myGroup != "print" && myGroup != "uri"
-        && m_strName != "application/octet-stream") {
+        && mimeTypeName != "application/octet-stream") {
         return "application/octet-stream";
     }
     return QString();
 }
 
-bool KMimeTypePrivate::inherits(const QString& mime) const
+static QStringList parentMimeTypesList(const QString& mimeTypeName)
 {
-    if (m_strName == mime) {
-        return true;
+    QStringList parents = KMimeTypeFactory::self()->parents(mimeTypeName);
+    if (parents.isEmpty()) {
+        const QString myParent = fallbackParent(mimeTypeName);
+        if (!myParent.isEmpty())
+            parents.append(myParent);
     }
-    foreach( const QString& parent, parentMimeTypes() ) {
-        KMimeType::Ptr parentMime = KMimeTypeFactory::self()->findMimeTypeByName(parent);
-        if (!parentMime) // error
-            return false;
-        if (parentMime->d_func()->inherits(mime)) // recurse
+    return parents;
+}
+
+QString KMimeType::parentMimeType() const
+{
+    Q_D(const KMimeType);
+
+    const QStringList parents = parentMimeTypesList(d->m_strName);
+    if (!parents.isEmpty())
+        return parents.first();
+    return QString();
+}
+
+bool KMimeTypePrivate::inherits(const QString& mime) const
+{
+    QStack<QString> toCheck;
+    toCheck.push(m_strName);
+    while (!toCheck.isEmpty()) {
+        const QString current = toCheck.pop();
+        if (current == mime)
             return true;
+        Q_FOREACH(const QString& parent, parentMimeTypesList(current)) {
+            toCheck.push(parent);
+        }
     }
     return false;
 }
@@ -624,23 +633,12 @@
 QStringList KMimeType::parentMimeTypes() const
 {
     Q_D(const KMimeType);
-    return d->parentMimeTypes();
+    return parentMimeTypesList(d->m_strName);
 }
 
-QStringList KMimeTypePrivate::parentMimeTypes() const
+static void collectParentMimeTypes(const QString& mime, QStringList& allParents)
 {
-    QStringList parents = m_parentMimeTypes;
-    if (parents.isEmpty()) {
-        const QString myParent = fallbackParent();
-        if (!myParent.isEmpty())
-            parents.append(myParent);
-    }
-    return parents;
-}
-
-void KMimeTypePrivate::collectParentMimeTypes(QStringList& allParents) const
-{
-    QStringList parents = parentMimeTypes();
+    QStringList parents = parentMimeTypesList(mime);
     Q_FOREACH(const QString& parent, parents) {
         // I would use QSet, but since order matters I better not
         if (!allParents.contains(parent))
@@ -649,19 +647,18 @@
     // We want a breadth-first search, so that the least-specific parent (octet-stream) is last
     // This means iterating twice, unfortunately.
     Q_FOREACH(const QString& parent, parents) {
-        KMimeType::Ptr parentMime = KMimeTypeFactory::self()->findMimeTypeByName(parent);
-        if (parentMime)
-            parentMime->d_func()->collectParentMimeTypes(allParents);
+        collectParentMimeTypes(parent, allParents);
     }
 }
 
 QStringList KMimeType::allParentMimeTypes() const
 {
+    Q_D(const KMimeType);
     QStringList allParents;
     const QString canonical = KMimeTypeFactory::self()->resolveAlias(name());
     if (!canonical.isEmpty())
         allParents.append(canonical);
-    d_func()->collectParentMimeTypes(allParents);
+    collectParentMimeTypes(d->m_strName, allParents);
     return allParents;
 }
 
@@ -690,18 +687,10 @@
     d->m_lstPatterns = patterns;
 }
 
-void KMimeType::setParentMimeType(const QString& parent)
-{
-    Q_D(KMimeType);
-    // kbuildmimetypefactory calls this multiple times, for each parent mimetype
-    d->m_parentMimeTypes.append(parent);
-}
-
 void KMimeType::internalClearData()
 {
     Q_D(KMimeType);
     // Clear the data that KBuildMimeTypeFactory is going to refill - and only that data.
-    d->m_parentMimeTypes.clear();
     d->m_lstPatterns.clear();
 }
 
--- trunk/KDE/kdelibs/kdecore/services/kmimetype.h #947970:947971
@@ -406,8 +406,6 @@
     /// @internal for kbuildsycoca
     void setPatterns(const QStringList& patterns);
     /// @internal for kbuildsycoca
-    void setParentMimeType(const QString& parent);
-    /// @internal for kbuildsycoca
     void internalClearData();
     /// @internal for kbuildsycoca
     void setUserSpecifiedIcon(const QString& icon);
--- trunk/KDE/kdelibs/kdecore/services/kmimetype_p.h #947970:947971
@@ -57,12 +57,8 @@
     }
 
     bool inherits(const QString& mime) const;
-    QString fallbackParent() const;
-    QStringList parentMimeTypes() const;
-    void collectParentMimeTypes(QStringList&) const;
 
     QStringList m_lstPatterns;
-    QStringList m_parentMimeTypes; // shared-mime-info supports multiple inheritance
     QString m_iconName; // user-specified
     // For any new field here, add it to loadInternal() and save(), for persistence.
 
--- trunk/KDE/kdelibs/kdecore/services/kmimetypefactory.cpp #947970:947971
@@ -1,6 +1,6 @@
 /*  This file is part of the KDE libraries
  *  Copyright (C) 1999 Waldo Bastian <bastian at kde.org>
- *  Copyright (C) 2006 David Faure <faure at kde.org>
+ *  Copyright (C) 2006-2009 David Faure <faure at kde.org>
  *
  *  This library is free software; you can redistribute it and/or
  *  modify it under the terms of the GNU Library General Public
@@ -31,6 +31,7 @@
     : KSycocaFactory( KST_KMimeTypeFactory ),
       m_highWeightPatternsLoaded(false),
       m_lowWeightPatternsLoaded(false),
+      m_parentsMapLoaded(false),
       m_magicFilesParsed(false)
 {
     kMimeTypeFactoryInstance->instanceCreated(this);
@@ -48,6 +49,8 @@
         // that's the old m_otherPatternOffset, kept for compat but unused
 
         // alias map
+        // TODO: to save time in apps that don't need this, we could
+        // do like the parents hash (move it, store offset, and load it delayed).
         qint32 n;
         (*str) >> n;
         QString str1, str2;
@@ -61,11 +64,15 @@
         m_highWeightPatternOffset = i;
         (*str) >> i;
         m_lowWeightPatternOffset = i;
+        (*str) >> i;
+        m_parentsMapOffset = i;
 
         const int saveOffset = str->device()->pos();
         // Init index tables
         m_fastPatternDict = new KSycocaDict(str, m_fastPatternOffset);
         str->device()->seek(saveOffset);
+    } else {
+        m_parentsMapLoaded = true;
     }
 }
 
@@ -88,7 +95,7 @@
 
     QString name = _name;
     if (options & KMimeType::ResolveAliases) {
-        QMap<QString, QString>::const_iterator it = m_aliases.constFind(_name);
+        AliasesMap::const_iterator it = m_aliases.constFind(_name);
         if (it != m_aliases.constEnd())
             name = *it;
     }
@@ -388,9 +395,24 @@
     return result;
 }
 
-QMap<QString, QString>& KMimeTypeFactory::aliases()
+QStringList KMimeTypeFactory::parents(const QString& mime)
 {
-    return m_aliases;
+    if (!m_parentsMapLoaded) {
+        m_parentsMapLoaded = true;
+        Q_ASSERT(m_parents.isEmpty());
+        QDataStream* str = stream();
+        str->device()->seek(m_parentsMapOffset);
+        qint32 n;
+        (*str) >> n;
+        QString str1, str2;
+        for(;n;n--) {
+            KSycocaEntry::read(*str, str1);
+            KSycocaEntry::read(*str, str2);
+            m_parents.insert(str1, str2.split('|'));
+        }
+
+    }
+    return m_parents.value(mime);
 }
 
 #include <arpa/inet.h> // for ntohs
--- trunk/KDE/kdelibs/kdecore/services/kmimetypefactory.h #947970:947971
@@ -66,6 +66,11 @@
      */
     QString resolveAlias(const QString& mime) const;
 
+    /**
+     * Returns the list of parents for a given mimetype
+     */
+    QStringList parents(const QString& mime);
+
 private: // only for KMimeType
     friend class KMimeType;
     friend class KMimeFileParserTest;
@@ -98,11 +103,18 @@
     bool checkMimeTypes();
 
 protected:
+    typedef QHash<QString, QString> AliasesMap;
     /**
      * @internal for kbuildsycoca
      */
-    QMap<QString, QString>& aliases();
+    AliasesMap& aliases() { return m_aliases; }
 
+    typedef QHash<QString, QStringList> ParentsMap;
+    /**
+     * @internal for kbuildsycoca
+     */
+    ParentsMap& parentsMap() { return m_parents; }
+
 public:
     /**
      * @return all mimetypes
@@ -135,6 +147,8 @@
     int m_highWeightPatternOffset;
     /// @internal
     int m_lowWeightPatternOffset;
+    /// @internal
+    int m_parentsMapOffset;
 
     KSycocaDict* m_fastPatternDict;
 
@@ -171,10 +185,12 @@
     OtherPatternList m_highWeightPatterns;
     OtherPatternList m_lowWeightPatterns;
 
-    QMap<QString, QString> m_aliases; // alias -> canonicalName
+    AliasesMap m_aliases; // alias -> canonicalName
+    ParentsMap m_parents;
 
     bool m_highWeightPatternsLoaded;
     bool m_lowWeightPatternsLoaded;
+    bool m_parentsMapLoaded;
     bool m_magicFilesParsed;
     QList<KMimeMagicRule> m_magicRules;
 
--- trunk/KDE/kdelibs/kdecore/sycoca/ksycoca.cpp #947970:947971
@@ -55,7 +55,7 @@
  * If the existing file is outdated, it will not get read
  * but instead we'll ask kded to regenerate a new one...
  */
-#define KSYCOCA_VERSION 142
+#define KSYCOCA_VERSION 143
 
 /**
  * Sycoca file name, used internally (by kbuildsycoca)
--- trunk/KDE/kdelibs/kdecore/tests/kmimetypetest.cpp #947970:947971
@@ -534,8 +534,7 @@
     KMimeType::Ptr mime = KMimeType::mimeType("text/x-chdr");
     QVERIFY(mime);
     QTime dt; dt.start();
-    const int numIterations = 200; // 2000 for a better test
-    for (int i = 0; i < numIterations; ++i) {
+    QBENCHMARK {
         QString match;
         foreach (const QString& mt, mimeTypes) {
             if (mime->is(mt)) {
@@ -546,7 +545,13 @@
         }
         QCOMPARE(match, QString("text/plain"));
     }
-    qDebug() << "Calling is()" << mimeTypes.count() * numIterations << "times took" << dt.elapsed() << "msecs";
+    // Results on David's machine (April 2009):
+    // With the KMimeType::is() code that loaded every parent KMimeType:
+    // 3.5 msec / 7,000,000 ticks / 5,021,498 instr. loads per iteration
+    // After the QHash for parent mimetypes in ksycoca, removing the need to load full mimetypes:
+    // 0.57 msec / 1,115,000 ticks / 938,356 instr. loads per iteration
+    // After converting the QMap for aliases into a QHash too:
+    // 0.48 msec / 960,000 ticks / 791,404 instr. loads per iteration
 }
 
 // Helper method for all the trader tests
--- trunk/KDE/kdelibs/kded/kbuildmimetypefactory.cpp #947970:947971
@@ -62,7 +62,7 @@
 
     QString name = _name;
     if (options & KMimeType::ResolveAliases) {
-        QMap<QString, QString>::const_iterator it = aliases().constFind(_name);
+        AliasesMap::const_iterator it = aliases().constFind(_name);
         if (it != aliases().constEnd())
             name = *it;
     }
@@ -185,17 +185,19 @@
     // This header is read by KMimeTypeFactory's constructor
     str << (qint32) m_fastPatternOffset;
     str << (qint32) m_oldOtherPatternOffset;
-    const QMap<QString, QString>& aliasMap = aliases();
+    const AliasesMap& aliasMap = aliases();
     str << (qint32) aliasMap.count();
-    for (QMap<QString, QString>::const_iterator it = aliasMap.begin(); it != aliasMap.end(); ++it) {
+    for (AliasesMap::const_iterator it = aliasMap.begin(); it != aliasMap.end(); ++it) {
         str << it.key() << it.value();
     }
     str << (qint32) m_highWeightPatternOffset;
     str << (qint32) m_lowWeightPatternOffset;
+    str << (qint32) m_parentsMapOffset;
 }
 
 void KBuildMimeTypeFactory::parseSubclassFile(const QString& fileName)
 {
+    ParentsMap& parentsMap = this->parentsMap();
     QFile qfile( fileName );
     //kDebug(7021) << "Now parsing" << fileName;
     if (qfile.open(QIODevice::ReadOnly)) {
@@ -215,7 +217,8 @@
             else {
                 const QString parentTypeName = line.mid(pos+1);
                 Q_ASSERT(!parentTypeName.isEmpty());
-                derivedType->setParentMimeType(parentTypeName);
+                //derivedType->setParentMimeType(parentTypeName);
+                parentsMap[derivedTypeName].append(parentTypeName);
             }
         }
     }
@@ -223,6 +226,7 @@
 
 void KBuildMimeTypeFactory::parseAliasFile(const QString& fileName)
 {
+    AliasesMap& aliasMap = aliases();
     QFile qfile( fileName );
     //kDebug(7021) << "Now parsing" << fileName;
     if (qfile.open(QIODevice::ReadOnly)) {
@@ -239,7 +243,7 @@
             const QString parentTypeName = line.mid(pos+1);
             Q_ASSERT(!aliasTypeName.isEmpty());
             Q_ASSERT(!parentTypeName.isEmpty());
-            aliases().insert(aliasTypeName, parentTypeName);
+            aliasMap.insert(aliasTypeName, parentTypeName);
         }
     }
 }
@@ -281,6 +285,13 @@
 
     savePatternLists(str);
 
+    m_parentsMapOffset = str.device()->pos();
+    ParentsMap& parentsMap = this->parentsMap();
+    str << (qint32) parentsMap.count();
+    for (ParentsMap::const_iterator it = parentsMap.begin(); it != parentsMap.end(); ++it) {
+        str << it.key() << it.value().join("|");
+    }
+
     int endOfFactoryData = str.device()->pos();
 
     // Update header (pass #3)
@@ -353,17 +364,7 @@
     }
     str << QString(""); // end of list marker (has to be a string !)
 
-    // For compat with kde-4.1 kdecore: write the old "other patterns" thing
+    // For compat with kde-4.1 kdecore: write the old "other patterns" thing, but empty
     m_oldOtherPatternOffset = str.device()->pos();
-    // TODO: remove once 4.2 is released.
-    Q_FOREACH(const OtherPattern& op, highWeightPatternOffset) {
-        str << op.pattern;
-        str << (qint32)op.offset;
-    }
-    Q_FOREACH(const OtherPattern& op, lowWeightPatternOffset) {
-        str << op.pattern;
-        str << (qint32)op.offset;
-    }
-    // END TODO
     str << QString(""); // end of list marker (has to be a string !)
 }


More information about the Kde-optimize mailing list