Clang: Add timer based path notification compression

QFileWatcher is only reporting one path per signal which is suboptimal if you change many files at once. This patch is introducing a timer which is waiting some time time to see if more path changes are reported and is collecting them in a vector. Change-Id: I50f7c21186353b199634e7b3cd5a41f8d581a31d Reviewed-by: Tim Jenssen <tim.jenssen@qt.io>
2017-01-31 11:14:54 +01:00
parent 9678c09355
commit 2c55a9c569
12 changed files with 426 additions and 51 deletions
--- a/src/tools/clangpchmanagerbackend/clangpchmanagerbackendmain.cpp
+++ b/src/tools/clangpchmanagerbackend/clangpchmanagerbackendmain.cpp
@@ -37,6 +37,7 @@
 #include <QFileSystemWatcher>
 #include <QLoggingCategory>
 #include <QTemporaryDir>
+#include <QTimer>

 using ClangBackEnd::ClangPathWatcher;
 using ClangBackEnd::ConnectionServer;
@@ -93,11 +94,15 @@ int main(int argc, char *argv[])
    const QString connection =  processArguments(application);

    StringCache<Utils::SmallString> filePathCache;
-    ClangPathWatcher<QFileSystemWatcher> includeWatcher(filePathCache);
+    ClangPathWatcher<QFileSystemWatcher, QTimer> includeWatcher(filePathCache);
    ApplicationEnvironment environment;
    PchCreator pchCreator(environment, filePathCache);
    ProjectParts projectParts;
-    PchManagerServer clangPchManagerServer(filePathCache, includeWatcher, pchCreator, projectParts);
+    PchManagerServer clangPchManagerServer(filePathCache,
+                                           includeWatcher,
+                                           pchCreator,
+                                           projectParts);
+    includeWatcher.setNotifier(&clangPchManagerServer);
    ConnectionServer<PchManagerServer, PchManagerClientProxy> connectionServer(connection);
    connectionServer.start();
    connectionServer.setServer(&clangPchManagerServer);
--- a/src/tools/clangpchmanagerbackend/source/changedfilepathcompressor.h
+++ b/src/tools/clangpchmanagerbackend/source/changedfilepathcompressor.h
@@ -0,0 +1,86 @@
+/****************************************************************************
+**
+** Copyright (C) 2016 The Qt Company Ltd.
+** Contact: https://www.qt.io/licensing/
+**
+** This file is part of Qt Creator.
+**
+** Commercial License Usage
+** Licensees holding valid commercial Qt licenses may use this file in
+** accordance with the commercial license agreement provided with the
+** Software or, alternatively, in accordance with the terms contained in
+** a written agreement between you and The Qt Company. For licensing terms
+** and conditions see https://www.qt.io/terms-conditions. For further
+** information use the contact form at https://www.qt.io/contact-us.
+**
+** GNU General Public License Usage
+** Alternatively, this file may be used under the terms of the GNU
+** General Public License version 3 as published by the Free Software
+** Foundation with exceptions as appearing in the file LICENSE.GPL3-EXCEPT
+** included in the packaging of this file. Please review the following
+** information to ensure the GNU General Public License requirements will
+** be met: https://www.gnu.org/licenses/gpl-3.0.html.
+**
+****************************************************************************/
+
+#pragma once
+
+#include <clangpchmanagerbackend_global.h>
+
+#include <utils/smallstringvector.h>
+
+#include <QTimer>
+
+#include <functional>
+
+namespace ClangBackEnd {
+
+template <typename Timer>
+class ChangedFilePathCompressor non_unittest_final
+{
+public:
+    ChangedFilePathCompressor()
+    {
+        m_timer.setSingleShot(true);
+    }
+
+    virtual ~ChangedFilePathCompressor()
+    {
+    }
+
+    void addFilePath(const QString &filePath)
+    {
+        m_filePaths.push_back(filePath);
+
+        restartTimer();
+    }
+
+    Utils::SmallStringVector takeFilePaths()
+    {
+        return std::move(m_filePaths);
+    }
+
+    virtual void setCallback(std::function<void(Utils::SmallStringVector &&)> &&callback)
+    {
+        QObject::connect(&m_timer,
+                         &Timer::timeout,
+                         [this, callback=std::move(callback)] { callback(takeFilePaths()); });
+    }
+
+unitttest_public:
+    virtual void restartTimer()
+    {
+        m_timer.start(20);
+    }
+
+    Timer &timer()
+    {
+        return m_timer;
+    }
+
+private:
+    Utils::SmallStringVector m_filePaths;
+    Timer m_timer;
+};
+
+} // namespace ClangBackEnd
--- a/src/tools/clangpchmanagerbackend/source/clangpathwatcher.h
+++ b/src/tools/clangpchmanagerbackend/source/clangpathwatcher.h
@@ -27,9 +27,10 @@

 #include "clangpathwatcherinterface.h"
 #include "clangpathwatchernotifier.h"
