From a41c61fb2d2f973fd1cd5e95ee5be1ac1a4f8433 Mon Sep 17 00:00:00 2001 From: Alex Trotsenko Date: Tue, 11 Aug 2020 19:30:15 +0300 Subject: [PATCH] QIODevice: implement a "zero-copy" strategy for buffered writes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit It works as follows: - user calls write(const QByteArray &); - this function keeps a pointer to the chunk and calls a regular write(data, len); - write(data, len) calls a virtual writeData(); - subclass calls a new QIODevicePrivate::write(); - QIODevicePrivate::write() makes a shallow copy of the byte array. Proposed solution is fully compatible with existing subclasses. By replacing a call to d->writeBuffer.append() with d->write(), subclasses can improve their performance. Bump the TypeInformationVersion field in qtHookData, to notify the Qt Creator developers that the offset of QFilePrivate::fileName was changed and dumpers should be adapted. Change-Id: I24713386cc74a9f37e5223c617e4b1ba97f968dc Reviewed-by: Oswald Buddenhagen Reviewed-by: Mårten Nordheim --- src/corelib/global/qhooks.cpp | 2 +- src/corelib/io/qiodevice.cpp | 32 +++++++++++++++++-- src/corelib/io/qiodevice_p.h | 4 ++- src/corelib/io/qprocess.cpp | 2 +- src/network/socket/qabstractsocket.cpp | 2 +- src/network/socket/qlocalsocket_win.cpp | 2 +- src/network/ssl/qsslsocket.cpp | 2 +- .../other/toolsupport/tst_toolsupport.cpp | 4 +-- 8 files changed, 40 insertions(+), 10 deletions(-) diff --git a/src/corelib/global/qhooks.cpp b/src/corelib/global/qhooks.cpp index 22f01853b73..5a0caa2ed0c 100644 --- a/src/corelib/global/qhooks.cpp +++ b/src/corelib/global/qhooks.cpp @@ -67,7 +67,7 @@ quintptr Q_CORE_EXPORT qtHookData[] = { // The required sizes and offsets are tested in tests/auto/other/toolsupport. // When this fails and the change was intentional, adjust the test and // adjust this value here. - 18 + 19 }; static_assert(QHooks::LastHookIndex == sizeof(qtHookData) / sizeof(qtHookData[0])); diff --git a/src/corelib/io/qiodevice.cpp b/src/corelib/io/qiodevice.cpp index 3d64e8820f3..b920fbb17b8 100644 --- a/src/corelib/io/qiodevice.cpp +++ b/src/corelib/io/qiodevice.cpp @@ -158,13 +158,14 @@ static void checkWarnMessage(const QIODevice *device, const char *function, cons QIODevicePrivate::QIODevicePrivate() : openMode(QIODevice::NotOpen), pos(0), devicePos(0), + transactionPos(0), readChannelCount(0), writeChannelCount(0), currentReadChannel(0), currentWriteChannel(0), readBufferChunkSize(QIODEVICE_BUFFERSIZE), writeBufferChunkSize(0), - transactionPos(0), + currentWriteChunk(nullptr), transactionStarted(false) , baseReadLineDataCalled(false) , accessMode(Unset) @@ -1750,7 +1751,34 @@ qint64 QIODevice::write(const char *data) qint64 QIODevice::write(const QByteArray &data) { - return write(data.constData(), data.size()); + Q_D(QIODevice); + + // Keep the chunk pointer for further processing in + // QIODevicePrivate::write(). To reduce fragmentation, + // the chunk size must be sufficiently large. + if (data.size() >= QRINGBUFFER_CHUNKSIZE) + d->currentWriteChunk = &data; + + const qint64 ret = write(data.constData(), data.size()); + + d->currentWriteChunk = nullptr; + return ret; +} + +/*! + \internal +*/ +void QIODevicePrivate::write(const char *data, qint64 size) +{ + if (currentWriteChunk != nullptr + && currentWriteChunk->constData() == data + && currentWriteChunk->size() == size) { + // We are called from write(const QByteArray &) overload. + // So, we can make a shallow copy of chunk. + writeBuffer.append(*currentWriteChunk); + } else { + writeBuffer.append(data, size); + } } /*! diff --git a/src/corelib/io/qiodevice_p.h b/src/corelib/io/qiodevice_p.h index d8e12d7259f..a485ec43b3e 100644 --- a/src/corelib/io/qiodevice_p.h +++ b/src/corelib/io/qiodevice_p.h @@ -124,13 +124,14 @@ public: QRingBufferRef writeBuffer; qint64 pos; qint64 devicePos; + qint64 transactionPos; int readChannelCount; int writeChannelCount; int currentReadChannel; int currentWriteChannel; int readBufferChunkSize; int writeBufferChunkSize; - qint64 transactionPos; + const QByteArray *currentWriteChunk; bool transactionStarted; bool baseReadLineDataCalled; @@ -175,6 +176,7 @@ public: virtual qint64 peek(char *data, qint64 maxSize); virtual QByteArray peek(qint64 maxSize); qint64 skipByReading(qint64 maxSize); + void write(const char *data, qint64 size); #ifdef QT_NO_QOBJECT QIODevice *q_ptr; diff --git a/src/corelib/io/qprocess.cpp b/src/corelib/io/qprocess.cpp index b56ead85840..8e8664fda47 100644 --- a/src/corelib/io/qprocess.cpp +++ b/src/corelib/io/qprocess.cpp @@ -1899,7 +1899,7 @@ qint64 QProcess::writeData(const char *data, qint64 len) } #endif - d->writeBuffer.append(data, len); + d->write(data, len); #ifdef Q_OS_WIN if (!d->stdinWriteTrigger->isActive()) d->stdinWriteTrigger->start(); diff --git a/src/network/socket/qabstractsocket.cpp b/src/network/socket/qabstractsocket.cpp index cf6a7672fb8..3bde2215add 100644 --- a/src/network/socket/qabstractsocket.cpp +++ b/src/network/socket/qabstractsocket.cpp @@ -2542,7 +2542,7 @@ qint64 QAbstractSocket::writeData(const char *data, qint64 size) // We just write to our write buffer and enable the write notifier // The write notifier then flush()es the buffer. - d->writeBuffer.append(data, size); + d->write(data, size); qint64 written = size; if (d->socketEngine && !d->writeBuffer.isEmpty()) diff --git a/src/network/socket/qlocalsocket_win.cpp b/src/network/socket/qlocalsocket_win.cpp index b585bea6255..e174952b963 100644 --- a/src/network/socket/qlocalsocket_win.cpp +++ b/src/network/socket/qlocalsocket_win.cpp @@ -214,7 +214,7 @@ qint64 QLocalSocket::writeData(const char *data, qint64 len) Q_D(QLocalSocket); if (len == 0) return 0; - d->writeBuffer.append(data, len); + d->write(data, len); if (!d->pipeWriter) { d->pipeWriter = new QWindowsPipeWriter(d->handle, this); connect(d->pipeWriter, &QWindowsPipeWriter::bytesWritten, diff --git a/src/network/ssl/qsslsocket.cpp b/src/network/ssl/qsslsocket.cpp index ab4ce1527e9..c5444b139e5 100644 --- a/src/network/ssl/qsslsocket.cpp +++ b/src/network/ssl/qsslsocket.cpp @@ -1819,7 +1819,7 @@ qint64 QSslSocket::writeData(const char *data, qint64 len) if (d->mode == UnencryptedMode && !d->autoStartHandshake) return d->plainSocket->write(data, len); - d->writeBuffer.append(data, len); + d->write(data, len); // make sure we flush to the plain socket's buffer if (!d->flushTriggered) { diff --git a/tests/auto/other/toolsupport/tst_toolsupport.cpp b/tests/auto/other/toolsupport/tst_toolsupport.cpp index 19b6adb4b34..fc3a13267ee 100644 --- a/tests/auto/other/toolsupport/tst_toolsupport.cpp +++ b/tests/auto/other/toolsupport/tst_toolsupport.cpp @@ -126,9 +126,9 @@ void tst_toolsupport::offsets_data() #ifdef Q_PROCESSOR_X86 // x86 32-bit has weird alignment rules. Refer to QtPrivate::AlignOf in // qglobal.h for more details. - data << 184 << 288; + data << 188 << 296; #else - data << 188 << 288; + data << 192 << 296; #endif } #endif