From b99fa32d70e6511ae8bc1c6e806dbbb042dc8090 Mon Sep 17 00:00:00 2001 From: Markus Goetz Date: Wed, 23 Jul 2014 16:35:01 +0200 Subject: QNAM: Fix CPU load for limited upload QIODevice This fixes high CPU load for upload devices that don't generate a constant stream of data. Their readData() function was called all the time without returning actual data. This was noticed when implementing an upload device that emits data in a limited way for bandwidth limiting. [ChangeLog][QtNetwork][QNetworkAccessManager] Fixed high CPU load when handling POST/upload QIODevice that generates data on readyRead(). Change-Id: Iefbcb1a21d8aedef1eb11761232dd16a049018dc Reviewed-by: Richard J. Moore --- src/network/access/qnetworkreplyhttpimpl.cpp | 19 +++++ src/network/access/qnetworkreplyhttpimpl_p.h | 4 + .../access/qnetworkreply/tst_qnetworkreply.cpp | 92 ++++++++++++++++++++++ 3 files changed, 115 insertions(+) diff --git a/src/network/access/qnetworkreplyhttpimpl.cpp b/src/network/access/qnetworkreplyhttpimpl.cpp index 56105a544b..4efb2f6a2a 100644 --- a/src/network/access/qnetworkreplyhttpimpl.cpp +++ b/src/network/access/qnetworkreplyhttpimpl.cpp @@ -863,6 +863,11 @@ void QNetworkReplyHttpImplPrivate::postRequest() forwardUploadDevice->setParent(delegate); // needed to make sure it is moved on moveToThread() delegate->httpRequest.setUploadByteDevice(forwardUploadDevice); + // If the device in the user thread claims it has more data, keep the flow to HTTP thread going + QObject::connect(uploadByteDevice.data(), SIGNAL(readyRead()), + q, SLOT(uploadByteDeviceReadyReadSlot()), + Qt::QueuedConnection); + // From main thread to user thread: QObject::connect(q, SIGNAL(haveUploadData(QByteArray,bool,qint64)), forwardUploadDevice, SLOT(haveDataSlot(QByteArray,bool,qint64)), Qt::QueuedConnection); @@ -1284,6 +1289,13 @@ void QNetworkReplyHttpImplPrivate::wantUploadDataSlot(qint64 maxSize) // call readPointer qint64 currentUploadDataLength = 0; char *data = const_cast(uploadByteDevice->readPointer(maxSize, currentUploadDataLength)); + + if (currentUploadDataLength == 0) { + // No bytes from upload byte device. There will be bytes later, it will emit readyRead() + // and our uploadByteDeviceReadyReadSlot() is called. + return; + } + // Let's make a copy of this data QByteArray dataArray(data, currentUploadDataLength); @@ -1291,6 +1303,13 @@ void QNetworkReplyHttpImplPrivate::wantUploadDataSlot(qint64 maxSize) emit q->haveUploadData(dataArray, uploadByteDevice->atEnd(), uploadByteDevice->size()); } +void QNetworkReplyHttpImplPrivate::uploadByteDeviceReadyReadSlot() +{ + // Start the flow between this thread and the HTTP thread again by triggering a upload. + wantUploadDataSlot(1024); +} + + /* A simple web page that can be used to test us: http://www.procata.com/cachetest/ */ diff --git a/src/network/access/qnetworkreplyhttpimpl_p.h b/src/network/access/qnetworkreplyhttpimpl_p.h index d21659ce82..06a5383ae4 100644 --- a/src/network/access/qnetworkreplyhttpimpl_p.h +++ b/src/network/access/qnetworkreplyhttpimpl_p.h @@ -129,6 +129,7 @@ public: Q_PRIVATE_SLOT(d_func(), void resetUploadDataSlot(bool *r)) Q_PRIVATE_SLOT(d_func(), void wantUploadDataSlot(qint64)) Q_PRIVATE_SLOT(d_func(), void sentUploadDataSlot(qint64)) + Q_PRIVATE_SLOT(d_func(), void uploadByteDeviceReadyReadSlot()) Q_PRIVATE_SLOT(d_func(), void emitReplyUploadProgress(qint64, qint64)) Q_PRIVATE_SLOT(d_func(), void _q_cacheSaveDeviceAboutToClose()) @@ -300,6 +301,9 @@ public: void wantUploadDataSlot(qint64); void sentUploadDataSlot(qint64); + // From user's QNonContiguousByteDevice + void uploadByteDeviceReadyReadSlot(); + Q_DECLARE_PUBLIC(QNetworkReplyHttpImpl) }; diff --git a/tests/auto/network/access/qnetworkreply/tst_qnetworkreply.cpp b/tests/auto/network/access/qnetworkreply/tst_qnetworkreply.cpp index 0509eb9a77..480eeecb63 100644 --- a/tests/auto/network/access/qnetworkreply/tst_qnetworkreply.cpp +++ b/tests/auto/network/access/qnetworkreply/tst_qnetworkreply.cpp @@ -461,6 +461,8 @@ private Q_SLOTS: void backgroundRequestConnectInBackground(); #endif + void putWithRateLimiting(); + // NOTE: This test must be last! void parentingRepliesToTheApp(); private: @@ -7776,6 +7778,96 @@ void tst_QNetworkReply::backgroundRequestConnectInBackground() } #endif +class RateLimitedUploadDevice : public QIODevice +{ + Q_OBJECT +public: + QByteArray data; + QBuffer buffer; + qint64 read; + qint64 bandwidthQuota; + QTimer timer; + + RateLimitedUploadDevice(QByteArray d) : QIODevice(),data(d),read(0),bandwidthQuota(0) { + buffer.setData(data); + buffer.open(QIODevice::ReadOnly); + timer.setInterval(200); + QObject::connect(&timer, SIGNAL(timeout()), this, SLOT(timeoutSlot())); + timer.start(); + } + + virtual qint64 writeData(const char* , qint64 ) { + Q_ASSERT(false); + return 0; + } + + virtual qint64 readData(char* data, qint64 maxlen) { + //qDebug() << Q_FUNC_INFO << maxlen << bandwidthQuota; + maxlen = qMin(maxlen, buffer.bytesAvailable()); + maxlen = qMin(maxlen, bandwidthQuota); + if (maxlen <= 0) { // no quota or at end + return 0; + } + bandwidthQuota -= maxlen; // reduce quota + + qint64 ret = buffer.read(data, maxlen); + if (ret == -1) { + return -1; + } + read += ret; + //qDebug() << Q_FUNC_INFO << maxlen << bandwidthQuota << read << ret << buffer.bytesAvailable(); + return ret; + } + virtual bool atEnd() const { + return buffer.atEnd(); + } + virtual qint64 size() const{ + return data.length(); + } + qint64 bytesAvailable() const + { + return buffer.bytesAvailable() + QIODevice::bytesAvailable(); + } + virtual bool isSequential() const{ // random access, we can seek + return false; + } + virtual bool seek ( qint64 pos ) { + return buffer.seek(pos); + } +protected slots: + void timeoutSlot() { + //qDebug() << Q_FUNC_INFO; + bandwidthQuota = 8*1024; // fill quota + emit readyRead(); + } +}; + +void tst_QNetworkReply::putWithRateLimiting() +{ + QFile reference(testDataDir + "/rfc3252.txt"); + reference.open(QIODevice::ReadOnly); + QByteArray data = reference.readAll(); + QVERIFY(data.length() > 0); + + QUrl url = QUrl::fromUserInput("http://" + QtNetworkSettings::serverName()+ "/qtest/cgi-bin/echo.cgi?"); + + QNetworkRequest request(url); + QNetworkReplyPtr reply; + + RateLimitedUploadDevice rateLimitedUploadDevice(data); + rateLimitedUploadDevice.open(QIODevice::ReadOnly); + + RUN_REQUEST(runCustomRequest(request, reply,QByteArray("POST"), &rateLimitedUploadDevice)); + QCOMPARE(reply->error(), QNetworkReply::NoError); + QCOMPARE(reply->attribute(QNetworkRequest::HttpStatusCodeAttribute).toInt(), 200); + + QByteArray uploadedData = reply->readAll(); + QCOMPARE(uploadedData.length(), data.length()); + QCOMPARE(uploadedData, data); +} + + + // NOTE: This test must be last testcase in tst_qnetworkreply! void tst_QNetworkReply::parentingRepliesToTheApp() { -- cgit v1.2.3