+#include "changedfilepathcompressor.h"
 #include "stringcache.h"

-#include <QObject>
+#include <QTimer>

 namespace ClangBackEnd {

@@ -60,7 +61,10 @@ public:
    }
 };

-template <typename FileSystemWatcher>
+using WatcherEntries = std::vector<WatcherEntry>;
+
+template <typename FileSystemWatcher,
+          typename Timer>
 class ClangPathWatcher : public ClangPathWatcherInterface
 {
 public:
@@ -71,7 +75,16 @@ public:
    {
        QObject::connect(&m_fileSystemWatcher,
                         &FileSystemWatcher::fileChanged,
-                         [&] (const QString &filePath) { addChangedPathForFilePath(filePath); });
+                         [&] (const QString &filePath) { compressChangedFilePath(filePath); });
+
+        m_changedFilePathCompressor.setCallback([&] (Utils::SmallStringVector &&filePaths) {
+            addChangedPathForFilePath(std::move(filePaths));
+        });
+    }
+
+    ~ClangPathWatcher()
+    {
+        m_changedFilePathCompressor.setCallback([&] (Utils::SmallStringVector &&) {});
    }

    void updateIdPaths(const std::vector<IdPaths> &idPaths) override
@@ -136,10 +149,10 @@ unitttest_public:
    }


-    std::pair<std::vector<WatcherEntry>,std::vector<uint>>
+    std::pair<WatcherEntries,std::vector<uint>>
    convertIdPathsToWatcherEntriesAndIds(const std::vector<IdPaths> &idPaths)
    {
-        std::vector<WatcherEntry> entries;
+        WatcherEntries entries;
        entries.reserve(sizeOfIdPaths(idPaths));
        std::vector<uint> ids;
        ids.reserve(ids.size());
@@ -164,7 +177,7 @@ unitttest_public:
        return {entries, ids};
    }

-    void addEntries(const std::vector<WatcherEntry> &entries)
+    void addEntries(const WatcherEntries &entries)
    {
        auto newEntries = notWatchedEntries(entries);

@@ -176,7 +189,7 @@ unitttest_public:
            m_fileSystemWatcher.addPaths(convertWatcherEntriesToQStringList(filteredPaths));
    }

-    void removeUnusedEntries(const std::vector<WatcherEntry> &entries,
+    void removeUnusedEntries(const WatcherEntries &entries,
                             const std::vector<uint> &ids)
    {
        auto oldEntries = notAnymoreWatchedEntriesWithIds(entries, ids);
@@ -195,7 +208,7 @@ unitttest_public:
    }

    QStringList convertWatcherEntriesToQStringList(
-            const std::vector<WatcherEntry> &watcherEntries)
+            const WatcherEntries &watcherEntries)
    {
        QStringList paths;
        paths.reserve(int(watcherEntries.size()));
@@ -209,10 +222,10 @@ unitttest_public:
    }

    template <typename Compare>
