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.

diff --git a/src/libsync/owncloudpropagator.cpp b/src/libsync/owncloudpropagator.cpp
index 22e8407511d68ee8035c7eccc5a9f5c6fe27b918..40e3c6786d9653601f69c84d9caa52be4a3ebcb6 100644 (file)
--- 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 b667edb2f5614036ce5b511100ecc564b8facd82..0ef06ee70e15b280efe59d5ce109619bc2dfe722 100644 (file)
--- 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<PropagateItemJob> _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 030826c0b022e22c2fa904e61386117594293d45..22b22235343a205162457134cde2245d576ab751 100644 (file)
--- 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 7109abb94e4f53bd8b0db965ec4e67e2cc4e2387..9803385be1008436c02108452abbad24b37fd7a5 100644 (file)
--- 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 916dcdcf9e6cd1b648d1263bd718f9cea8364f21..813029949e255212e9f169033d1e01c9adc9b45d 100644 (file)
--- 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 771b282ada988a6ef007304893587c6ee0719fff..3827daeefa95fde8d25502d63a6a84df6da6581f 100644 (file)
--- 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