Rewrite QRingBuffer

QRingBuffer is a fully inlined class used in many I/O classes.
So, it must be as fast and small as possible. To this end, a lot of
unnecessary special cases were replaced by generic structures.

Change-Id: Ic189ced3b200924da158ce511d69d324337d01b6
Reviewed-by: Oswald Buddenhagen <oswald.buddenhagen@digia.com>

src/corelib/tools/qringbuffer_p.h

@@ -61,9 +61,9 @@ QT_BEGIN_NAMESPACE
 class QRingBuffer
 {
 public:
-    explicit inline QRingBuffer(int growth = 4096) : basicBlockSize(growth) {
+    explicit inline QRingBuffer(int growth = 4096) :
-        buffers << QByteArray();
+        head(0), tail(0), tailBuffer(0), basicBlockSize(growth), bufferSize(0) {
-        clear();
+        buffers.append(QByteArray());
     }
 
     inline int nextDataBlockSize() const {
@@ -78,115 +78,65 @@ public:
     // the out-variable length will contain the amount of bytes readable
     // from there, e.g. the amount still the same QByteArray
     inline const char *readPointerAtPosition(qint64 pos, qint64 &length) const {
-        if (buffers.isEmpty()) {
+        if (pos >= 0) {
-            length = 0;
+            pos += head;
-            return 0;
+            for (int i = 0; i < buffers.size(); ++i) {
-        }
+                length = (i == tailBuffer ? tail : buffers[i].size());
-
+                if (length > pos) {
-        if (pos >= bufferSize) {
+                    length -= pos;
-            length = 0;
+                    return buffers[i].constData() + pos;
-            return 0;
+                }
-        }
+                pos -= length;
-
-        // special case: it is in the first buffer
-        int nextDataBlockSizeValue = nextDataBlockSize();
-        if (pos < nextDataBlockSizeValue) {
-            length = nextDataBlockSizeValue - pos;
-            return buffers.at(0).constData() + head + pos;
-        }
-
-        // special case: we only had one buffer and tried to read over it
-        if (buffers.length() == 1) {
-            length = 0;
-            return 0;
-        }
-
-        // skip the first
-        pos -= nextDataBlockSizeValue;
-
-        // normal case: it is somewhere in the second to the-one-before-the-tailBuffer
-        for (int i = 1; i < tailBuffer; i++) {
-            if (pos >= buffers[i].size()) {
-                pos -= buffers[i].size();
-                continue;
             }
-
-            length = buffers[i].length() - pos;
-            return buffers[i].constData() + pos;
         }
 
-        // it is in the tail buffer
+        length = 0;
-        length = tail - pos;
+        return 0;
-        return buffers[tailBuffer].constData() + pos;
     }
 
     inline void free(int bytes) {
-        bufferSize -= bytes;
+        while (bytes > 0) {
-        if (bufferSize < 0)
+            int blockSize = buffers.first().size() - head;
-            bufferSize = 0;
 
-        for (;;) {
+            if (tailBuffer == 0 || blockSize > bytes) {
-            int nextBlockSize = nextDataBlockSize();
+                bufferSize -= bytes;
-            if (bytes < nextBlockSize) {
+                if (bufferSize <= 0)
-                head += bytes;
+                    clear(); // try to minify/squeeze us
-                if (head == tail && tailBuffer == 0)
+                else
-                    head = tail = 0;
+                    head += bytes;
-                break;
+                return;
             }
 
-            bytes -= nextBlockSize;
+            bufferSize -= blockSize;
-            if (buffers.count() == 1) {
+            bytes -= blockSize;
-                if (buffers.at(0).size() != basicBlockSize)
+            buffers.removeFirst();
-                    buffers[0].resize(basicBlockSize);
-                head = tail = 0;
-                tailBuffer = 0;
-                break;
-            }
-
-            buffers.removeAt(0);
             --tailBuffer;
             head = 0;
         }
-
-        if (isEmpty())
-            clear(); // try to minify/squeeze us
     }
 
     inline char *reserve(int bytes) {
-        // if this is a fresh empty QRingBuffer
+        if (bytes <= 0)
-        if (bufferSize == 0) {
+            return 0;
-            buffers[0].resize(qMax(basicBlockSize, bytes));
+
-            bufferSize += bytes;
+        // if need buffer reallocation
-            tail = bytes;
+        if (tail + bytes > buffers.last().size()) {
-            return buffers[tailBuffer].data();
+            if (tail >= basicBlockSize) {
+                // shrink this buffer to its current size
+                buffers.last().resize(tail);
+
+                // create a new QByteArray
+                buffers.append(QByteArray());
+                ++tailBuffer;
+                tail = 0;
+            }
+            buffers.last().resize(qMax(basicBlockSize, tail + bytes));
         }
 
+        char *writePtr = buffers.last().data() + tail;
         bufferSize += bytes;
-
+        tail += bytes;
-        // if there is already enough space, simply return.
+        return writePtr;
-        if (tail + bytes <= buffers.at(tailBuffer).size()) {
-            char *writePtr = buffers[tailBuffer].data() + tail;
-            tail += bytes;
-            return writePtr;
-        }
-
-        // if our buffer isn't half full yet, simply resize it.
-        if (tail < buffers.at(tailBuffer).size() / 2) {
-            buffers[tailBuffer].resize(tail + bytes);
-            char *writePtr = buffers[tailBuffer].data() + tail;
-            tail += bytes;
-            return writePtr;
-        }
-
-        // shrink this buffer to its current size
-        buffers[tailBuffer].resize(tail);
-
-        // create a new QByteArray with the right size
-        buffers << QByteArray();
-        ++tailBuffer;
-        buffers[tailBuffer].resize(qMax(basicBlockSize, bytes));
-        tail = bytes;
-        return buffers[tailBuffer].data();
     }
 
     inline void truncate(int pos) {
@@ -195,33 +145,22 @@ public:
     }
 
     inline void chop(int bytes) {
-        bufferSize -= bytes;
+        while (bytes > 0) {
-        if (bufferSize < 0)
+            if (tailBuffer == 0 || tail > bytes) {
-            bufferSize = 0;
+                bufferSize -= bytes;
-
+                if (bufferSize <= 0)
-        for (;;) {
+                    clear(); // try to minify/squeeze us
-            // special case: head and tail are in the same buffer
+                else
-            if (tailBuffer == 0) {
+                    tail -= bytes;
-                tail -= bytes;
-                if (tail <= head)
-                    tail = head = 0;
-                return;
-            }
-
-            if (bytes <= tail) {
-                tail -= bytes;
                 return;
             }
 
+            bufferSize -= tail;
             bytes -= tail;
-            buffers.removeAt(tailBuffer);
+            buffers.removeLast();
-
             --tailBuffer;
-            tail = buffers.at(tailBuffer).size();
+            tail = buffers.last().size();
         }
-
-        if (isEmpty())
-            clear(); // try to minify/squeeze us
     }
 
     inline bool isEmpty() const {
@@ -245,11 +184,11 @@ public:
         --head;
         if (head < 0) {
             buffers.prepend(QByteArray());
-            buffers[0].resize(basicBlockSize);
+            buffers.first().resize(basicBlockSize);
             head = basicBlockSize - 1;
             ++tailBuffer;
         }
-        buffers[0][head] = c;
+        buffers.first()[head] = c;
         ++bufferSize;
     }
 
@@ -259,8 +198,7 @@ public:
 
     inline void clear() {
         buffers.erase(buffers.begin() + 1, buffers.end());
-        buffers[0].resize(0);
+        buffers.first().clear();
-        buffers[0].squeeze();
 
         head = tail = 0;
         tailBuffer = 0;
@@ -269,47 +207,34 @@ public:
 
     inline int indexOf(char c) const {
         int index = 0;
+        int j = head;
         for (int i = 0; i < buffers.size(); ++i) {
-            int start = 0;
+            const char *ptr = buffers[i].constData() + j;
-            int end = buffers.at(i).size();
+            j = index + (i == tailBuffer ? tail : buffers[i].size()) - j;
 
-            if (i == 0)
+            while (index < j) {
-                start = head;
-            if (i == tailBuffer)
-                end = tail;
-            const char *ptr = buffers.at(i).data() + start;
-            for (int j = start; j < end; ++j) {
                 if (*ptr++ == c)
                     return index;
                 ++index;
             }
+            j = 0;
         }
         return -1;
     }
 
     inline int indexOf(char c, int maxLength) const {
         int index = 0;
-        int remain = qMin(size(), maxLength);
+        int j = head;
-        for (int i = 0; remain && i < buffers.size(); ++i) {
+        for (int i = 0; index < maxLength && i < buffers.size(); ++i) {
-            int start = 0;
+            const char *ptr = buffers[i].constData() + j;
-            int end = buffers.at(i).size();
+            j = qMin(index + (i == tailBuffer ? tail : buffers[i].size()) - j, maxLength);
 
-            if (i == 0)
+            while (index < j) {
-                start = head;
-            if (i == tailBuffer)
-                end = tail;
-            if (remain < end - start) {
-                end = start + remain;
-                remain = 0;
-            } else {
-                remain -= end - start;
-            }
-            const char *ptr = buffers.at(i).data() + start;
-            for (int j = start; j < end; ++j) {
                 if (*ptr++ == c)
                     return index;
                 ++index;
             }
+            j = 0;
         }
         return -1;
     }
@@ -318,10 +243,9 @@ public:
         int bytesToRead = qMin(size(), maxLength);
         int readSoFar = 0;
         while (readSoFar < bytesToRead) {
-            const char *ptr = readPointer();
             int bytesToReadFromThisBlock = qMin(bytesToRead - readSoFar, nextDataBlockSize());
             if (data)
-                memcpy(data + readSoFar, ptr, bytesToReadFromThisBlock);
+                memcpy(data + readSoFar, readPointer(), bytesToReadFromThisBlock);
             readSoFar += bytesToReadFromThisBlock;
             free(bytesToReadFromThisBlock);
         }
@@ -333,37 +257,19 @@ public:
         if (bufferSize == 0)
             return QByteArray();
 
-        // multiple buffers, just take the first one
+        QByteArray qba(buffers.takeFirst());
-        if (head == 0 && tailBuffer != 0) {
-            QByteArray qba = buffers.takeFirst();
-            --tailBuffer;
-            bufferSize -= qba.length();
-            return qba;
-        }
 
-        // one buffer with good value for head. Just take it.
+        qba.reserve(0); // avoid that resizing needlessly reallocates
-        if (head == 0 && tailBuffer == 0) {
-            QByteArray qba = buffers.takeFirst();
-            qba.resize(tail);
-            buffers << QByteArray();
-            bufferSize = 0;
-            tail = 0;
-            return qba;
-        }
-
-        // Bad case: We have to memcpy.
-        // We can avoid by initializing the QRingBuffer with basicBlockSize of 0
-        // and only using this read() function.
-        QByteArray qba(readPointer(), nextDataBlockSize());
-        buffers.removeFirst();
-        head = 0;
         if (tailBuffer == 0) {
-            buffers << QByteArray();
+            qba.resize(tail);
             tail = 0;
+            buffers.append(QByteArray());
         } else {
             --tailBuffer;
         }
-        bufferSize -= qba.length();
+        qba.remove(0, head); // does nothing if head is 0
+        head = 0;
+        bufferSize -= qba.size();
         return qba;
     }
 
@@ -373,11 +279,11 @@ public:
             buffers.last() = qba;
         } else {
             buffers.last().resize(tail);
-            buffers << qba;
+            buffers.append(qba);
             ++tailBuffer;
         }
-        tail = qba.length();
+        tail = qba.size();
-        bufferSize += qba.length();
+        bufferSize += tail;
     }
 
     inline int skip(int length) {
@@ -385,35 +291,26 @@ public:
     }
 
     inline int readLine(char *data, int maxLength) {
-        int index = indexOf('\n');
+        if (!data || --maxLength <= 0)
-        if (index == -1)
-            return read(data, maxLength);
-        if (maxLength <= 0)
             return -1;
 
-        int readSoFar = 0;
+        int i = indexOf('\n', maxLength);
-        while (readSoFar < index + 1 && readSoFar < maxLength - 1) {
+        i = read(data, i >= 0 ? (i + 1) : maxLength);
-            int bytesToRead = qMin((index + 1) - readSoFar, nextDataBlockSize());
-            bytesToRead = qMin(bytesToRead, (maxLength - 1) - readSoFar);
-            memcpy(data + readSoFar, readPointer(), bytesToRead);
-            readSoFar += bytesToRead;
-            free(bytesToRead);
-        }
 
         // Terminate it.
-        data[readSoFar] = '\0';
+        data[i] = '\0';
-        return readSoFar;
+        return i;
     }
 
     inline bool canReadLine() const {
-        return indexOf('\n') != -1;
+        return indexOf('\n') >= 0;
     }
 
 private:
     QList<QByteArray> buffers;
     int head, tail;
     int tailBuffer; // always buffers.size() - 1
-    int basicBlockSize;
+    const int basicBlockSize;
     int bufferSize;
 };
 

tests/auto/corelib/tools/qringbuffer/tst_qringbuffer.cpp

@@ -54,7 +54,13 @@ private slots:
     void sizeWhenEmpty();
     void sizeWhenReservedAndChopped();
     void sizeWhenReserved();
+    void free();
+    void reserveAndRead();
+    void chop();
+    void ungetChar();
+    void indexOf();
     void appendAndRead();
+    void readLine();
 };
 
 void tst_QRingBuffer::sizeWhenReserved()
@@ -180,6 +186,93 @@ void tst_QRingBuffer::readPointerAtPositionWriteRead()
     QVERIFY(outData.buffer().startsWith(inData.buffer()));
 }
 
+void tst_QRingBuffer::free()
+{
+    QRingBuffer ringBuffer;
+    // make three byte arrays with different sizes
+    ringBuffer.reserve(4096);
+    ringBuffer.reserve(2048);
+    ringBuffer.append(QByteArray("01234", 5));
+
+    ringBuffer.free(1);
+    QCOMPARE(ringBuffer.size(), 4095 + 2048 + 5);
+    ringBuffer.free(4096);
+    QCOMPARE(ringBuffer.size(), 2047 + 5);
+    ringBuffer.free(48);
+    ringBuffer.free(2000);
+    QCOMPARE(ringBuffer.size(), 4);
+    QVERIFY(memcmp(ringBuffer.readPointer(), "1234", 4) == 0);
+}
+
+void tst_QRingBuffer::reserveAndRead()
+{
+    QRingBuffer ringBuffer;
+    // fill buffer with an arithmetic progression
+    for (int i = 1; i < 256; ++i) {
+        QByteArray ba(i, char(i));
+        char *ringPos = ringBuffer.reserve(i);
+        QVERIFY(ringPos);
+        memcpy(ringPos, ba.constData(), i);
+    }
+
+    // readback and check stored data
+    for (int i = 1; i < 256; ++i) {
+        QByteArray ba;
+        ba.resize(i);
+        int thisRead = ringBuffer.read(ba.data(), i);
+        QCOMPARE(thisRead, i);
+        QVERIFY(ba.count(char(i)) == i);
+    }
+    QVERIFY(ringBuffer.size() == 0);
+}
+
+void tst_QRingBuffer::chop()
+{
+    QRingBuffer ringBuffer;
+    // make three byte arrays with different sizes
+    ringBuffer.append(QByteArray("01234", 5));
+    ringBuffer.reserve(2048);
+    ringBuffer.reserve(4096);
+
+    ringBuffer.chop(1);
+    QCOMPARE(ringBuffer.size(), 5 + 2048 + 4095);
+    ringBuffer.chop(4096);
+    QCOMPARE(ringBuffer.size(), 5 + 2047);
+    ringBuffer.chop(48);
+    ringBuffer.chop(2000);
+    QCOMPARE(ringBuffer.size(), 4);
+    QVERIFY(memcmp(ringBuffer.readPointer(), "0123", 4) == 0);
+}
+
+void tst_QRingBuffer::ungetChar()
+{
+    QRingBuffer ringBuffer(16);
+    for (int i = 1; i < 32; ++i)
+        ringBuffer.putChar(char(i));
+
+    for (int i = 1; i < 31; ++i) {
+        int c = ringBuffer.getChar();
+        QVERIFY(c == 1);
+        ringBuffer.getChar();
+        ringBuffer.ungetChar(char(c)); // unget first char
+    }
+    QCOMPARE(ringBuffer.size(), 1);
+}
+
+void tst_QRingBuffer::indexOf()
+{
+    QRingBuffer ringBuffer(16);
+    for (int i = 1; i < 256; ++i)
+        ringBuffer.putChar(char(i));
+
+    for (int i = 1; i < 256; ++i) {
+        int index = ringBuffer.indexOf(char(i));
+        QCOMPARE(i - 1, index);
+        QCOMPARE(index, ringBuffer.indexOf(char(i), i));
+        QVERIFY(ringBuffer.indexOf(char(i), i - 1) == -1); // test for absent char
+    }
+}
+
 void tst_QRingBuffer::appendAndRead()
 {
     QRingBuffer ringBuffer;
@@ -195,5 +288,32 @@ void tst_QRingBuffer::appendAndRead()
     QVERIFY(ringBuffer.read() == ba3);
 }
 
+void tst_QRingBuffer::readLine()
+{
+    QRingBuffer ringBuffer;
+    QByteArray ba1("Hello world!\n", 13);
+    QByteArray ba2("\n", 1);
+    QByteArray ba3("Test string.", 12);
+    QByteArray ba4("0123456789", 10);
+    ringBuffer.append(ba1);
+    ringBuffer.append(ba2);
+    ringBuffer.append(ba3 + ba4 + ba2);
+
+    char stringBuf[102];
+    stringBuf[101] = 0; // non-crash terminator
+    QVERIFY(ringBuffer.readLine(stringBuf, sizeof(stringBuf) - 2) == ba1.size());
+    QVERIFY(QByteArray(stringBuf, strlen(stringBuf)) == ba1);
+
+    // check first empty string reading
+    stringBuf[0] = 0xFF;
+    QCOMPARE(ringBuffer.readLine(stringBuf, sizeof(stringBuf) - 2), ba2.size());
+    QVERIFY(stringBuf[0] == ba2[0]);
+
+    QVERIFY(ringBuffer.readLine(stringBuf, sizeof(stringBuf) - 2) == (ba3.size() + ba4.size()
+        + ba2.size()));
+    QVERIFY(QByteArray(stringBuf, strlen(stringBuf)) == (ba3 + ba4 + ba2));
+    QVERIFY(ringBuffer.size() == 0);
+}
+
 QTEST_APPLESS_MAIN(tst_QRingBuffer)
 #include "tst_qringbuffer.moc"

tests/benchmarks/corelib/tools/qringbuffer/main.cpp

@@ -0,0 +1,84 @@
+/****************************************************************************
+**
+** Copyright (C) 2014 Digia Plc and/or its subsidiary(-ies).
+** Contact: http://www.qt-project.org/legal
+**
+** This file is part of the test suite of the Qt Toolkit.
+**
+** $QT_BEGIN_LICENSE:LGPL$
+** Commercial License Usage
+** Licensees holding valid commercial Qt licenses may use this file in
+** accordance with the commercial license agreement provided with the
+** Software or, alternatively, in accordance with the terms contained in
+** a written agreement between you and Digia.  For licensing terms and
+** conditions see http://qt.digia.com/licensing.  For further information
+** use the contact form at http://qt.digia.com/contact-us.
+**
+** GNU Lesser General Public License Usage
+** Alternatively, this file may be used under the terms of the GNU Lesser
+** General Public License version 2.1 as published by the Free Software
+** Foundation and appearing in the file LICENSE.LGPL included in the
+** packaging of this file.  Please review the following information to
+** ensure the GNU Lesser General Public License version 2.1 requirements
+** will be met: http://www.gnu.org/licenses/old-licenses/lgpl-2.1.html.
+**
+** In addition, as a special exception, Digia gives you certain additional
+** rights.  These rights are described in the Digia Qt LGPL Exception
+** version 1.1, included in the file LGPL_EXCEPTION.txt in this package.
+**
+** GNU General Public License Usage
+** Alternatively, this file may be used under the terms of the GNU
+** General Public License version 3.0 as published by the Free Software
+** Foundation and appearing in the file LICENSE.GPL included in the
+** packaging of this file.  Please review the following information to
+** ensure the GNU General Public License version 3.0 requirements will be
+** met: http://www.gnu.org/copyleft/gpl.html.
+**
+**
+** $QT_END_LICENSE$
+**
+****************************************************************************/
+
+#include <private/qringbuffer_p.h>
+#include <QByteArray>
+
+#include <qtest.h>
+
+class tst_qringbuffer : public QObject
+{
+    Q_OBJECT
+private slots:
+    void reserveAndRead();
+    void free();
+};
+
+void tst_qringbuffer::reserveAndRead()
+{
+    QRingBuffer ringBuffer;
+    QBENCHMARK {
+        for (int i = 1; i < 256; ++i)
+            ringBuffer.reserve(i);
+
+        for (int i = 1; i < 256; ++i)
+            ringBuffer.read(0, i);
+    }
+}
+
+void tst_qringbuffer::free()
+{
+    QRingBuffer ringBuffer;
+    QBENCHMARK {
+        ringBuffer.reserve(4096);
+        ringBuffer.reserve(2048);
+        ringBuffer.append(QByteArray("01234", 5));
+
+        ringBuffer.free(1);
+        ringBuffer.free(4096);
+        ringBuffer.free(48);
+        ringBuffer.free(2000);
+    }
+}
+
+QTEST_MAIN(tst_qringbuffer)
+
+#include "main.moc"

tests/benchmarks/corelib/tools/qringbuffer/qringbuffer.pro

@@ -0,0 +1,7 @@
+TEMPLATE = app
+TARGET = tst_bench_qringbuffer
+
+QT = core-private testlib
+CONFIG += release
+
+SOURCES += main.cpp