From bf6897edb01c0d46fda5195e36bb529dbbf7e5dc Mon Sep 17 00:00:00 2001 From: Martin Petersson Date: Wed, 30 May 2012 17:03:57 +0200 Subject: QtNetwork: remove double buffering on sockets Removes the readBuffer from the QAbstractSocket since data is already buffered in the QIODevice. Change-Id: I4e50b791fd2852455e526fa2c07089d4d3f0b2a4 Reviewed-by: Prasanth Ullattil Reviewed-by: Thiago Macieira --- src/network/socket/qabstractsocket.cpp | 157 ++++++++----------------------- src/network/socket/qabstractsocket_p.h | 1 - src/network/socket/qlocalsocket_tcp.cpp | 2 +- src/network/socket/qlocalsocket_unix.cpp | 2 +- src/network/ssl/qsslsocket.cpp | 29 +++--- src/network/ssl/qsslsocket_openssl.cpp | 5 +- 6 files changed, 55 insertions(+), 141 deletions(-) diff --git a/src/network/socket/qabstractsocket.cpp b/src/network/socket/qabstractsocket.cpp index 8d1c134d7d..3959419fed 100644 --- a/src/network/socket/qabstractsocket.cpp +++ b/src/network/socket/qabstractsocket.cpp @@ -548,7 +548,6 @@ QAbstractSocketPrivate::QAbstractSocketPrivate() socketEngine(0), cachedSocketDescriptor(-1), readBufferMaxSize(0), - readBuffer(QABSTRACTSOCKET_BUFFERSIZE), writeBuffer(QABSTRACTSOCKET_BUFFERSIZE), isBuffered(false), blockingTimeout(30000), @@ -684,7 +683,7 @@ bool QAbstractSocketPrivate::canReadNotification() qint64 newBytes = 0; if (isBuffered) { // Return if there is no space in the buffer - if (readBufferMaxSize && readBuffer.size() >= readBufferMaxSize) { + if (readBufferMaxSize && buffer.size() >= readBufferMaxSize) { #if defined (QABSTRACTSOCKET_DEBUG) qDebug("QAbstractSocketPrivate::canReadNotification() buffer is full"); #endif @@ -693,7 +692,7 @@ bool QAbstractSocketPrivate::canReadNotification() // If reading from the socket fails after getting a read // notification, close the socket. - newBytes = readBuffer.size(); + newBytes = buffer.size(); if (!readFromSocket()) { #if defined (QABSTRACTSOCKET_DEBUG) qDebug("QAbstractSocketPrivate::canReadNotification() disconnecting socket"); @@ -701,10 +700,10 @@ bool QAbstractSocketPrivate::canReadNotification() q->disconnectFromHost(); return false; } - newBytes = readBuffer.size() - newBytes; + newBytes = buffer.size() - newBytes; // If read buffer is full, disable the read socket notifier. - if (readBufferMaxSize && readBuffer.size() == readBufferMaxSize) { + if (readBufferMaxSize && buffer.size() == readBufferMaxSize) { socketEngine->setReadNotificationEnabled(false); } } @@ -1221,8 +1220,8 @@ bool QAbstractSocketPrivate::readFromSocket() // host has _not_ disappeared). bytesToRead = 4096; } - if (readBufferMaxSize && bytesToRead > (readBufferMaxSize - readBuffer.size())) - bytesToRead = readBufferMaxSize - readBuffer.size(); + if (readBufferMaxSize && bytesToRead > (readBufferMaxSize - buffer.size())) + bytesToRead = readBufferMaxSize - buffer.size(); #if defined(QABSTRACTSOCKET_DEBUG) qDebug("QAbstractSocketPrivate::readFromSocket() about to read %d bytes", @@ -1230,17 +1229,17 @@ bool QAbstractSocketPrivate::readFromSocket() #endif // Read from the socket, store data in the read buffer. - char *ptr = readBuffer.reserve(bytesToRead); + char *ptr = buffer.reserve(bytesToRead); qint64 readBytes = socketEngine->read(ptr, bytesToRead); if (readBytes == -2) { // No bytes currently available for reading. - readBuffer.chop(bytesToRead); + buffer.chop(bytesToRead); return true; } - readBuffer.chop(int(bytesToRead - (readBytes < 0 ? qint64(0) : readBytes))); + buffer.chop(int(bytesToRead - (readBytes < 0 ? qint64(0) : readBytes))); #if defined(QABSTRACTSOCKET_DEBUG) qDebug("QAbstractSocketPrivate::readFromSocket() got %d bytes, buffer size = %d", - int(readBytes), readBuffer.size()); + int(readBytes), buffer.size()); #endif if (!socketEngine->isValid()) { @@ -1563,7 +1562,7 @@ void QAbstractSocket::connectToHost(const QString &hostName, quint16 port, d->hostName = hostName; d->port = port; d->state = UnconnectedState; - d->readBuffer.clear(); + d->buffer.clear(); d->writeBuffer.clear(); d->abortCalled = false; d->closeCalled = false; @@ -1675,8 +1674,6 @@ qint64 QAbstractSocket::bytesAvailable() const Q_D(const QAbstractSocket); qint64 available = QIODevice::bytesAvailable(); - available += (qint64) d->readBuffer.size(); - if (!d->isBuffered && d->socketEngine && d->socketEngine->isValid()) available += d->socketEngine->bytesAvailable(); @@ -1758,10 +1755,10 @@ QString QAbstractSocket::peerName() const */ bool QAbstractSocket::canReadLine() const { - bool hasLine = d_func()->readBuffer.canReadLine(); + bool hasLine = d_func()->buffer.canReadLine(); #if defined (QABSTRACTSOCKET_DEBUG) qDebug("QAbstractSocket::canReadLine() == %s, buffer size = %d, size = %d", hasLine ? "true" : "false", - d_func()->readBuffer.size(), d_func()->buffer.size()); + d_func()->buffer.size(), d_func()->buffer.size()); #endif return hasLine || QIODevice::canReadLine(); } @@ -2290,7 +2287,7 @@ bool QAbstractSocket::isSequential() const */ bool QAbstractSocket::atEnd() const { - return QIODevice::atEnd() && (!isOpen() || d_func()->readBuffer.isEmpty()); + return QIODevice::atEnd() && (!isOpen() || d_func()->buffer.isEmpty()); } /*! @@ -2328,114 +2325,38 @@ qint64 QAbstractSocket::readData(char *data, qint64 maxSize) Q_D(QAbstractSocket); // This is for a buffered QTcpSocket - if (d->isBuffered && d->readBuffer.isEmpty()) + if (d->isBuffered && d->buffer.isEmpty()) // if we're still connected, return 0 indicating there may be more data in the future // if we're not connected, return -1 indicating EOF return d->state == QAbstractSocket::ConnectedState ? qint64(0) : qint64(-1); - // short cut for a char read if we have something in the buffer - if (maxSize == 1 && !d->readBuffer.isEmpty()) { - *data = d->readBuffer.getChar(); -#if defined (QABSTRACTSOCKET_DEBUG) - qDebug("QAbstractSocket::readData(%p '%c (0x%.2x)', 1) == 1 [char buffer]", - data, isprint(int(uchar(*data))) ? *data : '?', *data); -#endif - if (d->readBuffer.isEmpty() && d->socketEngine && d->socketEngine->isValid()) - d->socketEngine->setReadNotificationEnabled(true); - return 1; - } - - // Special case for an Unbuffered QTcpSocket - // Re-filling the buffer. - if (d->socketType == TcpSocket - && !d->isBuffered - && d->readBuffer.size() < maxSize - && d->readBufferMaxSize > 0 - && maxSize < d->readBufferMaxSize - && d->socketEngine - && d->socketEngine->isValid()) { - // Our buffer is empty and a read() was requested for a byte amount that is smaller - // than the readBufferMaxSize. This means that we should fill our buffer since we want - // such small reads come from the buffer and not always go to the costly socket engine read() - qint64 bytesToRead = d->socketEngine->bytesAvailable(); - if (bytesToRead > 0) { - char *ptr = d->readBuffer.reserve(bytesToRead); - qint64 readBytes = d->socketEngine->read(ptr, bytesToRead); - if (readBytes == -2) { - // No bytes currently available for reading. - d->readBuffer.chop(bytesToRead); - } else { - d->readBuffer.chop(int(bytesToRead - (readBytes < 0 ? qint64(0) : readBytes))); - } - } - } - - // First try to satisfy the read from the buffer - qint64 bytesToRead = qMin(qint64(d->readBuffer.size()), maxSize); - qint64 readSoFar = 0; - while (readSoFar < bytesToRead) { - const char *ptr = d->readBuffer.readPointer(); - int bytesToReadFromThisBlock = qMin(int(bytesToRead - readSoFar), - d->readBuffer.nextDataBlockSize()); - memcpy(data + readSoFar, ptr, bytesToReadFromThisBlock); - readSoFar += bytesToReadFromThisBlock; - d->readBuffer.free(bytesToReadFromThisBlock); - } - - if (d->socketEngine && !d->socketEngine->isReadNotificationEnabled() && d->socketEngine->isValid()) + if (!d->socketEngine) + return -1; // no socket engine is probably EOF + if (!d->socketEngine->isValid()) + return -1; // This is for unbuffered TCP when we already had been disconnected + if (d->state != QAbstractSocket::ConnectedState) + return -1; // This is for unbuffered TCP if we're not connected yet + qint64 readBytes = d->socketEngine->read(data, maxSize); + if (readBytes == -2) { + // -2 from the engine means no bytes available (EAGAIN) so read more later + return 0; + } else if (readBytes < 0) { + d->socketError = d->socketEngine->error(); + setErrorString(d->socketEngine->errorString()); + d->resetSocketLayer(); + d->state = QAbstractSocket::UnconnectedState; + } else if (!d->socketEngine->isReadNotificationEnabled()) { + // Only do this when there was no error d->socketEngine->setReadNotificationEnabled(true); - - if (readSoFar > 0) { - // we read some data from buffer. - // Just return, readyRead will be emitted again -#if defined (QABSTRACTSOCKET_DEBUG) - qDebug("QAbstractSocket::readData(%p '%c (0x%.2x)', %lli) == %lli [buffer]", - data, isprint(int(uchar(*data))) ? *data : '?', *data, maxSize, readSoFar); -#endif - - if (d->readBuffer.isEmpty() && d->socketEngine) - d->socketEngine->setReadNotificationEnabled(true); - return readSoFar; - } - - // This code path is for Unbuffered QTcpSocket or for connected UDP - - if (!d->isBuffered) { - if (!d->socketEngine) - return -1; // no socket engine is probably EOF - if (!d->socketEngine->isValid()) - return -1; // This is for unbuffered TCP when we already had been disconnected - if (d->state != QAbstractSocket::ConnectedState) - return -1; // This is for unbuffered TCP if we're not connected yet - qint64 readBytes = d->socketEngine->read(data, maxSize); - if (readBytes == -2) { - // -2 from the engine means no bytes available (EAGAIN) so read more later - return 0; - } else if (readBytes < 0) { - d->socketError = d->socketEngine->error(); - setErrorString(d->socketEngine->errorString()); - d->resetSocketLayer(); - d->state = QAbstractSocket::UnconnectedState; - } else if (!d->socketEngine->isReadNotificationEnabled()) { - // Only do this when there was no error - d->socketEngine->setReadNotificationEnabled(true); - } - -#if defined (QABSTRACTSOCKET_DEBUG) - qDebug("QAbstractSocket::readData(%p \"%s\", %lli) == %lld [engine]", - data, qt_prettyDebug(data, 32, readBytes).data(), maxSize, - readBytes); -#endif - return readBytes; } - #if defined (QABSTRACTSOCKET_DEBUG) - qDebug("QAbstractSocket::readData(%p \"%s\", %lli) == %lld [unreachable]", - data, qt_prettyDebug(data, qMin(32, readSoFar), readSoFar).data(), - maxSize, readSoFar); + qDebug("QAbstractSocket::readData(%p \"%s\", %lli) == %lld [engine]", + data, qt_prettyDebug(data, 32, readBytes).data(), maxSize, + readBytes); #endif - return readSoFar; + return readBytes; + } /*! \reimp @@ -2754,7 +2675,7 @@ void QAbstractSocket::disconnectFromHost() #if defined(QABSTRACTSOCKET_DEBUG) qDebug("QAbstractSocket::disconnectFromHost() closed!"); #endif - d->readBuffer.clear(); + d->buffer.clear(); d->writeBuffer.clear(); QIODevice::close(); } @@ -2808,7 +2729,7 @@ void QAbstractSocket::setReadBufferSize(qint64 size) // ensure that the read notification is enabled if we've now got // room in the read buffer // but only if we're not inside canReadNotification -- that will take care on its own - if ((size == 0 || d->readBuffer.size() < size) && d->state == QAbstractSocket::ConnectedState) // Do not change the notifier unless we are connected. + if ((size == 0 || d->buffer.size() < size) && d->state == QAbstractSocket::ConnectedState) // Do not change the notifier unless we are connected. d->socketEngine->setReadNotificationEnabled(true); } } diff --git a/src/network/socket/qabstractsocket_p.h b/src/network/socket/qabstractsocket_p.h index 578213f6de..21d85f74fe 100644 --- a/src/network/socket/qabstractsocket_p.h +++ b/src/network/socket/qabstractsocket_p.h @@ -141,7 +141,6 @@ public: bool readFromSocket(); qint64 readBufferMaxSize; - QRingBuffer readBuffer; QRingBuffer writeBuffer; bool isBuffered; diff --git a/src/network/socket/qlocalsocket_tcp.cpp b/src/network/socket/qlocalsocket_tcp.cpp index fb9011f0f2..41553608ba 100644 --- a/src/network/socket/qlocalsocket_tcp.cpp +++ b/src/network/socket/qlocalsocket_tcp.cpp @@ -299,7 +299,7 @@ qintptr QLocalSocket::socketDescriptor() const qint64 QLocalSocket::readData(char *data, qint64 c) { Q_D(QLocalSocket); - return d->tcpSocket->readData(data, c); + return d->tcpSocket->read(data, c); } qint64 QLocalSocket::writeData(const char *data, qint64 c) diff --git a/src/network/socket/qlocalsocket_unix.cpp b/src/network/socket/qlocalsocket_unix.cpp index 7b728be38d..29c4ceedcc 100644 --- a/src/network/socket/qlocalsocket_unix.cpp +++ b/src/network/socket/qlocalsocket_unix.cpp @@ -401,7 +401,7 @@ qintptr QLocalSocket::socketDescriptor() const qint64 QLocalSocket::readData(char *data, qint64 c) { Q_D(QLocalSocket); - return d->unixSocket.readData(data, c); + return d->unixSocket.read(data, c); } qint64 QLocalSocket::writeData(const char *data, qint64 c) diff --git a/src/network/ssl/qsslsocket.cpp b/src/network/ssl/qsslsocket.cpp index 2dee60408b..b2547487ba 100644 --- a/src/network/ssl/qsslsocket.cpp +++ b/src/network/ssl/qsslsocket.cpp @@ -707,7 +707,7 @@ qint64 QSslSocket::bytesAvailable() const Q_D(const QSslSocket); if (d->mode == UnencryptedMode) return QIODevice::bytesAvailable() + (d->plainSocket ? d->plainSocket->bytesAvailable() : 0); - return QIODevice::bytesAvailable() + d->readBuffer.size(); + return QIODevice::bytesAvailable(); } /*! @@ -764,7 +764,7 @@ bool QSslSocket::canReadLine() const Q_D(const QSslSocket); if (d->mode == UnencryptedMode) return QIODevice::canReadLine() || (d->plainSocket && d->plainSocket->canReadLine()); - return QIODevice::canReadLine() || (!d->readBuffer.isEmpty() && d->readBuffer.canReadLine()); + return QIODevice::canReadLine(); } /*! @@ -781,12 +781,8 @@ void QSslSocket::close() QTcpSocket::close(); // must be cleared, reading/writing not possible on closed socket: - d->readBuffer.clear(); + d->buffer.clear(); d->writeBuffer.clear(); - // for QTcpSocket this is already done because it uses the readBuffer/writeBuffer - // if the QIODevice it is based on - // ### FIXME QSslSocket should probably do similar instead of having - // its own readBuffer/writeBuffer } /*! @@ -797,7 +793,7 @@ bool QSslSocket::atEnd() const Q_D(const QSslSocket); if (d->mode == UnencryptedMode) return QIODevice::atEnd() && (!d->plainSocket || d->plainSocket->atEnd()); - return QIODevice::atEnd() && d->readBuffer.isEmpty(); + return QIODevice::atEnd(); } /*! @@ -1829,21 +1825,18 @@ qint64 QSslSocket::readData(char *data, qint64 maxlen) if (d->mode == UnencryptedMode && !d->autoStartHandshake) { readBytes = d->plainSocket->read(data, maxlen); } else { - do { - const char *readPtr = d->readBuffer.readPointer(); - int bytesToRead = qMin(maxlen - readBytes, d->readBuffer.nextDataBlockSize()); - ::memcpy(data + readBytes, readPtr, bytesToRead); - readBytes += bytesToRead; - d->readBuffer.free(bytesToRead); - } while (!d->readBuffer.isEmpty() && readBytes < maxlen); + int bytesToRead = qMin(maxlen, d->buffer.size()); + readBytes = d->buffer.read(data, bytesToRead); } + #ifdef QSSLSOCKET_DEBUG qDebug() << "QSslSocket::readData(" << (void *)data << ',' << maxlen << ") ==" << readBytes; #endif // possibly trigger another transmit() to decrypt more data from the socket - if (d->readBuffer.isEmpty() && d->plainSocket->bytesAvailable()) + if (d->buffer.isEmpty() && d->plainSocket->bytesAvailable()) { QMetaObject::invokeMethod(this, "_q_flushReadBuffer", Qt::QueuedConnection); + } return readBytes; } @@ -1907,7 +1900,7 @@ void QSslSocketPrivate::init() // that it is possible setting it before connecting // ignoreErrorsList.clear(); - readBuffer.clear(); + buffer.clear(); writeBuffer.clear(); configuration.peerCertificate.clear(); configuration.peerCertificateChain.clear(); @@ -2112,7 +2105,7 @@ void QSslSocketPrivate::createPlainSocket(QIODevice::OpenMode openMode) q, SIGNAL(proxyAuthenticationRequired(QNetworkProxy,QAuthenticator*))); #endif - readBuffer.clear(); + buffer.clear(); writeBuffer.clear(); connectionEncrypted = false; configuration.peerCertificate.clear(); diff --git a/src/network/ssl/qsslsocket_openssl.cpp b/src/network/ssl/qsslsocket_openssl.cpp index 38368583ff..83071f2c7a 100644 --- a/src/network/ssl/qsslsocket_openssl.cpp +++ b/src/network/ssl/qsslsocket_openssl.cpp @@ -953,12 +953,13 @@ void QSslSocketBackendPrivate::transmit() } // Check if we've got any data to be read from the socket. - if (!connectionEncrypted || !readBufferMaxSize || readBuffer.size() < readBufferMaxSize) + if (!connectionEncrypted || !readBufferMaxSize || buffer.size() < readBufferMaxSize) while ((pendingBytes = plainSocket->bytesAvailable()) > 0) { // Read encrypted data from the socket into a buffer. data.resize(pendingBytes); // just peek() here because q_BIO_write could write less data than expected int encryptedBytesRead = plainSocket->peek(data.data(), pendingBytes); + #ifdef QSSLSOCKET_DEBUG qDebug() << "QSslSocketBackendPrivate::transmit: read" << encryptedBytesRead << "encrypted bytes from the socket"; #endif @@ -1025,7 +1026,7 @@ void QSslSocketBackendPrivate::transmit() #ifdef QSSLSOCKET_DEBUG qDebug() << "QSslSocketBackendPrivate::transmit: decrypted" << readBytes << "bytes"; #endif - char *ptr = readBuffer.reserve(readBytes); + char *ptr = buffer.reserve(readBytes); ::memcpy(ptr, data.data(), readBytes); if (readyReadEmittedPointer) -- cgit v1.2.3