Re: bring speed back into KConfig

Jakub Stachowski <[email protected]> Fri, 18 Apr 2008 00:24:58 +0200
Newsgroups gmane.comp.kde.devel.optimize,gmane.comp.kde.devel.core
Message-ID <[email protected]>
--Boundary-00=_748BIY2NaLAccEe
Content-Type: text/plain;
  charset="utf-8"
Content-Transfer-Encoding: quoted-printable
Content-Disposition: inline

Dnia czwartek, 17 kwietnia 2008, Olivier Goffart napisa=C5=82:
> Le mardi 15 avril 2008, Dirk Mueller a =C3=A9crit=C2=A0:
> > Hi,
> >
> > the "knotify4 eats cpu" bugreport seems to be highly popular, so the
> > reason for that seems to be the incredible performance loss in KConfigI=
ni
> >  due to the "kconfig refactoring branch" being merged last autumn by
> > Andreas Pakulat.
>
> Oh, i though it was a problem in the xine backend, which is why i did not
> spend much attention to it.
> I may have a closer look.

I did some more optimizing to  minimize copying data around. Instead of usi=
ng=20
QByteArray everywhere I added class BufferFragment with very similar (bare=
=20
minimum used by parser) API, but operating on allocated earlier big buffer.=
=20
like left(), trim(), mid(), etc. are only pointer and int operations.

Results:
 - 500x parsing of kwin.notifyrc takes 1.3s instead of 5.8s
 - KConfig from KDE3 takes 1.4s=20
 - kconfig unit test pass

BufferFragment class contains very short functions (most of them 1-3 lines)=
=20
that could be inlined, so all definitions are in header file. Is it OK or=20
separate .cpp file is necessary?

--Boundary-00=_748BIY2NaLAccEe
Content-Type: text/x-diff;
  charset="utf-8";
  name="kconfig-opt.patch"
Content-Transfer-Encoding: 7bit
Content-Disposition: attachment;
	filename="kconfig-opt.patch"

Index: kconfigini_p.h
===================================================================
--- kconfigini_p.h	(wersja 797091)
+++ kconfigini_p.h	(kopia robocza)
@@ -27,6 +27,8 @@
 #include <kconfigbackend.h>
 #include <klockfile.h>
 
+
+class BufferFragment;
 class KConfigIniBackend : public KConfigBackend
 {
     KLockFile::Ptr lockFile;
@@ -61,7 +63,7 @@
         KeyString = 1,
         ValueString = 2
     };
-    static QByteArray printableToString(const QByteArray& aString, const QFile& file, int line);
+    static void printableToString(BufferFragment& aString, const QFile& file, int line);
     static QByteArray stringToPrintable(const QByteArray& aString, StringType type);
     static char charFromHex(const char *str, const QFile& file, int line);
     static QString warningProlog(const QFile& file, int line);
Index: kconfigini.cpp
===================================================================
--- kconfigini.cpp	(wersja 797721)
+++ kconfigini.cpp	(kopia robocza)
@@ -28,6 +28,7 @@
 
 #include "kconfig.h"
 #include "kconfigbackend.h"
+#include "bufferfragment_p.h"
 #include "kconfigini_p.h"
 #include "kconfigdata.h"
 #include <ksavefile.h>
@@ -90,11 +91,15 @@
 
     int lineNo=0;
     // on systems using \r\n as end of line, \r will be taken care of by 
