Don't keep the list of touched files for the whole sync

We only want to know if they were touched within the last 15 seconds,
so change the data structure to use a QMultiMap, and sort them by
QElapsedTimer. This allows us to iterate over old entries ordered by
time and to stop once we find a recent entry.

This makes the look-up slower but in most cases the folder watcher
will report any change within milliseconds, and we start from the
most recent. What this really makes slower are actual user file
changes while a fast sync is underways which will need to iterate
over the whole map to find out the file isn't there.

This reduces the growth of the memory usage when downloading a large
amount of files.

diff --git a/src/gui/folder.cpp b/src/gui/folder.cpp
index f5e23c88be87987d38459ac04b63b541fc453778..4ca5de844fb4b63c0d95f8a5b5d693668007c5f5 100644 (file)
--- a/src/gui/folder.cpp
+++ b/src/gui/folder.cpp
@@ -459,9 +459,8 @@ void Folder::slotWatchedPathChanged(const QString &path)
 // own process. Therefore nothing needs to be done here!
 #else
     // Use the path to figure out whether it was our own change
-    const auto maxNotificationDelay = 15 * 1000;
-    qint64 time = _engine->timeSinceFileTouched(path);
-    if (time != -1 && time < maxNotificationDelay) {
+    if (_engine->wasFileTouched(path)) {
+        qCDebug(lcFolder) << "Changed path was touched by SyncEngine, ignoring:" << path;
         return;
     }
 #endif

diff --git a/src/libsync/syncengine.cpp b/src/libsync/syncengine.cpp
index 967f5ca6a002826c727d734ac4fa07ccc69f4f8f..d4afcc57c6880ff6b91f0d087bfc484f79359c0b 100644 (file)
--- a/src/libsync/syncengine.cpp
+++ b/src/libsync/syncengine.cpp
@@ -55,6 +55,7 @@ namespace OCC {
 
 Q_LOGGING_CATEGORY(lcEngine, "sync.engine", QtInfoMsg)
 
+static const int s_touchedFilesMaxAgeMs = 15 * 1000;
 bool SyncEngine::s_anySyncRunning = false;
 
 qint64 SyncEngine::minimumFileAgeForUpload = 2000;
@@ -1512,12 +1513,27 @@ void SyncEngine::restoreOldFiles(SyncFileItemVector &syncItems)
 
 void SyncEngine::slotAddTouchedFile(const QString &fn)
 {
+    QElapsedTimer now;
+    now.start();
     QString file = QDir::cleanPath(fn);
 
-    QElapsedTimer timer;
-    timer.start();
+    // Iterate from the oldest and remove anything older than 15 seconds.
+    while (true) {
+        auto first = _touchedFiles.begin();
+        if (first == _touchedFiles.end())
+            break;
+        // Compare to our new QElapsedTimer instead of using elapsed().
+        // This avoids querying the current time from the OS for every loop.
+        if (now.msecsSinceReference() - first.key().msecsSinceReference() <= s_touchedFilesMaxAgeMs) {
+            // We found the first path younger than 15 second, keep the rest.
+            break;
+        }
 
-    _touchedFiles.insert(file, timer);
+        _touchedFiles.erase(first);
+    }
+
+    // This should be the largest QElapsedTimer yet, use constEnd() as hint.
+    _touchedFiles.insert(_touchedFiles.constEnd(), now, file);
 }
 
 void SyncEngine::slotClearTouchedFiles()
@@ -1525,13 +1541,15 @@ void SyncEngine::slotClearTouchedFiles()
     _touchedFiles.clear();
 }
 
-qint64 SyncEngine::timeSinceFileTouched(const QString &fn) const
+bool SyncEngine::wasFileTouched(const QString &fn) const
 {
-    if (!_touchedFiles.contains(fn)) {
-        return -1;
+    // Start from the end (most recent) and look for our path. Check the time just in case.
+    auto begin = _touchedFiles.constBegin();
+    for (auto it = _touchedFiles.constEnd(); it != begin; --it) {
+        if ((it-1).value() == fn)
+            return (it-1).key().elapsed() <= s_touchedFilesMaxAgeMs;
     }
-
-    return _touchedFiles[fn].elapsed();
+    return false;
 }
 
 AccountPtr SyncEngine::account() const

diff --git a/src/libsync/syncengine.h b/src/libsync/syncengine.h
index 4f96e404f00a724a65e8945bc261adf5e0638131..a275618a43e6d352106367f869384def89c858bb 100644 (file)
--- a/src/libsync/syncengine.h
+++ b/src/libsync/syncengine.h
@@ -87,11 +87,7 @@ public:
     /* Returns whether another sync is needed to complete the sync */
     AnotherSyncNeeded isAnotherSyncNeeded() { return _anotherSyncNeeded; }
 
-    /** Get the ms since a file was touched, or -1 if it wasn't.
-     *
-     * Thread-safe.
-     */
-    qint64 timeSinceFileTouched(const QString &fn) const;
+    bool wasFileTouched(const QString &fn) const;
 
     AccountPtr account() const;
     SyncJournalDb *journal() const { return _journal; }
@@ -272,7 +268,7 @@ private:
     AnotherSyncNeeded _anotherSyncNeeded;
 
     /** Stores the time since a job touched a file. */
-    QHash<QString, QElapsedTimer> _touchedFiles;
+    QMultiMap<QElapsedTimer, QString> _touchedFiles;
 
     /** For clearing the _touchedFiles variable after sync finished */
     QTimer _clearTouchedFilesTimer;