From 48345e5d3c07d6dca12dd4ccee18fa3e0ca0ebda Mon Sep 17 00:00:00 2001 From: Peter Hartmann Date: Tue, 11 Jun 2013 10:45:41 +0200 Subject: [PATCH] HTTP internals: do not open too many sockets when preconnecting Each pair of (normal request, preconnect request) requires only one socket. E.g. if there is 1 preconnect request in-flight and 2 normal requests, we need only 2 sockets in total, and not 3. Therefore, we need to keep track of whether a request is preconnecting or a normal one. Task-number: QTBUG-31594 Change-Id: If92ccc35abadfa6090d64ee92bd466615909c94c Reviewed-by: Richard J. Moore --- src/network/access/qhttpnetworkconnection.cpp | 32 ++++++++++++++----- src/network/access/qhttpnetworkconnection_p.h | 4 +++ .../access/qhttpnetworkconnectionchannel.cpp | 1 + src/network/access/qhttpnetworkrequest.cpp | 16 ++++++++-- src/network/access/qhttpnetworkrequest_p.h | 4 +++ src/network/access/qnetworkreplyhttpimpl.cpp | 3 ++ .../qnetworkreply/tst_qnetworkreply.cpp | 20 ++++++++++-- 7 files changed, 68 insertions(+), 12 deletions(-) diff --git a/src/network/access/qhttpnetworkconnection.cpp b/src/network/access/qhttpnetworkconnection.cpp index 5c3c38606d..c3e3716b26 100644 --- a/src/network/access/qhttpnetworkconnection.cpp +++ b/src/network/access/qhttpnetworkconnection.cpp @@ -85,6 +85,7 @@ QHttpNetworkConnectionPrivate::QHttpNetworkConnectionPrivate(const QString &host #ifndef QT_NO_NETWORKPROXY , networkProxy(QNetworkProxy::NoProxy) #endif + , preConnectRequests(0) { channels = new QHttpNetworkConnectionChannel[channelCount]; } @@ -96,6 +97,7 @@ QHttpNetworkConnectionPrivate::QHttpNetworkConnectionPrivate(quint16 channelCoun #ifndef QT_NO_NETWORKPROXY , networkProxy(QNetworkProxy::NoProxy) #endif + , preConnectRequests(0) { channels = new QHttpNetworkConnectionChannel[channelCount]; } @@ -541,6 +543,9 @@ QHttpNetworkReply* QHttpNetworkConnectionPrivate::queueRequest(const QHttpNetwor reply->d_func()->connectionChannel = &channels[0]; // will have the correct one set later HttpMessagePair pair = qMakePair(request, reply); + if (request.isPreConnect()) + preConnectRequests++; + switch (request.priority()) { case QHttpNetworkRequest::HighPriority: highPriorityQueue.prepend(pair); @@ -925,15 +930,24 @@ void QHttpNetworkConnectionPrivate::_q_startNextRequest() // If there is not already any connected channels we need to connect a new one. // We do not pair the channel with the request until we know if it is // connected or not. This is to reuse connected channels before we connect new once. - int queuedRequest = highPriorityQueue.count() + lowPriorityQueue.count(); - for (int i = 0; i < channelCount; ++i) { + int queuedRequests = highPriorityQueue.count() + lowPriorityQueue.count(); + + // in case we have in-flight preconnect requests and normal requests, + // we only need one socket for each (preconnect, normal request) pair + int neededOpenChannels = queuedRequests; + if (preConnectRequests > 0) { + int normalRequests = queuedRequests - preConnectRequests; + neededOpenChannels = qMax(normalRequests, preConnectRequests); + } + for (int i = 0; i < channelCount && neededOpenChannels > 0; ++i) { bool connectChannel = false; if (channels[i].socket) { if ((channels[i].socket->state() == QAbstractSocket::ConnectingState) || (channels[i].socket->state() == QAbstractSocket::HostLookupState) || channels[i].pendingEncrypt) // pendingEncrypt == "EncryptingState" - queuedRequest--; - if ( queuedRequest <=0 ) + neededOpenChannels--; + + if (neededOpenChannels <= 0) break; if (!channels[i].reply && !channels[i].isSocketBusy() && (channels[i].socket->state() == QAbstractSocket::UnconnectedState)) connectChannel = true; @@ -947,11 +961,8 @@ void QHttpNetworkConnectionPrivate::_q_startNextRequest() else if (networkLayerState == IPv6) channels[i].networkLayerPreference = QAbstractSocket::IPv6Protocol; channels[i].ensureConnection(); - queuedRequest--; + neededOpenChannels--; } - - if ( queuedRequest <=0 ) - break; } } @@ -1272,6 +1283,11 @@ void QHttpNetworkConnection::ignoreSslErrors(const QList &errors, int #endif //QT_NO_SSL +void QHttpNetworkConnection::preConnectFinished() +{ + d_func()->preConnectRequests--; +} + #ifndef QT_NO_NETWORKPROXY // only called from QHttpNetworkConnectionChannel::_q_proxyAuthenticationRequired, not // from QHttpNetworkConnectionChannel::handleAuthenticationChallenge diff --git a/src/network/access/qhttpnetworkconnection_p.h b/src/network/access/qhttpnetworkconnection_p.h index 956499ddab..c54250f6ed 100644 --- a/src/network/access/qhttpnetworkconnection_p.h +++ b/src/network/access/qhttpnetworkconnection_p.h @@ -131,6 +131,8 @@ public: void setSslContext(QSharedPointer context); #endif + void preConnectFinished(); + private: Q_DECLARE_PRIVATE(QHttpNetworkConnection) Q_DISABLE_COPY(QHttpNetworkConnection) @@ -239,6 +241,8 @@ public: QList highPriorityQueue; QList lowPriorityQueue; + int preConnectRequests; + #ifndef QT_NO_SSL QSharedPointer sslContext; #endif diff --git a/src/network/access/qhttpnetworkconnectionchannel.cpp b/src/network/access/qhttpnetworkconnectionchannel.cpp index a756f22937..1c7a61dca6 100644 --- a/src/network/access/qhttpnetworkconnectionchannel.cpp +++ b/src/network/access/qhttpnetworkconnectionchannel.cpp @@ -214,6 +214,7 @@ bool QHttpNetworkConnectionChannel::sendRequest() state = QHttpNetworkConnectionChannel::IdleState; reply->d_func()->state = QHttpNetworkReplyPrivate::AllDoneState; allDone(); + connection->preConnectFinished(); // will only decrease the counter reply = 0; // so we can reuse this channel return true; // we have a working connection and are done } diff --git a/src/network/access/qhttpnetworkrequest.cpp b/src/network/access/qhttpnetworkrequest.cpp index e5b2eced99..f26559ce9b 100644 --- a/src/network/access/qhttpnetworkrequest.cpp +++ b/src/network/access/qhttpnetworkrequest.cpp @@ -49,7 +49,8 @@ QT_BEGIN_NAMESPACE QHttpNetworkRequestPrivate::QHttpNetworkRequestPrivate(QHttpNetworkRequest::Operation op, QHttpNetworkRequest::Priority pri, const QUrl &newUrl) : QHttpNetworkHeaderPrivate(newUrl), operation(op), priority(pri), uploadByteDevice(0), - autoDecompress(false), pipeliningAllowed(false), withCredentials(true) + autoDecompress(false), pipeliningAllowed(false), withCredentials(true), + preConnect(false) { } @@ -64,6 +65,7 @@ QHttpNetworkRequestPrivate::QHttpNetworkRequestPrivate(const QHttpNetworkRequest customVerb = other.customVerb; withCredentials = other.withCredentials; ssl = other.ssl; + preConnect = other.preConnect; } QHttpNetworkRequestPrivate::~QHttpNetworkRequestPrivate() @@ -75,7 +77,8 @@ bool QHttpNetworkRequestPrivate::operator==(const QHttpNetworkRequestPrivate &ot return QHttpNetworkHeaderPrivate::operator==(other) && (operation == other.operation) && (ssl == other.ssl) - && (uploadByteDevice == other.uploadByteDevice); + && (uploadByteDevice == other.uploadByteDevice) + && (preConnect == other.preConnect); } QByteArray QHttpNetworkRequestPrivate::methodName() const @@ -205,6 +208,15 @@ void QHttpNetworkRequest::setSsl(bool s) d->ssl = s; } +bool QHttpNetworkRequest::isPreConnect() const +{ + return d->preConnect; +} +void QHttpNetworkRequest::setPreConnect(bool preConnect) +{ + d->preConnect = preConnect; +} + qint64 QHttpNetworkRequest::contentLength() const { return d->contentLength(); diff --git a/src/network/access/qhttpnetworkrequest_p.h b/src/network/access/qhttpnetworkrequest_p.h index fc4a6928c6..ce9fbb1509 100644 --- a/src/network/access/qhttpnetworkrequest_p.h +++ b/src/network/access/qhttpnetworkrequest_p.h @@ -120,6 +120,9 @@ public: bool isSsl() const; void setSsl(bool); + bool isPreConnect() const; + void setPreConnect(bool preConnect); + void setUploadByteDevice(QNonContiguousByteDevice *bd); QNonContiguousByteDevice* uploadByteDevice() const; @@ -151,6 +154,7 @@ public: bool pipeliningAllowed; bool withCredentials; bool ssl; + bool preConnect; }; diff --git a/src/network/access/qnetworkreplyhttpimpl.cpp b/src/network/access/qnetworkreplyhttpimpl.cpp index 443339f029..cd2f9e11ed 100644 --- a/src/network/access/qnetworkreplyhttpimpl.cpp +++ b/src/network/access/qnetworkreplyhttpimpl.cpp @@ -635,6 +635,9 @@ void QNetworkReplyHttpImplPrivate::postRequest() q->setAttribute(QNetworkRequest::ConnectionEncryptedAttribute, ssl); httpRequest.setSsl(ssl); + bool preConnect = (scheme == QLatin1String("preconnect-http") + || scheme == QLatin1String("preconnect-https")); + httpRequest.setPreConnect(preConnect); #ifndef QT_NO_NETWORKPROXY QNetworkProxy transparentProxy, cacheProxy; diff --git a/tests/benchmarks/network/access/qnetworkreply/tst_qnetworkreply.cpp b/tests/benchmarks/network/access/qnetworkreply/tst_qnetworkreply.cpp index 860bd3cf4d..a99fd17a64 100644 --- a/tests/benchmarks/network/access/qnetworkreply/tst_qnetworkreply.cpp +++ b/tests/benchmarks/network/access/qnetworkreply/tst_qnetworkreply.cpp @@ -463,6 +463,7 @@ private slots: #ifndef QT_NO_SSL void echoPerformance_data(); void echoPerformance(); + void preConnectEncrypted_data(); void preConnectEncrypted(); #endif @@ -476,6 +477,7 @@ private slots: void httpDownloadPerformanceDownloadBuffer(); void httpsRequestChain(); void httpsUpload(); + void preConnect_data(); void preConnect(); private: @@ -548,6 +550,13 @@ void tst_qnetworkreply::echoPerformance() } } +void tst_qnetworkreply::preConnectEncrypted_data() +{ + QTest::addColumn("sleepTime"); + QTest::newRow("2secs") << 2000; // to start a new request after preconnecting is done + QTest::newRow("100ms") << 100; // to start a new request while preconnecting is in-flight +} + void tst_qnetworkreply::preConnectEncrypted() { QString hostName = QLatin1String("www.google.com"); @@ -579,8 +588,9 @@ void tst_qnetworkreply::preConnectEncrypted() manager.clearAccessCache(); // now try to make the connection beforehand + QFETCH(int, sleepTime); manager.connectToHostEncrypted(hostName); - QTestEventLoop::instance().enterLoop(2); + QTestEventLoop::instance().enterLoopMSecs(sleepTime); // now make another request and hopefully use the existing connection QPair preConnectResult = runGetRequest(&manager, request); @@ -917,6 +927,11 @@ void tst_qnetworkreply::httpsUpload() } } +void tst_qnetworkreply::preConnect_data() +{ + preConnectEncrypted_data(); +} + void tst_qnetworkreply::preConnect() { QString hostName = QLatin1String("www.google.com"); @@ -948,8 +963,9 @@ void tst_qnetworkreply::preConnect() manager.clearAccessCache(); // now try to make the connection beforehand + QFETCH(int, sleepTime); manager.connectToHost(hostName); - QTestEventLoop::instance().enterLoop(2); + QTestEventLoop::instance().enterLoopMSecs(sleepTime); // now make another request and hopefully use the existing connection QPair preConnectResult = runGetRequest(&manager, request);