-    std::vector<WatcherEntry> notWatchedEntries(const std::vector<WatcherEntry> &entries,
+    WatcherEntries notWatchedEntries(const WatcherEntries &entries,
                                                Compare compare) const
    {
-        std::vector<WatcherEntry> notWatchedEntries;
+        WatcherEntries notWatchedEntries;
        notWatchedEntries.reserve(entries.size());

        std::set_difference(entries.begin(),
@@ -225,12 +238,12 @@ unitttest_public:
        return notWatchedEntries;
    }

-    std::vector<WatcherEntry> notWatchedEntries(const std::vector<WatcherEntry> &entries) const
+    WatcherEntries notWatchedEntries(const WatcherEntries &entries) const
    {
        return notWatchedEntries(entries, std::less<WatcherEntry>());
    }

-    std::vector<WatcherEntry> notWatchedPaths(const std::vector<WatcherEntry> &entries) const
+    WatcherEntries notWatchedPaths(const WatcherEntries &entries) const
    {
        auto compare = [] (const WatcherEntry &first, const WatcherEntry &second) {
            return first.path < second.path;
@@ -240,11 +253,11 @@ unitttest_public:
    }

    template <typename Compare>
-    std::vector<WatcherEntry> notAnymoreWatchedEntries(
-            const std::vector<WatcherEntry> &newEntries,
+    WatcherEntries notAnymoreWatchedEntries(
+            const WatcherEntries &newEntries,
            Compare compare) const
    {
-        std::vector<WatcherEntry> notAnymoreWatchedEntries;
+        WatcherEntries notAnymoreWatchedEntries;
        notAnymoreWatchedEntries.reserve(m_watchedEntries.size());

        std::set_difference(m_watchedEntries.cbegin(),
@@ -257,8 +270,8 @@ unitttest_public:
        return notAnymoreWatchedEntries;
    }

-    std::vector<WatcherEntry> notAnymoreWatchedEntriesWithIds(
-            const std::vector<WatcherEntry> &newEntries,
+    WatcherEntries notAnymoreWatchedEntriesWithIds(
+            const WatcherEntries &newEntries,
            const std::vector<uint> &ids) const
    {
        auto oldEntries = notAnymoreWatchedEntries(newEntries, std::less<WatcherEntry>());
@@ -274,9 +287,9 @@ unitttest_public:
        return oldEntries;
    }

-    void mergeToWatchedEntries(const std::vector<WatcherEntry> &newEntries)
+    void mergeToWatchedEntries(const WatcherEntries &newEntries)
    {
-        std::vector<WatcherEntry> newWatchedEntries;
+        WatcherEntries newWatchedEntries;
        newWatchedEntries.reserve(m_watchedEntries.size() + newEntries.size());

        std::merge(m_watchedEntries.cbegin(),
@@ -289,9 +302,9 @@ unitttest_public:
    }

    static
-    std::vector<WatcherEntry> uniquePaths(const std::vector<WatcherEntry> &pathEntries)
+    WatcherEntries uniquePaths(const WatcherEntries &pathEntries)
    {
-        std::vector<WatcherEntry> uniqueEntries;
+        WatcherEntries uniqueEntries;

        auto compare = [] (const WatcherEntry &first, const WatcherEntry &second) {
            return first.path == second.path;
@@ -305,17 +318,17 @@ unitttest_public:
        return uniqueEntries;
    }

-    std::vector<WatcherEntry> filterNotWatchedPaths(const std::vector<WatcherEntry> &entries)
+    WatcherEntries filterNotWatchedPaths(const WatcherEntries &entries)
    {
        return notWatchedPaths(uniquePaths(entries));
    }

-    const std::vector<WatcherEntry> &watchedEntries() const
+    const WatcherEntries &watchedEntries() const
    {
        return m_watchedEntries;
    }

-    std::vector<WatcherEntry> removeIdsFromWatchedEntries(const std::vector<uint> &ids)
+    WatcherEntries removeIdsFromWatchedEntries(const std::vector<uint> &ids)
    {

        auto keep = [&] (const WatcherEntry &entry) {
@@ -326,16 +339,16 @@ unitttest_public:
                                           m_watchedEntries.end(),
                                           keep);

-        std::vector<WatcherEntry> removedEntries(found, m_watchedEntries.end());
+        WatcherEntries removedEntries(found, m_watchedEntries.end());

        m_watchedEntries.erase(found, m_watchedEntries.end());

        return removedEntries;
    }

-    void removeFromWatchedEntries(const std::vector<WatcherEntry> &oldEntries)
+    void removeFromWatchedEntries(const WatcherEntries &oldEntries)
    {
-        std::vector<WatcherEntry> newWatchedEntries;
+        WatcherEntries newWatchedEntries;
        newWatchedEntries.reserve(m_watchedEntries.size() - oldEntries.size());

        std::set_difference(m_watchedEntries.cbegin(),
@@ -348,25 +361,57 @@ unitttest_public:
        m_watchedEntries = newWatchedEntries;
    }

-    void addChangedPathForFilePath(const QString &filePath)
+    void compressChangedFilePath(const QString &filePath)
    {
-        uint pathId = m_pathCache.stringId(Utils::SmallString(filePath));
+        m_changedFilePathCompressor.addFilePath(filePath);
+    }

-        auto range = std::equal_range(m_watchedEntries.begin(), m_watchedEntries.end(), pathId);
+    WatcherEntries watchedEntriesForPaths(Utils::SmallStringVector &&filePaths)
+    {
+        std::vector<uint> pathIds = m_pathCache.stringIds(filePaths);

-        Utils::SmallStringVector changedIds;
+        WatcherEntries foundEntries;

-        std::transform(range.first,
-                       range.second,
-                       std::back_inserter(changedIds),
+        for (uint pathId : pathIds) {
+            auto range = std::equal_range(m_watchedEntries.begin(), m_watchedEntries.end(), pathId);
+            foundEntries.insert(foundEntries.end(), range.first, range.second);
+        }
+
+        return foundEntries;
+    }
+
+    Utils::SmallStringVector idsForWatcherEntries(const WatcherEntries &foundEntries)
+    {
+        Utils::SmallStringVector ids;
+
+        std::transform(foundEntries.begin(),
+                       foundEntries.end(),
+                       std::back_inserter(ids),
                       [&] (const WatcherEntry &entry) {
            return m_idCache.string(entry.id);
        });

-        std::sort(changedIds.begin(), changedIds.end());
+        return ids;
+    }

-        if (m_notifier)
-            m_notifier->pathsWithIdsChanged(changedIds);
+    Utils::SmallStringVector uniqueIds(Utils::SmallStringVector &&ids)
+    {
+        std::sort(ids.begin(), ids.end());
+        auto newEnd = std::unique(ids.begin(), ids.end());
+        ids.erase(newEnd, ids.end());
+
+        return std::move(ids);
+    }
+
+    void addChangedPathForFilePath(Utils::SmallStringVector &&filePaths)
+    {
+        if (m_notifier) {
+            WatcherEntries foundEntries = watchedEntriesForPaths(std::move(filePaths));
+
+            Utils::SmallStringVector changedIds = idsForWatcherEntries(foundEntries);
+
+            m_notifier->pathsWithIdsChanged(uniqueIds(std::move(changedIds)));
+        }
    }

    StringCache<Utils::SmallString> &pathCache()
@@ -381,7 +426,8 @@ unitttest_public:

 private:
    StringCache<Utils::SmallString> m_idCache;
-    std::vector<WatcherEntry> m_watchedEntries;
+    WatcherEntries m_watchedEntries;
+    ChangedFilePathCompressor<Timer> m_changedFilePathCompressor;
    FileSystemWatcher m_fileSystemWatcher;
    StringCache<Utils::SmallString> &m_pathCache;
    ClangPathWatcherNotifier *m_notifier;
--- a/src/tools/clangpchmanagerbackend/source/clangpchmanagerbackend-source.pri
+++ b/src/tools/clangpchmanagerbackend/source/clangpchmanagerbackend-source.pri
@@ -29,4 +29,5 @@ HEADERS += \
    $$PWD/pchcreatorinterface.h \
    $$PWD/clangpathwatcherinterface.h \
    $$PWD/projectpartsinterface.h \
-    $$PWD/clangpathwatchernotifier.h
+    $$PWD/clangpathwatchernotifier.h \
+    $$PWD/changedfilepathcompressor.h
--- a/src/tools/clangpchmanagerbackend/source/clangpchmanagerbackend_global.h
+++ b/src/tools/clangpchmanagerbackend/source/clangpchmanagerbackend_global.h
@@ -27,8 +27,10 @@

 #ifdef UNIT_TESTS
 #define unitttest_public public
+#define non_unittest_final
 #else
 #define unitttest_public private
+#define non_unittest_final final
 #endif

 namespace llvm {
--- a/src/tools/clangpchmanagerbackend/source/includecollector.cpp
+++ b/src/tools/clangpchmanagerbackend/source/includecollector.cpp
@@ -65,14 +65,12 @@ std::vector<uint> IncludeCollector::generateExcludedIncludeFileUIDs(clang::FileM
    std::vector<uint> fileUIDs;
    fileUIDs.reserve(m_excludedIncludes.size());

-    auto generateUID = [&] (const Utils::SmallString &filePath) {
-        return fileManager.getFile({filePath.data(), filePath.size()})->getUID();
-    };
+    for (const Utils::SmallString &filePath : m_excludedIncludes) {
+        const clang::FileEntry *file = fileManager.getFile({filePath.data(), filePath.size()});

-    std::transform(m_excludedIncludes.begin(),
-                   m_excludedIncludes.end(),
-                   std::back_inserter(fileUIDs),
-                   generateUID);
+        if (file)
+            fileUIDs.push_back(file->getUID());
+    }

    std::sort(fileUIDs.begin(), fileUIDs.end());