-    // trimmed() below
-    QList<QByteArray> lines=file.readAll().split('\n');
-    for (int i=0;i<lines.size();i++) {
-        QByteArray& line=lines[i];
-        line=line.trimmed();
+    // trim() below
+    QByteArray buffer=file.readAll();
+    BufferFragment contents(buffer.data(), buffer.size());
+    unsigned int len=contents.length();
+    unsigned int startOfLine=0;
+
+    while (startOfLine<len) {
+        BufferFragment line=contents.split('\n', startOfLine);
+        line.trim();
         lineNo++;
 
         // skip empty lines and lines beginning with '#'
@@ -129,7 +134,9 @@
                 else {
                     if (!newGroup.isEmpty())
                         newGroup += '\x1d';
-                    newGroup += printableToString(line.mid(start, end - start), file, lineNo);
+                    BufferFragment namePart=line.mid(start, end - start);
+                    printableToString(namePart, file, lineNo);
+                    newGroup += namePart;
                 }
             } while ((start = end + 2) <= line.length() && line.at(end + 1) == '[');
             currentGroup = newGroup;
@@ -147,14 +154,16 @@
             if (groupSkip && !bDefault)
                 continue; // skip entry
 
-            QByteArray aKey;
+            BufferFragment aKey;
             int eqpos = line.indexOf('=');
             if (eqpos < 0) {
                 aKey = line;
                 line.clear();
             } else {
-                aKey = line.left(eqpos).trimmed();
-                line.remove(0, eqpos + 1);
+                BufferFragment temp=line.left(eqpos);
+                temp.trim();
+                aKey = temp;
+                line.truncateLeft(eqpos + 1);
             }
             if (aKey.isEmpty()) {
                 qWarning() << warningProlog(file, lineNo) << "Invalid entry (empty key)";
@@ -165,10 +174,10 @@
             if (groupOptionImmutable)
                 entryOptions |= KEntryMap::EntryImmutable;
 
-            QByteArray locale;
-            QByteArray rawKey;
+            BufferFragment locale;
+            BufferFragment rawKey;
             int start;
-            while ((start = aKey.indexOf('[')) >= 0) {
+            while ((start = aKey.lastIndexOf('[')) >= 0) {
                 int end = aKey.indexOf(']', start);
                 if (end < 0) {
                     qWarning() << warningProlog(file, lineNo)
@@ -188,7 +197,8 @@
                                 break;
                             case 'd':
                                 entryOptions |= KEntryMap::EntryDeleted;
-                                aKey = printableToString(aKey.left(start), file, lineNo);
+                                aKey = aKey.left(start);
+                                printableToString(aKey, file, lineNo);
                                 entryMap.setEntry(currentGroup, aKey, QByteArray(), entryOptions);
                                 goto next_line;
                             default:
@@ -206,14 +216,13 @@
                     locale = aKey.mid(start+1,end-start-1);
                     rawKey = aKey.left(end+1);
                 }
-                aKey.remove(start, end-start+1);
+                aKey.truncate(start);
             }
-
             if (eqpos < 0) { // Do this here after [$d] was checked
                 qWarning() << warningProlog(file, lineNo) << "Invalid entry (missing '=')";
                 continue;
             }
-            aKey = printableToString(aKey, file, lineNo);
+            printableToString(aKey, file, lineNo);
             if (!locale.isEmpty()) {
                 if (locale != currentLocale) {
                     // backward compatibility. C == en_US
@@ -221,12 +230,12 @@
                         if (merging){
                             entryOptions |= KEntryMap::EntryRawKey;
                             aKey = rawKey; // store as unprocessed key
-                            locale = QByteArray();
+                            locale = BufferFragment();
                         } else
                             goto next_line; // skip this entry if we're not merging
                     }
                 }
-            }
+            } 
 
             if (options&ParseGlobal)
                 entryOptions |= KEntryMap::EntryGlobal;
@@ -234,7 +243,8 @@
                 entryOptions |= KEntryMap::EntryDefault;
             if (!locale.isNull())
                 entryOptions |= KEntryMap::EntryLocalized;
-            entryMap.setEntry(currentGroup, aKey, printableToString(line, file, lineNo), entryOptions);
+            printableToString(line, file, lineNo);
+            entryMap.setEntry(currentGroup, aKey, line, entryOptions);
         }
 next_line:
         continue;
@@ -642,31 +652,18 @@
     return char(ret);
 }
 
-QByteArray KConfigIniBackend::printableToString(const QByteArray& aString, const QFile& file, int line)
+void KConfigIniBackend::printableToString(BufferFragment& aString, const QFile& file, int line)
 {
-    if (aString.isEmpty())
-        return QByteArray("");
-
-    const char *str = aString.constData();
+    if (aString.isEmpty() || aString.indexOf('\\')==-1) 
+        return;
+    aString.trim();
     int l = aString.length();
+    char *r = aString.data();
+    char *str=r;
 
-    // Strip leading white-space.
-    while((l > 0) && ((*str == ' ') || (*str == '\t') || (*str == '\r'))) {
-        str++; l--;
-     }
-
-
-    // Strip trailing white-space.
-    while((l > 0) && ((str[l-1] == ' ') || (str[l-1] == '\t') || (str[l-1] == '\r'))) {
-        l--;
-    }
-
-    QByteArray result(l, 0);
-    char *r = result.data();
-
     for(int i = 0; i < l; i++, r++) {
-        if (str[i] != '\\') {
-            *r = str[i];
+        if (str[i]!= '\\') {
+            *r=str[i];
         } else {
             // Probable escape sequence
             i++;
@@ -707,6 +704,5 @@
             }
         }
     }
-    result.truncate(r - result.constData());
-    return result;
+    aString.truncate(r - aString.constData());
 }
