From 19bf5e2ff13d5bf94eec4aa972272fa5b5ab62fc Mon Sep 17 00:00:00 2001 From: Jocelyn Turcotte Date: Tue, 14 Feb 2017 12:46:44 +0100 Subject: [PATCH] Reduce the connection data used by PropagateDirectory objects Avoid using connections to report up the job tree for signals that we can directly communicate to the OwncloudPropagator. This slightly reduces the memory usage and avoid passing those calls through the whole parent chain. --- src/libsync/owncloudpropagator.cpp | 58 +++++++++++++++--------------- src/libsync/owncloudpropagator.h | 34 ++++++------------ src/libsync/propagatedownload.cpp | 4 +-- src/libsync/propagateuploadng.cpp | 4 +-- src/libsync/propagateuploadv1.cpp | 6 ++-- src/libsync/propagatorjobs.cpp | 4 +-- 6 files changed, 49 insertions(+), 61 deletions(-) diff --git a/src/libsync/owncloudpropagator.cpp b/src/libsync/owncloudpropagator.cpp index 22e8407511..40e3c6786d 100644 --- a/src/libsync/owncloudpropagator.cpp +++ b/src/libsync/owncloudpropagator.cpp @@ -169,7 +169,7 @@ void PropagateItemJob::done(SyncFileItem::Status status, const QString &errorStr _item->_status = status; - emit itemCompleted(_item); + emit propagator()->itemCompleted(_item); emit finished(status); } @@ -218,8 +218,8 @@ bool PropagateItemJob::checkForProblemsWithShared(int httpStatusCode, const QStr if( newJob ) { newJob->setRestoreJobMsg(msg); _restoreJob.reset(newJob); - connect(_restoreJob.data(), SIGNAL(itemCompleted(const SyncFileItemPtr &)), - this, SLOT(slotRestoreJobCompleted(const SyncFileItemPtr &))); + connect(_restoreJob.data(), SIGNAL(finished(SyncFileItem::Status)), + this, SLOT(slotRestoreJobFinished(SyncFileItem::Status))); QMetaObject::invokeMethod(newJob, "start"); } return true; @@ -227,7 +227,7 @@ bool PropagateItemJob::checkForProblemsWithShared(int httpStatusCode, const QStr return false; } -void PropagateItemJob::slotRestoreJobCompleted(const SyncFileItem& item ) +void PropagateItemJob::slotRestoreJobFinished(SyncFileItem::Status status) { QString msg; if(_restoreJob) { @@ -235,11 +235,11 @@ void PropagateItemJob::slotRestoreJobCompleted(const SyncFileItem& item ) _restoreJob->setRestoreJobMsg(); } - if( item._status == SyncFileItem::Success || item._status == SyncFileItem::Conflict - || item._status == SyncFileItem::Restoration) { + if( status == SyncFileItem::Success || status == SyncFileItem::Conflict + || status == SyncFileItem::Restoration) { done( SyncFileItem::SoftError, msg); } else { - done( item._status, tr("A file or folder was removed from a read only share, but restoring failed: %1").arg(item._errorString) ); + done( status, tr("A file or folder was removed from a read only share, but restoring failed: %1").arg(msg) ); } } @@ -399,15 +399,11 @@ void OwncloudPropagator::start(const SyncFileItemVector& items) _rootJob->appendJob(it); } - connect(_rootJob.data(), SIGNAL(itemCompleted(const SyncFileItemPtr &)), - this, SIGNAL(itemCompleted(const SyncFileItemPtr &))); - connect(_rootJob.data(), SIGNAL(progress(const SyncFileItem &,quint64)), this, SIGNAL(progress(const SyncFileItem &,quint64))); connect(_rootJob.data(), SIGNAL(finished(SyncFileItem::Status)), this, SLOT(emitFinished(SyncFileItem::Status))); - connect(_rootJob.data(), SIGNAL(ready()), this, SLOT(scheduleNextJob()), Qt::QueuedConnection); qDebug() << "Using QNAM/HTTP parallel code path"; - QTimer::singleShot(0, this, SLOT(scheduleNextJob())); + scheduleNextJob(); } // ownCloud server < 7.0 did not had permissions so we need some other euristics @@ -514,6 +510,11 @@ QString OwncloudPropagator::getFilePath(const QString& tmp_file_name) const } void OwncloudPropagator::scheduleNextJob() +{ + QTimer::singleShot(0, this, SLOT(scheduleNextJobImpl())); +} + +void OwncloudPropagator::scheduleNextJobImpl() { // TODO: If we see that the automatic up-scaling has a bad impact we // need to check how to avoid this. @@ -521,8 +522,8 @@ void OwncloudPropagator::scheduleNextJob() // Making sure we do up/down at same time? https://github.com/owncloud/client/issues/1633 if (_activeJobList.count() < maximumActiveTransferJob()) { - if (_rootJob->scheduleNextJob()) { - QTimer::singleShot(0, this, SLOT(scheduleNextJob())); + if (_rootJob->scheduleSelfOrChild()) { + scheduleNextJob(); } } else if (_activeJobList.count() < hardMaximumActiveJob()) { int likelyFinishedQuicklyCount = 0; @@ -537,13 +538,18 @@ void OwncloudPropagator::scheduleNextJob() } if (_activeJobList.count() < maximumActiveTransferJob() + likelyFinishedQuicklyCount) { qDebug() << "Can pump in another request! activeJobs =" << _activeJobList.count(); - if (_rootJob->scheduleNextJob()) { - QTimer::singleShot(0, this, SLOT(scheduleNextJob())); + if (_rootJob->scheduleSelfOrChild()) { + scheduleNextJob(); } } } } +void OwncloudPropagator::reportProgress(const SyncFileItem &item, quint64 bytes) +{ + emit progress(item, bytes); +} + AccountPtr OwncloudPropagator::account() const { return _account; @@ -595,7 +601,7 @@ PropagatorJob::JobParallelism PropagatorCompositeJob::parallelism() } -bool PropagatorCompositeJob::scheduleNextJob() +bool PropagatorCompositeJob::scheduleSelfOrChild() { if (_state == Finished) { return false; @@ -615,7 +621,7 @@ bool PropagatorCompositeJob::scheduleNextJob() } // If any of the running sub jobs is not parallel, we have to cancel the scheduling - // of the rest of the list and wait for the blocking job to finish and emit ready(). + // of the rest of the list and wait for the blocking job to finish and schedule the next one. auto paral = _runningJobs.at(i)->parallelism(); if (paral == WaitForFinished) { return false; @@ -675,7 +681,7 @@ void PropagatorCompositeJob::slotSubJobFinished(SyncFileItem::Status status) if (_jobsToDo.isEmpty() && _tasksToDo.isEmpty() && _runningJobs.isEmpty()) { finalize(); } else { - emit ready(); + propagator()->scheduleNextJob(); } } @@ -709,14 +715,8 @@ PropagateDirectory::PropagateDirectory(OwncloudPropagator *propagator, const Syn { if (_firstJob) { connect(_firstJob.data(), SIGNAL(finished(SyncFileItem::Status)), this, SLOT(slotFirstJobFinished(SyncFileItem::Status))); - connect(_firstJob.data(), SIGNAL(itemCompleted(const SyncFileItemPtr &)), this, SIGNAL(itemCompleted(const SyncFileItemPtr &))); - connect(_firstJob.data(), SIGNAL(progress(const SyncFileItem &,quint64)), this, SIGNAL(progress(const SyncFileItem &,quint64))); - connect(_firstJob.data(), SIGNAL(ready()), this, SIGNAL(ready())); } connect(&_subJobs, SIGNAL(finished(SyncFileItem::Status)), this, SLOT(slotSubJobsFinished(SyncFileItem::Status))); - connect(&_subJobs, SIGNAL(itemCompleted(const SyncFileItemPtr &)), this, SIGNAL(itemCompleted(const SyncFileItemPtr &))); - connect(&_subJobs, SIGNAL(progress(const SyncFileItem &,quint64)), this, SIGNAL(progress(const SyncFileItem &,quint64))); - connect(&_subJobs, SIGNAL(ready()), this, SIGNAL(ready())); } PropagatorJob::JobParallelism PropagateDirectory::parallelism() @@ -732,7 +732,7 @@ PropagatorJob::JobParallelism PropagateDirectory::parallelism() } -bool PropagateDirectory::scheduleNextJob() +bool PropagateDirectory::scheduleSelfOrChild() { if (_state == Finished) { return false; @@ -743,7 +743,7 @@ bool PropagateDirectory::scheduleNextJob() } if (_firstJob && _firstJob->_state == NotYetStarted) { - return _firstJob->scheduleNextJob(); + return _firstJob->scheduleSelfOrChild(); } if (_firstJob && _firstJob->_state == Running) { @@ -751,7 +751,7 @@ bool PropagateDirectory::scheduleNextJob() return false; } - return _subJobs.scheduleNextJob(); + return _subJobs.scheduleSelfOrChild(); } @@ -766,7 +766,7 @@ void PropagateDirectory::slotFirstJobFinished(SyncFileItem::Status status) return; } - emit ready(); + propagator()->scheduleNextJob(); } void PropagateDirectory::slotSubJobsFinished(SyncFileItem::Status status) diff --git a/src/libsync/owncloudpropagator.h b/src/libsync/owncloudpropagator.h index b667edb2f5..0ef06ee70e 100644 --- a/src/libsync/owncloudpropagator.h +++ b/src/libsync/owncloudpropagator.h @@ -98,26 +98,13 @@ public slots: /** Starts this job, or a new subjob * returns true if a job was started. */ - virtual bool scheduleNextJob() = 0; + virtual bool scheduleSelfOrChild() = 0; signals: /** * Emitted when the job is fully finished */ void finished(SyncFileItem::Status); - /** - * Emitted when one item has been completed within a job. - */ - void itemCompleted(const SyncFileItemPtr &); - - /** - * Emitted when all the sub-jobs have been finished and - * more jobs might be started (so scheduleNextJob can/must be called again) - */ - void ready(); - - void progress(const SyncFileItem& item, quint64 bytes); - protected: OwncloudPropagator *propagator() const; }; @@ -146,7 +133,7 @@ protected: } protected slots: - void slotRestoreJobCompleted(const SyncFileItem& ); + void slotRestoreJobFinished(SyncFileItem::Status status); private: QScopedPointer _restoreJob; @@ -155,7 +142,7 @@ public: PropagateItemJob(OwncloudPropagator* propagator, const SyncFileItemPtr &item) : PropagatorJob(propagator), _item(item) {} - bool scheduleNextJob() Q_DECL_OVERRIDE { + bool scheduleSelfOrChild() Q_DECL_OVERRIDE { if (_state != NotYetStarted) { return false; } @@ -199,7 +186,7 @@ public: _tasksToDo.append(item); } - virtual bool scheduleNextJob() Q_DECL_OVERRIDE; + virtual bool scheduleSelfOrChild() Q_DECL_OVERRIDE; virtual JobParallelism parallelism() Q_DECL_OVERRIDE; virtual void abort() Q_DECL_OVERRIDE { foreach (PropagatorJob *j, _runningJobs) @@ -212,11 +199,8 @@ private slots: bool possiblyRunNextJob(PropagatorJob *next) { if (next->_state == NotYetStarted) { connect(next, SIGNAL(finished(SyncFileItem::Status)), this, SLOT(slotSubJobFinished(SyncFileItem::Status))); - connect(next, SIGNAL(itemCompleted(const SyncFileItemPtr &)), this, SIGNAL(itemCompleted(const SyncFileItemPtr &))); - connect(next, SIGNAL(progress(const SyncFileItem &,quint64)), this, SIGNAL(progress(const SyncFileItem &,quint64))); - connect(next, SIGNAL(ready()), this, SIGNAL(ready())); } - return next->scheduleNextJob(); + return next->scheduleSelfOrChild(); } void slotSubJobFinished(SyncFileItem::Status status); @@ -246,7 +230,7 @@ public: _subJobs.appendTask(item); } - virtual bool scheduleNextJob() Q_DECL_OVERRIDE; + virtual bool scheduleSelfOrChild() Q_DECL_OVERRIDE; virtual JobParallelism parallelism() Q_DECL_OVERRIDE; virtual void abort() Q_DECL_OVERRIDE { if (_firstJob) @@ -341,6 +325,9 @@ public: QString getFilePath(const QString& tmp_file_name) const; PropagateItemJob *createJob(const SyncFileItemPtr& item); + void scheduleNextJob(); + void reportProgress(const SyncFileItem&, quint64 bytes); + void abort() { _abortRequested.fetchAndStoreOrdered(true); if (_rootJob) { @@ -380,7 +367,7 @@ private slots: _finishedEmited = true; } - void scheduleNextJob(); + void scheduleNextJobImpl(); signals: void itemCompleted(const SyncFileItemPtr &); @@ -404,6 +391,7 @@ private: #if QT_VERSION < QT_VERSION_CHECK(5, 0, 0) // access to signals which are protected in Qt4 friend class PropagateDownloadFile; + friend class PropagateItemJob; friend class PropagateLocalMkdir; friend class PropagateLocalRename; friend class PropagateRemoteMove; diff --git a/src/libsync/propagatedownload.cpp b/src/libsync/propagatedownload.cpp index 030826c0b0..22b2223534 100644 --- a/src/libsync/propagatedownload.cpp +++ b/src/libsync/propagatedownload.cpp @@ -333,7 +333,7 @@ void PropagateDownloadFile::start() return; } - emit progress(*_item, 0); + propagator()->reportProgress(*_item, 0); QString tmpFileName; QByteArray expectedEtagForResume; @@ -834,7 +834,7 @@ void PropagateDownloadFile::slotDownloadProgress(qint64 received, qint64) { if (!_job) return; _downloadProgress = received; - emit progress(*_item, _resumeStart + received); + propagator()->reportProgress(*_item, _resumeStart + received); } diff --git a/src/libsync/propagateuploadng.cpp b/src/libsync/propagateuploadng.cpp index 7109abb94e..9803385be1 100644 --- a/src/libsync/propagateuploadng.cpp +++ b/src/libsync/propagateuploadng.cpp @@ -225,7 +225,7 @@ void PropagateUploadFileNG::startNewUpload() _sent = 0; _currentChunk = 0; - emit progress(*_item, 0); + propagator()->reportProgress(*_item, 0); SyncJournalDb::UploadInfo pi; pi._valid = true; @@ -507,7 +507,7 @@ void PropagateUploadFileNG::slotUploadProgress(qint64 sent, qint64 total) if (sent == 0 && total == 0) { return; } - emit progress(*_item, _sent + sent - total); + propagator()->reportProgress(*_item, _sent + sent - total); } } diff --git a/src/libsync/propagateuploadv1.cpp b/src/libsync/propagateuploadv1.cpp index 916dcdcf9e..813029949e 100644 --- a/src/libsync/propagateuploadv1.cpp +++ b/src/libsync/propagateuploadv1.cpp @@ -51,7 +51,7 @@ void PropagateUploadFileV1::doStartUpload() _currentChunk = 0; - emit progress(*_item, 0); + propagator()->reportProgress(*_item, 0); startNextChunk(); } @@ -165,7 +165,7 @@ void PropagateUploadFileV1::startNextChunk() startNextChunk(); } if (!parallelChunkUpload || _chunkCount - _currentChunk <= 0) { - emit ready(); + propagator()->scheduleNextJob(); } } @@ -386,7 +386,7 @@ void PropagateUploadFileV1::slotUploadProgress(qint64 sent, qint64 total) // sender() is the only current job, no need to look at the byteWritten properties amount += sent; } - emit progress(*_item, amount); + propagator()->reportProgress(*_item, amount); } } diff --git a/src/libsync/propagatorjobs.cpp b/src/libsync/propagatorjobs.cpp index 771b282ada..3827daeefa 100644 --- a/src/libsync/propagatorjobs.cpp +++ b/src/libsync/propagatorjobs.cpp @@ -131,7 +131,7 @@ void PropagateLocalRemove::start() return; } } - emit progress(*_item, 0); + propagator()->reportProgress(*_item, 0); propagator()->_journal->deleteFileRecord(_item->_originalFile, _item->_isDirectory); propagator()->_journal->commit("Local remove"); done(SyncFileItem::Success); @@ -202,7 +202,7 @@ void PropagateLocalRename::start() // if the file is a file underneath a moved dir, the _item->file is equal // to _item->renameTarget and the file is not moved as a result. if (_item->_file != _item->_renameTarget) { - emit progress(*_item, 0); + propagator()->reportProgress(*_item, 0); qDebug() << "MOVE " << existingFile << " => " << targetFile; if (QString::compare(_item->_file, _item->_renameTarget, Qt::CaseInsensitive) != 0