QNAM: try to send headers and body together
For HTTP connections, QNAM defaults to opening its TCP socket in unbuffered mode. This means that Qt will send the data written into the socket right to the kernel, queueing only if the kernel says it doesn't want more data for the moment being. QNAM itself then uses separate write() calls to write the HTTP headers and the body of the request (like POST or PUT). These 2+ writes result in headers and body being sent over different TCP segments -- even if, in principle, a POST with a few bytes of data (e.g. a HTML form, or a REST or SOAP request) could fit in the same segment as the request. Multiple writes like this interact extremely poorly with other TCP features, e.g. delayed ACKs, Nagle's algorithm and the like. In a typical scenario, the kernel will send a segment containing just the headers, wait for the ACK (which may be delayed), and only then send the body (it wasn't sent before because Nagle was blocking it). The reply at this point is immediate (because the server can process the request and starts replying), but the delayed ACK is typically 40-50ms, and documented up to 500ms (!). If one uses QNAM to access a service, this introduces unacceptable latency. These multiple writes to the OS should be avoided. The first thing that comes into mind is to use buffered sockets. Now, there are good reasons to keep the socket unbuffered, so we don't want to change that. But the deal breaker is that even buffered sockets won't help in general: for instance, on Windows, a buffered write will immediately detect that the socket is ready for write and flush the buffer right away (not 100% sure of why this is necessary; basically, after populating the QTcpSocket write buffer, Qt enables a write socket notifier on the socket -- notifier that fires synchronously and immediately, without even returning to the event loop, and that causes the write buffer flush). Linux of course offers the perfect solution: corking the socket via TCP_CORK, which tells the kernel not to send the data right away but to buffer it up to a timeout (or when the option gets disabled again, whichever comes first). It's explicitly designed to support the case of sending headers followed by something like a sendfile(2). Setting this socket option moves the problem to the kernel and we could happily keep issuing multiple writes. Ça va sans dire, no other OS supports that option or any other similar option. We have therefore to deal with this in userspace: don't write in the socket multiple times, but try and coalesce the write of the headers with the writing of the data. This patch implements that, by storing the headers and sending them together with the very first chunk of data. If the data is small enough, this sends the entire request in one TCP segment. Interestingly enough, QNAM has a call setting TCP_NODELAY currently commented out because Qt doesn't combine "HTTP requests" (whatever that means). The call comes all the way back from pre-public history (before 2011) (!). This patch doesn't touch it. Fixes: QTBUG-41907 Change-Id: Id555d14e0702c9f75c3134b18277692eb3659afe Reviewed-by: Mårten Nordheim <marten.nordheim@qt.io>
This commit is contained in:
parent
354ea7bd96
commit
7a5e0c5712
@ -313,12 +313,11 @@ bool QHttpProtocolHandler::sendRequest()
|
||||
if (m_channel->request.withCredentials())
|
||||
m_connection->d_func()->createAuthorization(m_socket, m_channel->request);
|
||||
#ifndef QT_NO_NETWORKPROXY
|
||||
QByteArray header = QHttpNetworkRequestPrivate::header(m_channel->request,
|
||||
m_header = QHttpNetworkRequestPrivate::header(m_channel->request,
|
||||
(m_connection->d_func()->networkProxy.type() != QNetworkProxy::NoProxy));
|
||||
#else
|
||||
QByteArray header = QHttpNetworkRequestPrivate::header(m_channel->request, false);
|
||||
m_header = QHttpNetworkRequestPrivate::header(m_channel->request, false);
|
||||
#endif
|
||||
m_socket->write(header);
|
||||
// flushing is dangerous (QSslSocket calls transmit which might read or error)
|
||||
// m_socket->flush();
|
||||
QNonContiguousByteDevice* uploadByteDevice = m_channel->request.uploadByteDevice();
|
||||
@ -331,6 +330,8 @@ bool QHttpProtocolHandler::sendRequest()
|
||||
m_channel->state = QHttpNetworkConnectionChannel::WritingState; // start writing data
|
||||
sendRequest(); //recurse
|
||||
} else {
|
||||
// no data to send: just send the HTTP headers
|
||||
m_socket->write(qExchange(m_header, {}));
|
||||
m_channel->state = QHttpNetworkConnectionChannel::WaitingState; // now wait for response
|
||||
sendRequest(); //recurse
|
||||
}
|
||||
@ -342,6 +343,10 @@ bool QHttpProtocolHandler::sendRequest()
|
||||
// write the data
|
||||
QNonContiguousByteDevice* uploadByteDevice = m_channel->request.uploadByteDevice();
|
||||
if (!uploadByteDevice || m_channel->bytesTotal == m_channel->written) {
|
||||
// the upload device might have no data to send, but we still have to send the headers,
|
||||
// do it now.
|
||||
if (!m_header.isEmpty())
|
||||
m_socket->write(qExchange(m_header, {}));
|
||||
if (uploadByteDevice)
|
||||
emit m_reply->dataSendProgress(m_channel->written, m_channel->bytesTotal);
|
||||
m_channel->state = QHttpNetworkConnectionChannel::WaitingState; // now wait for response
|
||||
@ -349,24 +354,31 @@ bool QHttpProtocolHandler::sendRequest()
|
||||
break;
|
||||
}
|
||||
|
||||
// only feed the QTcpSocket buffer when there is less than 32 kB in it
|
||||
// only feed the QTcpSocket buffer when there is less than 32 kB in it;
|
||||
// note that the headers do not count towards these limits.
|
||||
const qint64 socketBufferFill = 32*1024;
|
||||
const qint64 socketWriteMaxSize = 16*1024;
|
||||
|
||||
|
||||
// if it is really an ssl socket, check more than just bytesToWrite()
|
||||
#ifndef QT_NO_SSL
|
||||
QSslSocket *sslSocket = qobject_cast<QSslSocket*>(m_socket);
|
||||
// if it is really an ssl socket, check more than just bytesToWrite()
|
||||
while ((m_socket->bytesToWrite() + (sslSocket ? sslSocket->encryptedBytesToWrite() : 0))
|
||||
<= socketBufferFill && m_channel->bytesTotal != m_channel->written)
|
||||
const auto encryptedBytesToWrite = [sslSocket]() -> qint64
|
||||
{
|
||||
return sslSocket ? sslSocket->encryptedBytesToWrite() : 0;
|
||||
};
|
||||
#else
|
||||
while (m_socket->bytesToWrite() <= socketBufferFill
|
||||
&& m_channel->bytesTotal != m_channel->written)
|
||||
const auto encryptedBytesToWrite = [](){ return qint64(0); };
|
||||
#endif
|
||||
|
||||
// throughout this loop, we want to send the data coming from uploadByteDevice.
|
||||
// we also need to send the headers, as we try to coalesce their write with the data.
|
||||
// we won't send more than the limits above, excluding the headers
|
||||
while ((m_socket->bytesToWrite() + encryptedBytesToWrite()) <= socketBufferFill
|
||||
&& m_channel->bytesTotal != m_channel->written)
|
||||
{
|
||||
// get pointer to upload data
|
||||
qint64 currentReadSize = 0;
|
||||
qint64 desiredReadSize = qMin(socketWriteMaxSize, m_channel->bytesTotal - m_channel->written);
|
||||
const qint64 desiredReadSize = qMin(socketWriteMaxSize, m_channel->bytesTotal - m_channel->written);
|
||||
const char *readPointer = uploadByteDevice->readPointer(desiredReadSize, currentReadSize);
|
||||
|
||||
if (currentReadSize == -1) {
|
||||
@ -384,7 +396,17 @@ bool QHttpProtocolHandler::sendRequest()
|
||||
m_connection->d_func()->emitReplyError(m_socket, m_reply, QNetworkReply::ProtocolFailure);
|
||||
return false;
|
||||
}
|
||||
qint64 currentWriteSize = m_socket->write(readPointer, currentReadSize);
|
||||
qint64 currentWriteSize;
|
||||
if (m_header.isEmpty()) {
|
||||
currentWriteSize = m_socket->write(readPointer, currentReadSize);
|
||||
} else {
|
||||
// assemble header and data and send them together
|
||||
const qint64 headerSize = m_header.size();
|
||||
m_header.append(readPointer, currentReadSize);
|
||||
currentWriteSize = m_socket->write(qExchange(m_header, {}));
|
||||
if (currentWriteSize != -1)
|
||||
currentWriteSize -= headerSize;
|
||||
}
|
||||
if (currentWriteSize == -1 || currentWriteSize != currentReadSize) {
|
||||
// socket broke down
|
||||
m_connection->d_func()->emitReplyError(m_socket, m_reply, QNetworkReply::UnknownNetworkError);
|
||||
|
@ -55,6 +55,8 @@
|
||||
#include <QtNetwork/private/qtnetworkglobal_p.h>
|
||||
#include <private/qabstractprotocolhandler_p.h>
|
||||
|
||||
#include <QtCore/qbytearray.h>
|
||||
|
||||
QT_REQUIRE_CONFIG(http);
|
||||
|
||||
QT_BEGIN_NAMESPACE
|
||||
@ -67,6 +69,8 @@ private:
|
||||
virtual void _q_receiveReply() override;
|
||||
virtual void _q_readyRead() override;
|
||||
virtual bool sendRequest() override;
|
||||
|
||||
QByteArray m_header;
|
||||
};
|
||||
|
||||
QT_END_NAMESPACE
|
||||
|
Loading…
Reference in New Issue
Block a user