Re: bring speed back into KConfig
Jakub Stachowski <[email protected]> Sat, 19 Apr 2008 22:19:42 +0200
| Newsgroups | gmane.comp.kde.devel.optimize |
|---|---|
| Message-ID | <[email protected]> |
--Boundary-00=_ePlCI4uvLEg1wpd
Content-Type: text/plain;
charset="utf-8"
Content-Transfer-Encoding: quoted-printable
Content-Disposition: inline
Dnia pi=C4=85tek, 18 kwietnia 2008, Jakub Stachowski napisa=C5=82:
> Dnia czwartek, 17 kwietnia 2008, Olivier Goffart napisa=C5=82:
> > Le mardi 15 avril 2008, Dirk Mueller a =C3=A9crit=C2=A0:
> > > Hi,
[ ... ]
>
> I did some more optimizing to minimize copying data around. Instead of
> using QByteArray everywhere I added class BufferFragment with very similar
> (bare minimum used by parser) API, but operating on allocated earlier big
> buffer. 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
> - kconfig unit test pass
>
> BufferFragment class contains very short functions (most of them 1-3 line=
s)
> that could be inlined, so all definitions are in header file. Is it OK or
> separate .cpp file is necessary?
How nice to see traffic on kde-optimize again :-)
Here is second version of the patch with fixes to address comments:
Dirk Mueller:
a) I agree with Oswald that if reading whole file in one go might kill the=
=20
app, then storing all parsed keys in memory will definitely do it.=20
b) I added some comments to class and more interesting functions
c) OK, implicit conversion changed to toByteArray() function
Oswald Buddenhagen:
=2D I don't care one way or another about spaces around operators, but KCon=
fig=20
has them so I changed my code to match it.
=2D Changed BufferFragment& inout parameter to FragmentBuffer*
Lubos Lunak:
=2D Whole BufferFragment class got dumped into kconfigini_p.h and is now pa=
rt of=20
KConfigIniBackend
--Boundary-00=_ePlCI4uvLEg1wpd
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,8 +27,161 @@
#include <kconfigbackend.h>
#include <klockfile.h>
+#define isspace(str) ((str == ' ') || (str == '\t') || (str == '\r'))
+
class KConfigIniBackend : public KConfigBackend
{
+ private:
+ // This class provides wrapper around fragment of existing buffer (array of bytes).
+ // If underlying buffer gets deleted, all BufferFragment objects referencing it become invalid.
+ // Use toByteArray() to make deep copy of the buffer fragment.
+ //
+ // API is designed to subset of QByteArray methods with some changes:
+ // - trim() is like QByteArray.trimmed(), but it modifies curren object
+ // - truncateLeft() provides way to cut off beginning of the buffer
+ // - split() works more like strtok_r than QByteArray.split()
+ // - truncateLeft() and mid() require position argument to be valid
+
+ 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--;
+ }
+
+ // similar to strtok_r . On first call variable pointed by start should be set to 0.
+ // Each call will update *start to new starting position.
+ 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() == (int)len && memcmp(d,other.constData(),len) == 0);
+ }
+
+ bool operator!=(const QByteArray& other) const
+ {
+ return (other.size() != (int)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;
+ }
+
+ QByteArray toByteArray() const {
+ return QByteArray(d,len);
+ }
+
+ private:
+ char* d;
+ unsigned int len;
+ };
+
+
KLockFile::Ptr lockFile;
public:
@@ -61,7 +214,9 @@
KeyString = 1,
ValueString = 2
};
- static QByteArray printableToString(const QByteArray& aString, const QFile& file, int line);
+ // Warning: this modifies data in-place. Other BufferFragment objects referencing the same buffer
+ // fragment will get their data modified too.
+ 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)
@@ -88,13 +88,17 @@
bool groupOptionImmutable = false;
bool groupSkip = false;
- int lineNo=0;
+ 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 +133,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.toByteArray();
}
} while ((start = end + 2) <= line.length() && line.at(end + 1) == '[');
currentGroup = newGroup;
@@ -147,14 +153,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,17 +173,17 @@
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)
<< "Invalid entry (missing ']')";
goto next_line;
} else if (end > start + 1 && aKey.at(start + 1) == '$') { // found option(s)
- int i = start+2;
+ int i = start + 2;
while (i < end) {
switch (aKey.at(i)) {
case 'i':
@@ -188,8 +196,9 @@
break;
case 'd':
entryOptions |= KEntryMap::EntryDeleted;
- aKey = printableToString(aKey.left(start), file, lineNo);
- entryMap.setEntry(currentGroup, aKey, QByteArray(), entryOptions);
+ aKey = aKey.left(start);
+ printableToString(&aKey, file, lineNo);
+ entryMap.setEntry(currentGroup, aKey.toByteArray(), QByteArray(), entryOptions);
goto next_line;
default:
break;
@@ -203,17 +212,16 @@
goto next_line;
}
- locale = aKey.mid(start+1,end-start-1);
- rawKey = aKey.left(end+1);
+ 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 +229,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 +242,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.toByteArray(), line.toByteArray(), entryOptions);
}
next_line:
continue;
@@ -642,31 +651,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("");
+ if (aString->isEmpty() || aString->indexOf('\\')==-1)
+ return;
+ aString->trim();
+ int l = aString->length();
+ char *r = aString->data();
+ char *str=r;
- const char *str = aString.constData();
- int l = aString.length();
-
- // 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 +703,5 @@
}
}
}
- result.truncate(r - result.constData());
- return result;
+ aString->truncate(r - aString->constData());
}
--Boundary-00=_ePlCI4uvLEg1wpd
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=_ePlCI4uvLEg1wpd--