Hold QRingBuffer and QNonContiguousByteDevice in shared_ptr

... instead of QSharedPointer.

QSharedPointer performs twice as many atomic operations per pointer
copy as std::shared_ptr, and this is private API, we're not bound by
BC constraints, so we can port to the more efficient version.

Change-Id: I9572a8321aae381e5dbe4a51119f2c9494a8fbc7
Reviewed-by: Qt CI Bot <qt_ci_bot@qt-project.org>
Reviewed-by: Mårten Nordheim <marten.nordheim@qt.io>
This commit is contained in:
Marc Mutz 2021-07-10 08:17:06 +02:00
parent 6c1bc7798b
commit 41a5480cc7
9 changed files with 45 additions and 39 deletions

View File

@ -227,10 +227,9 @@ qint64 QNonContiguousByteDeviceByteArrayImpl::pos() const
return currentPosition; return currentPosition;
} }
QNonContiguousByteDeviceRingBufferImpl::QNonContiguousByteDeviceRingBufferImpl(QSharedPointer<QRingBuffer> rb) QNonContiguousByteDeviceRingBufferImpl::QNonContiguousByteDeviceRingBufferImpl(std::shared_ptr<QRingBuffer> rb)
: QNonContiguousByteDevice(), currentPosition(0) : QNonContiguousByteDevice(), ringBuffer(std::move(rb))
{ {
ringBuffer = rb;
} }
QNonContiguousByteDeviceRingBufferImpl::~QNonContiguousByteDeviceRingBufferImpl() QNonContiguousByteDeviceRingBufferImpl::~QNonContiguousByteDeviceRingBufferImpl()
@ -495,44 +494,44 @@ QNonContiguousByteDevice *QNonContiguousByteDeviceFactory::create(QIODevice *dev
} }
/*! /*!
Create a QNonContiguousByteDevice out of a QIODevice, return it in a QSharedPointer. Create a QNonContiguousByteDevice out of a QIODevice, return it in a std::shared_ptr.
For QFile, QBuffer and all other QIODevice, sequential or not. For QFile, QBuffer and all other QIODevice, sequential or not.
\internal \internal
*/ */
QSharedPointer<QNonContiguousByteDevice> QNonContiguousByteDeviceFactory::createShared(QIODevice *device) std::shared_ptr<QNonContiguousByteDevice> QNonContiguousByteDeviceFactory::createShared(QIODevice *device)
{ {
// shortcut if it is a QBuffer // shortcut if it is a QBuffer
if (QBuffer *buffer = qobject_cast<QBuffer*>(device)) if (QBuffer *buffer = qobject_cast<QBuffer*>(device))
return QSharedPointer<QNonContiguousByteDeviceBufferImpl>::create(buffer); return std::make_shared<QNonContiguousByteDeviceBufferImpl>(buffer);
// ### FIXME special case if device is a QFile that supports map() // ### FIXME special case if device is a QFile that supports map()
// then we can actually deal with the file without using read/peek // then we can actually deal with the file without using read/peek
// generic QIODevice // generic QIODevice
return QSharedPointer<QNonContiguousByteDeviceIoDeviceImpl>::create(device); // FIXME return std::make_shared<QNonContiguousByteDeviceIoDeviceImpl>(device); // FIXME
} }
/*! /*!
\fn static QNonContiguousByteDevice* QNonContiguousByteDeviceFactory::create(QSharedPointer<QRingBuffer> ringBuffer) \fn static QNonContiguousByteDevice* QNonContiguousByteDeviceFactory::create(std::shared_ptr<QRingBuffer> ringBuffer)
Create a QNonContiguousByteDevice out of a QRingBuffer. Create a QNonContiguousByteDevice out of a QRingBuffer.
\internal \internal
*/ */
QNonContiguousByteDevice* QNonContiguousByteDeviceFactory::create(QSharedPointer<QRingBuffer> ringBuffer) QNonContiguousByteDevice* QNonContiguousByteDeviceFactory::create(std::shared_ptr<QRingBuffer> ringBuffer)
{ {
return new QNonContiguousByteDeviceRingBufferImpl(ringBuffer); return new QNonContiguousByteDeviceRingBufferImpl(ringBuffer);
} }
/*! /*!
Create a QNonContiguousByteDevice out of a QRingBuffer, return it in a QSharedPointer. Create a QNonContiguousByteDevice out of a QRingBuffer, return it in a std::shared_ptr.
\internal \internal
*/ */
QSharedPointer<QNonContiguousByteDevice> QNonContiguousByteDeviceFactory::createShared(QSharedPointer<QRingBuffer> ringBuffer) std::shared_ptr<QNonContiguousByteDevice> QNonContiguousByteDeviceFactory::createShared(std::shared_ptr<QRingBuffer> ringBuffer)
{ {
return QSharedPointer<QNonContiguousByteDeviceRingBufferImpl>::create(std::move(ringBuffer)); return std::make_shared<QNonContiguousByteDeviceRingBufferImpl>(std::move(ringBuffer));
} }
/*! /*!
@ -552,9 +551,9 @@ QNonContiguousByteDevice* QNonContiguousByteDeviceFactory::create(QByteArray *by
\internal \internal
*/ */
QSharedPointer<QNonContiguousByteDevice> QNonContiguousByteDeviceFactory::createShared(QByteArray *byteArray) std::shared_ptr<QNonContiguousByteDevice> QNonContiguousByteDeviceFactory::createShared(QByteArray *byteArray)
{ {
return QSharedPointer<QNonContiguousByteDeviceByteArrayImpl>::create(byteArray); return std::make_shared<QNonContiguousByteDeviceByteArrayImpl>(byteArray);
} }
/*! /*!

View File

@ -55,9 +55,10 @@
#include <QtCore/qbytearray.h> #include <QtCore/qbytearray.h>
#include <QtCore/qbuffer.h> #include <QtCore/qbuffer.h>
#include <QtCore/qiodevice.h> #include <QtCore/qiodevice.h>
#include <QtCore/QSharedPointer>
#include "private/qringbuffer_p.h" #include "private/qringbuffer_p.h"
#include <memory>
QT_BEGIN_NAMESPACE QT_BEGIN_NAMESPACE
class Q_CORE_EXPORT QNonContiguousByteDevice : public QObject class Q_CORE_EXPORT QNonContiguousByteDevice : public QObject
@ -85,13 +86,13 @@ class Q_CORE_EXPORT QNonContiguousByteDeviceFactory
{ {
public: public:
static QNonContiguousByteDevice *create(QIODevice *device); static QNonContiguousByteDevice *create(QIODevice *device);
static QSharedPointer<QNonContiguousByteDevice> createShared(QIODevice *device); static std::shared_ptr<QNonContiguousByteDevice> createShared(QIODevice *device);
static QNonContiguousByteDevice *create(QByteArray *byteArray); static QNonContiguousByteDevice *create(QByteArray *byteArray);
static QSharedPointer<QNonContiguousByteDevice> createShared(QByteArray *byteArray); static std::shared_ptr<QNonContiguousByteDevice> createShared(QByteArray *byteArray);
static QNonContiguousByteDevice *create(QSharedPointer<QRingBuffer> ringBuffer); static QNonContiguousByteDevice *create(std::shared_ptr<QRingBuffer> ringBuffer);
static QSharedPointer<QNonContiguousByteDevice> createShared(QSharedPointer<QRingBuffer> ringBuffer); static std::shared_ptr<QNonContiguousByteDevice> createShared(std::shared_ptr<QRingBuffer> ringBuffer);
static QIODevice *wrap(QNonContiguousByteDevice *byteDevice); static QIODevice *wrap(QNonContiguousByteDevice *byteDevice);
}; };
@ -119,7 +120,7 @@ protected:
class QNonContiguousByteDeviceRingBufferImpl : public QNonContiguousByteDevice class QNonContiguousByteDeviceRingBufferImpl : public QNonContiguousByteDevice
{ {
public: public:
explicit QNonContiguousByteDeviceRingBufferImpl(QSharedPointer<QRingBuffer> rb); explicit QNonContiguousByteDeviceRingBufferImpl(std::shared_ptr<QRingBuffer> rb);
~QNonContiguousByteDeviceRingBufferImpl(); ~QNonContiguousByteDeviceRingBufferImpl();
const char *readPointer(qint64 maximumLength, qint64 &len) override; const char *readPointer(qint64 maximumLength, qint64 &len) override;
bool advanceReadPointer(qint64 amount) override; bool advanceReadPointer(qint64 amount) override;
@ -129,8 +130,8 @@ public:
qint64 pos() const override; qint64 pos() const override;
protected: protected:
QSharedPointer<QRingBuffer> ringBuffer; std::shared_ptr<QRingBuffer> ringBuffer;
qint64 currentPosition; qint64 currentPosition = 0;
}; };
class QNonContiguousByteDeviceIoDeviceImpl : public QNonContiguousByteDevice class QNonContiguousByteDeviceIoDeviceImpl : public QNonContiguousByteDevice

View File

@ -80,7 +80,7 @@ public:
QNetworkAccessBackend::TargetTypes m_targetTypes; QNetworkAccessBackend::TargetTypes m_targetTypes;
QNetworkAccessBackend::SecurityFeatures m_securityFeatures; QNetworkAccessBackend::SecurityFeatures m_securityFeatures;
QNetworkAccessBackend::IOFeatures m_ioFeatures; QNetworkAccessBackend::IOFeatures m_ioFeatures;
QSharedPointer<QNonContiguousByteDevice> uploadByteDevice; std::shared_ptr<QNonContiguousByteDevice> uploadByteDevice;
QIODevice *wrappedUploadByteDevice; QIODevice *wrappedUploadByteDevice;
QNetworkReplyImplPrivate *m_reply = nullptr; QNetworkReplyImplPrivate *m_reply = nullptr;
QNetworkAccessManagerPrivate *m_manager = nullptr; QNetworkAccessManagerPrivate *m_manager = nullptr;
@ -566,7 +566,7 @@ QIODevice *QNetworkAccessBackend::createUploadByteDevice()
// We want signal emissions only for normal asynchronous uploads // We want signal emissions only for normal asynchronous uploads
if (!isSynchronous()) { if (!isSynchronous()) {
connect(d->uploadByteDevice.data(), &QNonContiguousByteDevice::readProgress, this, connect(d->uploadByteDevice.get(), &QNonContiguousByteDevice::readProgress, this,
[this](qint64 a, qint64 b) { [this](qint64 a, qint64 b) {
Q_D(QNetworkAccessBackend); Q_D(QNetworkAccessBackend);
if (!d->m_reply->isFinished) if (!d->m_reply->isFinished)
@ -574,7 +574,7 @@ QIODevice *QNetworkAccessBackend::createUploadByteDevice()
}); });
} }
d->wrappedUploadByteDevice = QNonContiguousByteDeviceFactory::wrap(d->uploadByteDevice.data()); d->wrappedUploadByteDevice = QNonContiguousByteDeviceFactory::wrap(d->uploadByteDevice.get());
return d->wrappedUploadByteDevice; return d->wrappedUploadByteDevice;
} }

View File

@ -199,7 +199,7 @@ QNetworkReplyHttpImpl::QNetworkReplyHttpImpl(QNetworkAccessManager* const manage
if (d->synchronous && outgoingData) { if (d->synchronous && outgoingData) {
// The synchronous HTTP is a corner case, we will put all upload data in one big QByteArray in the outgoingDataBuffer. // The synchronous HTTP is a corner case, we will put all upload data in one big QByteArray in the outgoingDataBuffer.
// Yes, this is not the most efficient thing to do, but on the other hand synchronous XHR needs to die anyway. // Yes, this is not the most efficient thing to do, but on the other hand synchronous XHR needs to die anyway.
d->outgoingDataBuffer = QSharedPointer<QRingBuffer>::create(); d->outgoingDataBuffer = std::make_shared<QRingBuffer>();
qint64 previousDataSize = 0; qint64 previousDataSize = 0;
do { do {
previousDataSize = d->outgoingDataBuffer->size(); previousDataSize = d->outgoingDataBuffer->size();
@ -923,14 +923,14 @@ void QNetworkReplyHttpImplPrivate::postRequest(const QNetworkRequest &newHttpReq
delegate->httpRequest.setUploadByteDevice(forwardUploadDevice); delegate->httpRequest.setUploadByteDevice(forwardUploadDevice);
// If the device in the user thread claims it has more data, keep the flow to HTTP thread going // 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()), QObject::connect(uploadByteDevice.get(), SIGNAL(readyRead()),
q, SLOT(uploadByteDeviceReadyReadSlot()), q, SLOT(uploadByteDeviceReadyReadSlot()),
Qt::QueuedConnection); Qt::QueuedConnection);
// From user thread to http thread: // From user thread to http thread:
QObject::connect(q, SIGNAL(haveUploadData(qint64,QByteArray,bool,qint64)), QObject::connect(q, SIGNAL(haveUploadData(qint64,QByteArray,bool,qint64)),
forwardUploadDevice, SLOT(haveDataSlot(qint64,QByteArray,bool,qint64)), Qt::QueuedConnection); forwardUploadDevice, SLOT(haveDataSlot(qint64,QByteArray,bool,qint64)), Qt::QueuedConnection);
QObject::connect(uploadByteDevice.data(), SIGNAL(readyRead()), QObject::connect(uploadByteDevice.get(), SIGNAL(readyRead()),
forwardUploadDevice, SIGNAL(readyRead()), forwardUploadDevice, SIGNAL(readyRead()),
Qt::QueuedConnection); Qt::QueuedConnection);
@ -953,7 +953,7 @@ void QNetworkReplyHttpImplPrivate::postRequest(const QNetworkRequest &newHttpReq
// use the uploadByteDevice provided to us by the QNetworkReplyImpl. // use the uploadByteDevice provided to us by the QNetworkReplyImpl.
// The code that is in start() makes sure it is safe to use from a thread // The code that is in start() makes sure it is safe to use from a thread
// since it only wraps a QRingBuffer // since it only wraps a QRingBuffer
delegate->httpRequest.setUploadByteDevice(uploadByteDevice.data()); delegate->httpRequest.setUploadByteDevice(uploadByteDevice.get());
} }
} }
@ -1966,7 +1966,7 @@ void QNetworkReplyHttpImplPrivate::_q_bufferOutgoingData()
if (!outgoingDataBuffer) { if (!outgoingDataBuffer) {
// first call, create our buffer // first call, create our buffer
outgoingDataBuffer = QSharedPointer<QRingBuffer>::create(); outgoingDataBuffer = std::make_shared<QRingBuffer>();
QObject::connect(outgoingData, SIGNAL(readyRead()), q, SLOT(_q_bufferOutgoingData())); QObject::connect(outgoingData, SIGNAL(readyRead()), q, SLOT(_q_bufferOutgoingData()));
QObject::connect(outgoingData, SIGNAL(readChannelFinished()), q, SLOT(_q_bufferOutgoingDataFinished())); QObject::connect(outgoingData, SIGNAL(readChannelFinished()), q, SLOT(_q_bufferOutgoingDataFinished()));
@ -2066,10 +2066,10 @@ QNonContiguousByteDevice* QNetworkReplyHttpImplPrivate::createUploadByteDevice()
// We want signal emissions only for normal asynchronous uploads // We want signal emissions only for normal asynchronous uploads
if (!synchronous) if (!synchronous)
QObject::connect(uploadByteDevice.data(), SIGNAL(readProgress(qint64,qint64)), QObject::connect(uploadByteDevice.get(), SIGNAL(readProgress(qint64,qint64)),
q, SLOT(emitReplyUploadProgress(qint64,qint64))); q, SLOT(emitReplyUploadProgress(qint64,qint64)));
return uploadByteDevice.data(); return uploadByteDevice.get();
} }
void QNetworkReplyHttpImplPrivate::_q_finished() void QNetworkReplyHttpImplPrivate::_q_finished()

View File

@ -75,6 +75,8 @@ Q_MOC_INCLUDE(<QtNetwork/QAuthenticator>)
#include <private/qdecompresshelper_p.h> #include <private/qdecompresshelper_p.h>
#include <memory>
QT_REQUIRE_CONFIG(http); QT_REQUIRE_CONFIG(http);
QT_BEGIN_NAMESPACE QT_BEGIN_NAMESPACE
@ -197,11 +199,11 @@ public:
// upload // upload
QNonContiguousByteDevice* createUploadByteDevice(); QNonContiguousByteDevice* createUploadByteDevice();
QSharedPointer<QNonContiguousByteDevice> uploadByteDevice; std::shared_ptr<QNonContiguousByteDevice> uploadByteDevice;
qint64 uploadByteDevicePosition; qint64 uploadByteDevicePosition;
bool uploadDeviceChoking; // if we couldn't readPointer() any data at the moment bool uploadDeviceChoking; // if we couldn't readPointer() any data at the moment
QIODevice *outgoingData; QIODevice *outgoingData;
QSharedPointer<QRingBuffer> outgoingDataBuffer; std::shared_ptr<QRingBuffer> outgoingDataBuffer;
void emitReplyUploadProgress(qint64 bytesSent, qint64 bytesTotal); // dup? void emitReplyUploadProgress(qint64 bytesSent, qint64 bytesTotal); // dup?
void onRedirected(const QUrl &redirectUrl, int httpStatus, int maxRedirectsRemainig); void onRedirected(const QUrl &redirectUrl, int httpStatus, int maxRedirectsRemainig);
void followRedirect(); void followRedirect();

View File

@ -193,7 +193,7 @@ void QNetworkReplyImplPrivate::_q_bufferOutgoingData()
if (!outgoingDataBuffer) { if (!outgoingDataBuffer) {
// first call, create our buffer // first call, create our buffer
outgoingDataBuffer = QSharedPointer<QRingBuffer>::create(); outgoingDataBuffer = std::make_shared<QRingBuffer>();
QObject::connect(outgoingData, SIGNAL(readyRead()), q, SLOT(_q_bufferOutgoingData())); QObject::connect(outgoingData, SIGNAL(readyRead()), q, SLOT(_q_bufferOutgoingData()));
QObject::connect(outgoingData, SIGNAL(readChannelFinished()), q, SLOT(_q_bufferOutgoingDataFinished())); QObject::connect(outgoingData, SIGNAL(readChannelFinished()), q, SLOT(_q_bufferOutgoingDataFinished()));
@ -249,7 +249,7 @@ void QNetworkReplyImplPrivate::setup(QNetworkAccessManager::Operation op, const
// The synchronous HTTP is a corner case, we will put all upload data in one big QByteArray in the outgoingDataBuffer. // The synchronous HTTP is a corner case, we will put all upload data in one big QByteArray in the outgoingDataBuffer.
// Yes, this is not the most efficient thing to do, but on the other hand synchronous XHR needs to die anyway. // Yes, this is not the most efficient thing to do, but on the other hand synchronous XHR needs to die anyway.
if (synchronousHttpAttribute.toBool() && outgoingData) { if (synchronousHttpAttribute.toBool() && outgoingData) {
outgoingDataBuffer = QSharedPointer<QRingBuffer>::create(); outgoingDataBuffer = std::make_shared<QRingBuffer>();
qint64 previousDataSize = 0; qint64 previousDataSize = 0;
do { do {
previousDataSize = outgoingDataBuffer->size(); previousDataSize = outgoingDataBuffer->size();

View File

@ -63,6 +63,8 @@
#include "private/qbytedata_p.h" #include "private/qbytedata_p.h"
#include <QSharedPointer> #include <QSharedPointer>
#include <memory>
QT_BEGIN_NAMESPACE QT_BEGIN_NAMESPACE
class QAbstractNetworkCache; class QAbstractNetworkCache;
@ -153,7 +155,7 @@ public:
QNetworkAccessBackend *backend; QNetworkAccessBackend *backend;
QIODevice *outgoingData; QIODevice *outgoingData;
QSharedPointer<QRingBuffer> outgoingDataBuffer; std::shared_ptr<QRingBuffer> outgoingDataBuffer;
QIODevice *copyDevice; QIODevice *copyDevice;
QAbstractNetworkCache *networkCache() const; QAbstractNetworkCache *networkCache() const;

View File

@ -406,7 +406,7 @@ void QNetworkReplyWasmImplPrivate::_q_bufferOutgoingData()
if (!outgoingDataBuffer) { if (!outgoingDataBuffer) {
// first call, create our buffer // first call, create our buffer
outgoingDataBuffer = QSharedPointer<QRingBuffer>::create(); outgoingDataBuffer = std::make_shared<QRingBuffer>();
QObject::connect(outgoingData, SIGNAL(readyRead()), q, SLOT(_q_bufferOutgoingData())); QObject::connect(outgoingData, SIGNAL(readyRead()), q, SLOT(_q_bufferOutgoingData()));
QObject::connect(outgoingData, SIGNAL(readChannelFinished()), q, SLOT(_q_bufferOutgoingDataFinished())); QObject::connect(outgoingData, SIGNAL(readChannelFinished()), q, SLOT(_q_bufferOutgoingDataFinished()));

View File

@ -63,6 +63,8 @@
#include <emscripten.h> #include <emscripten.h>
#include <emscripten/fetch.h> #include <emscripten/fetch.h>
#include <memory>
QT_BEGIN_NAMESPACE QT_BEGIN_NAMESPACE
class QIODevice; class QIODevice;
@ -134,7 +136,7 @@ public:
QByteArray downloadBuffer; QByteArray downloadBuffer;
QIODevice *outgoingData; QIODevice *outgoingData;
QSharedPointer<QRingBuffer> outgoingDataBuffer; std::shared_ptr<QRingBuffer> outgoingDataBuffer;
QByteArray requestData; QByteArray requestData;
static void downloadProgress(emscripten_fetch_t *fetch); static void downloadProgress(emscripten_fetch_t *fetch);