Index: bufferfragment_p.h
===================================================================
--- bufferfragment_p.h	(wersja 0)
+++ bufferfragment_p.h	(wersja 0)
@@ -0,0 +1,166 @@
+/*
+   This file is part of the KDE libraries
+   Copyright (c) 2008 Jakub Stachowski <[email protected]>
+   Portions copyright (c) 1997 Matthias Kalle Dalheimer <[email protected]>
+
+   This library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Library General Public
+   License as published by the Free Software Foundation; either
+   version 2 of the License, or (at your option) any later version.
+
+   This library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Library General Public License for more details.
+
+   You should have received a copy of the GNU Library General Public License
+   along with this library; see the file COPYING.LIB.  If not, write to
+   the Free Software Foundation, Inc., 51 Franklin Street, Fifth Floor,
+   Boston, MA 02110-1301, USA.
+*/
+
+#ifndef BUFFERFRAGMENT_H
+#define BUFFERFRAGMENT_H
+
+#include <qbytearray.h>
+
+#define isspace(str) ((str == ' ') || (str == '\t') || (str == '\r'))
+
+class BufferFragment 
+{
+public:
+    
+    BufferFragment() : d(0), len(0) 
+    {
+    }
+    
+    BufferFragment(char* buf, int size) : d(buf), len(size) 
+    {
+    }
+    
+    int length() const 
+    {
+        return len;
+    }
+    
+    char at(unsigned int i) const 
+    {
+        Q_ASSERT(i<len);
+        return d[i];
+    }
+    
+    void clear() 
+    {
+        len=0;
+    }
+    
+    const char* constData() const 
+    {
+        return d;
+    }
+    
+    char* data() const 
+    {
+        return d;
+    }
+    
+    void trim() 
+    {
+        while (isspace(*d) && len>0) {
+            d++;
+            len--;
+        }
+        while (len>0 && isspace(d[len-1])) len--;
+    }
+    
+    BufferFragment split(char c, unsigned int &start) 
+    {
+        while (start<len) {
+            int end=indexOf(c, start);
+            if (end==-1) end=len;
+            BufferFragment line(d+start, end-start);
+            start=end+1;
+            return line;
+        }
+        return BufferFragment();
+    }
+        
+    bool isEmpty() const 
+    {
+        return (len==0);
+    }
+    
+    BufferFragment left(unsigned int size) const 
+    {
+        return BufferFragment(d, qMin(size,len));
+    }
+    
+    void truncateLeft(unsigned int size) 
+    {
+        Q_ASSERT(size<=len);
+        d+=size;
+        len-=size;
+    }
+    
+    void truncate(unsigned int pos) 
+    {
+        if (pos<len) len=pos;
+    }
+    
+    bool isNull() const 
+    {
+        return (d==0);
+    }
+            
+    BufferFragment mid(unsigned int pos, int length=-1) const 
+    {
+        Q_ASSERT(pos<len);
+        int size=length;
+        if (length==-1 || (pos+length)>len) {
+            size=len-pos;
+        }
+        return BufferFragment(d+pos, size);
+    }
+    
+    bool operator==(const QByteArray& other) const
+    {
+        return (other.size()==len && memcmp(d,other.constData(),len) == 0);
+    }
+    
+    bool operator!=(const QByteArray& other) const 
+    {
+        return (other.size()!=len || memcmp(d,other.constData(),len) != 0);
+    }
+        
+    int indexOf(char c, unsigned int from=0) const 
+    {
+        const char* cursor=d+from-1;
+        const char* end=d+len;
+        while ( ++cursor < end) 
+            if (*cursor==c) 
+                return cursor-d; 
+        return -1;
+    }
+
+    int lastIndexOf(char c) const 
+    {
+        int from=len-1;
+        while (from>=0) 
+            if (d[from]==c) {
+                return from; 
+            } else {
+                from--;
+            }
+        return -1;
+    }
+    
+    operator QByteArray() {
+        return QByteArray(d,len);
+    }
+    
+private:
+    char* d;
+    unsigned int len;
+};
+
+#endif

--Boundary-00=_748BIY2NaLAccEe
Content-Type: text/plain; charset="us-ascii"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Content-Disposition: inline

_______________________________________________
Kde-optimize mailing list
[email protected]
https://mail.kde.org/mailman/listinfo/kde-optimize

--Boundary-00=_748BIY2NaLAccEe--