Improve QIODevice::peek() performance on buffered devices

Since 5.7, QIODevice::peek() implementation is based on transaction
mechanism. While technically it's correct, seeking backward on a
buffered random-access device clears the internal buffer that affects
the performance of reading.

To solve the problem, this patch implements peek mode directly inside
the reading procedure.

Task-number: QTBUG-56032
Change-Id: Ic5269f76e44c491a0309e13aba87fa7cf7b9259f
Reviewed-by: Edward Welbourne <edward.welbourne@qt.io>

src/corelib/io/qiodevice.cpp

@@ -1047,26 +1047,29 @@ qint64 QIODevice::read(char *data, qint64 maxSize)
 /*!
     \internal
 */
-qint64 QIODevicePrivate::read(char *data, qint64 maxSize)
+qint64 QIODevicePrivate::read(char *data, qint64 maxSize, bool peeking)
 {
     Q_Q(QIODevice);
 
     const bool buffered = (openMode & QIODevice::Unbuffered) == 0;
     const bool sequential = isSequential();
-    const bool keepDataInBuffer = sequential && transactionStarted;
+    const bool keepDataInBuffer = sequential
+                                  ? peeking || transactionStarted
+                                  : peeking && buffered;
+    const qint64 savedPos = pos;
     qint64 readSoFar = 0;
     bool madeBufferReadsOnly = true;
     bool deviceAtEof = false;
     char *readPtr = data;
+    qint64 bufferPos = (sequential && transactionStarted) ? transactionPos : Q_INT64_C(0);
     forever {
         // Try reading from the buffer.
         qint64 bufferReadChunkSize = keepDataInBuffer
-                                     ? buffer.peek(data, maxSize, transactionPos)
+                                     ? buffer.peek(data, maxSize, bufferPos)
                                      : buffer.read(data, maxSize);
         if (bufferReadChunkSize > 0) {
-            if (keepDataInBuffer)
-                transactionPos += bufferReadChunkSize;
-            else if (!sequential)
+            bufferPos += bufferReadChunkSize;
+            if (!sequential)
                 pos += bufferReadChunkSize;
 #if defined QIODEVICE_DEBUG
             printf("%p \treading %lld bytes from buffer into position %lld\n", q,
@@ -1160,6 +1163,16 @@ qint64 QIODevicePrivate::read(char *data, qint64 maxSize)
         break;
     }
 
+    // Restore positions after reading
+    if (keepDataInBuffer) {
+        if (peeking)
+            pos = savedPos; // does nothing on sequential devices
+        else
+            transactionPos = bufferPos;
+    } else if (peeking) {
+        seekBuffer(savedPos); // unbuffered random-access device
+    }
+
     if (madeBufferReadsOnly && isBufferEmpty())
         q->readData(data, 0);
 
@@ -1773,27 +1786,7 @@ bool QIODevicePrivate::putCharHelper(char c)
 */
 qint64 QIODevicePrivate::peek(char *data, qint64 maxSize)
 {
-    Q_Q(QIODevice);
-
-    if (transactionStarted) {
-        const qint64 savedTransactionPos = transactionPos;
-        const qint64 savedPos = pos;
-
-        qint64 readBytes = q->read(data, maxSize);
-
-        // Restore initial position
-        if (isSequential())
-            transactionPos = savedTransactionPos;
-        else
-            seekBuffer(savedPos);
-        return readBytes;
-    }
-
-    q->startTransaction();
-    qint64 readBytes = q->read(data, maxSize);
-    q->rollbackTransaction();
-
-    return readBytes;
+    return read(data, maxSize, true);
 }
 
 /*!
@@ -1801,26 +1794,17 @@ qint64 QIODevicePrivate::peek(char *data, qint64 maxSize)
 */
 QByteArray QIODevicePrivate::peek(qint64 maxSize)
 {
-    Q_Q(QIODevice);
+    QByteArray result(maxSize, Qt::Uninitialized);
 
-    if (transactionStarted) {
-        const qint64 savedTransactionPos = transactionPos;
-        const qint64 savedPos = pos;
+    const qint64 readBytes = read(result.data(), maxSize, true);
 
-        QByteArray result = q->read(maxSize);
-
-        // Restore initial position
-        if (isSequential())
-            transactionPos = savedTransactionPos;
+    if (readBytes < maxSize) {
+        if (readBytes <= 0)
+            result.clear();
         else
-            seekBuffer(savedPos);
-        return result;
+            result.resize(readBytes);
     }
 
-    q->startTransaction();
-    QByteArray result = q->read(maxSize);
-    q->rollbackTransaction();
-
     return result;
 }
 
@@ -1858,7 +1842,12 @@ bool QIODevice::getChar(char *c)
 */
 qint64 QIODevice::peek(char *data, qint64 maxSize)
 {
-    return d_func()->peek(data, maxSize);
+    Q_D(QIODevice);
+
+    CHECK_MAXLEN(peek, qint64(-1));
+    CHECK_READABLE(peek, qint64(-1));
+
+    return d->peek(data, maxSize);
 }
 
 /*!
@@ -1880,7 +1869,13 @@ qint64 QIODevice::peek(char *data, qint64 maxSize)
 */
 QByteArray QIODevice::peek(qint64 maxSize)
 {
-    return d_func()->peek(maxSize);
+    Q_D(QIODevice);
+
+    CHECK_MAXLEN(peek, QByteArray());
+    CHECK_MAXBYTEARRAYSIZE(peek);
+    CHECK_READABLE(peek, QByteArray());
+
+    return d->peek(maxSize);
 }
 
 /*!

src/corelib/io/qiodevice_p.h

@@ -171,7 +171,7 @@ public:
     void setReadChannelCount(int count);
     void setWriteChannelCount(int count);
 
-    qint64 read(char *data, qint64 maxSize);
+    qint64 read(char *data, qint64 maxSize, bool peeking = false);
     virtual qint64 peek(char *data, qint64 maxSize);
     virtual QByteArray peek(qint64 maxSize);
 

tests/benchmarks/corelib/io/qiodevice/main.cpp

@@ -39,6 +39,8 @@ class tst_qiodevice : public QObject
 private slots:
     void read_old();
     void read_old_data() { read_data(); }
+    void peekAndRead();
+    void peekAndRead_data() { read_data(); }
     //void read_new();
     //void read_new_data() { read_data(); }
 private:
@@ -86,6 +88,36 @@ void tst_qiodevice::read_old()
     }
 }
 
+void tst_qiodevice::peekAndRead()
+{
+    QFETCH(qint64, size);
+
+    QString name = "tmp" + QString::number(size);
+
+    {
+        QFile file(name);
+        file.open(QIODevice::WriteOnly);
+        file.seek(size);
+        file.write("x", 1);
+        file.close();
+    }
+
+    QBENCHMARK {
+        QFile file(name);
+        file.open(QIODevice::ReadOnly);
+
+        QByteArray ba(size / 1024, Qt::Uninitialized);
+        while (!file.atEnd()) {
+            file.peek(ba.data(), ba.size());
+            file.read(ba.data(), ba.size());
+        }
+    }
+
+    {
+        QFile file(name);
+        file.remove();
+    }
+}
 
 QTEST_MAIN(tst_qiodevice)