From a41dc00160f1a2510ab71cfcff9ef0c61ff3f6e8 Mon Sep 17 00:00:00 2001
From: Jocelyn Turcotte <jturcotte@woboq.com>
Date: Mon, 24 Jul 2017 08:03:05 +0200
Subject: [PATCH] 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.
---
 src/gui/folder.cpp         |  5 ++---
 src/libsync/syncengine.cpp | 34 ++++++++++++++++++++++++++--------
 src/libsync/syncengine.h   |  8 ++------
 3 files changed, 30 insertions(+), 17 deletions(-)

diff --git a/src/gui/folder.cpp b/src/gui/folder.cpp
index f5e23c88b..4ca5de844 100644
--- 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 967f5ca6a..d4afcc57c 100644
--- 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 4f96e404f..a275618a4 100644
--- 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;
-- 
2.30.2