ClassView: Move delaying of update into the main thread

Minimize the communication with parser's thread.
Don't call getParseDocumentTree() multiple times
when subsequent request for the same document comes
before the timeout.
Don't queue again already queued documentUpdated()
signal (it's emitted form non-gui thread).
Prepare for moving the calls to SessionManager out of the
parser's thread.

Task-number: QTCREATORBUG-25317
Change-Id: I5d4898b5addbb589d415e00c66de5cba7b96d512
Reviewed-by: Christian Kandeler <christian.kandeler@qt.io>

src/plugins/classview/classviewmanager.cpp

@@ -41,6 +41,7 @@
 #include <texteditor/texteditor.h>
 
 #include <QThread>
+#include <QTimer>
 
 using namespace Core;
 using namespace Utils;
@@ -92,13 +93,65 @@ public:
     QThread m_parserThread;
     ParserTreeItem::ConstPtr m_root;
 
+    QTimer m_timer;
+    QHash<QString, CPlusPlus::Document::Ptr> m_awaitingDocuments;
+
     //! Internal manager state. \sa Manager::state
     bool state = false;
 
     //! there is some massive operation ongoing so temporary we should wait
     bool disableCodeParser = false;
+
+    void cancelScheduledUpdate();
+    void resetParser();
+    ParserTreeItem::ConstPtr findItemByRoot(const QStandardItem *item, bool skipRoot = false) const;
 };
 
+void ManagerPrivate::cancelScheduledUpdate()
+{
+    m_timer.stop();
+    m_awaitingDocuments.clear();
+}
+
+void ManagerPrivate::resetParser()
+{
+    cancelScheduledUpdate();
+    QMetaObject::invokeMethod(m_parser, &Parser::resetDataToCurrentState, Qt::QueuedConnection);
+}
+
+/*!
+    Returns the internal tree item for \a item. \a skipRoot skips the root
+    item.
+*/
+ParserTreeItem::ConstPtr ManagerPrivate::findItemByRoot(const QStandardItem *item, bool skipRoot) const
+{
+    if (!item)
+        return ParserTreeItem::ConstPtr();
+
+    // go item by item to the root
+    QList<const QStandardItem *> uiList;
+    const QStandardItem *cur = item;
+    while (cur) {
+        uiList.append(cur);
+        cur = cur->parent();
+    }
+
+    if (skipRoot && uiList.count() > 0)
+        uiList.removeLast();
+
+    ParserTreeItem::ConstPtr internal = m_root;
+    while (uiList.count() > 0) {
+        cur = uiList.last();
+        uiList.removeLast();
+        const SymbolInformation &inf = Internal::symbolInformationFromItem(cur);
+        internal = internal->child(inf);
+        if (internal.isNull())
+            break;
+    }
+
+    return internal;
+}
+
 ///////////////////////////////// Manager //////////////////////////////////
 
 Manager::Manager(QObject *parent)
@@ -132,45 +185,12 @@ Manager *Manager::instance()
     return managerInstance;
 }
 
-/*!
-    Returns the internal tree item for \a item. \a skipRoot skips the root
-    item.
-*/
-ParserTreeItem::ConstPtr Manager::findItemByRoot(const QStandardItem *item, bool skipRoot) const
-{
-    if (!item)
-        return ParserTreeItem::ConstPtr();
-
-    // go item by item to the root
-    QList<const QStandardItem *> uiList;
-    const QStandardItem *cur = item;
-    while (cur) {
-        uiList.append(cur);
-        cur = cur->parent();
-    }
-
-    if (skipRoot && uiList.count() > 0)
-        uiList.removeLast();
-
-    ParserTreeItem::ConstPtr internal = d->m_root;
-    while (uiList.count() > 0) {
-        cur = uiList.last();
-        uiList.removeLast();
-        const SymbolInformation &inf = Internal::symbolInformationFromItem(cur);
-        internal = internal->child(inf);
-        if (internal.isNull())
-            break;
-    }
-
-    return internal;
-}
-
 /*!
     Checks \a item for lazy data population of a QStandardItemModel.
 */
 bool Manager::canFetchMore(QStandardItem *item, bool skipRoot) const
 {
-    ParserTreeItem::ConstPtr ptr = findItemByRoot(item, skipRoot);
+    ParserTreeItem::ConstPtr ptr = d->findItemByRoot(item, skipRoot);
     if (ptr.isNull())
         return false;
     return ptr->canFetchMore(item);
@@ -182,7 +202,7 @@ bool Manager::canFetchMore(QStandardItem *item, bool skipRoot) const
 */
 void Manager::fetchMore(QStandardItem *item, bool skipRoot)
 {
-    ParserTreeItem::ConstPtr ptr = findItemByRoot(item, skipRoot);
+    ParserTreeItem::ConstPtr ptr = d->findItemByRoot(item, skipRoot);
     if (ptr.isNull())
         return;
     ptr->fetchMore(item);
@@ -190,7 +210,7 @@ void Manager::fetchMore(QStandardItem *item, bool skipRoot)
 
 bool Manager::hasChildren(QStandardItem *item) const
 {
-    ParserTreeItem::ConstPtr ptr = findItemByRoot(item);
+    ParserTreeItem::ConstPtr ptr = d->findItemByRoot(item);
     if (ptr.isNull())
         return false;
     return ptr->childCount();
@@ -199,6 +219,7 @@ bool Manager::hasChildren(QStandardItem *item) const
 void Manager::initialize()
 {
     using ProjectExplorer::SessionManager;
+    d->m_timer.setSingleShot(true);
 
     // connections to enable/disable navi widget factory
     SessionManager *sessionManager = SessionManager::instance();
@@ -215,6 +236,7 @@ void Manager::initialize()
 
         // disable tree updates to speed up
         d->disableCodeParser = true;
+        d->cancelScheduledUpdate();
     });
     connect(ProgressManager::instance(), &ProgressManager::allTasksFinished,
             this, [this](Id type) {
@@ -229,7 +251,7 @@ void Manager::initialize()
             return;
 
         // request to update a tree to the current state
-        resetParser();
+        d->resetParser();
     });
 
     connect(d->m_parser, &Parser::treeRegenerated,
@@ -258,10 +280,23 @@ void Manager::initialize()
         if (d->disableCodeParser)
             return;
 
-        QMetaObject::invokeMethod(d->m_parser, [this, doc]() {
+        if (doc.data() == nullptr)
-            d->m_parser->parseDocument(doc); }, Qt::QueuedConnection);
+            return;
+
+        d->m_awaitingDocuments.insert(doc->fileName(), doc);
+        d->m_timer.start(400); // Accumulate multiple requests into one, restarts the timer
+    });
+
+    connect(&d->m_timer, &QTimer::timeout, this, [this]() {
+        const QList<CPlusPlus::Document::Ptr> docsToBeUpdated = d->m_awaitingDocuments.values();
+        d->cancelScheduledUpdate();
+        if (!state() || d->disableCodeParser) // enabling any of them will trigger the total update
+            return;
+        QMetaObject::invokeMethod(d->m_parser, [this, docsToBeUpdated]() {
+            d->m_parser->updateDocuments(docsToBeUpdated);
         }, Qt::QueuedConnection);
-    //
+    });
+
     connect(codeModelManager, &CppTools::CppModelManager::aboutToRemoveFiles,
             d->m_parser, &Parser::removeFiles, Qt::QueuedConnection);
 }
@@ -295,12 +330,7 @@ void Manager::setState(bool state)
     d->state = state;
 
     if (state) // enabled - request a current snapshots etc?..
-        resetParser();
+        d->resetParser();
-}
-
-void Manager::resetParser()
-{
-    QMetaObject::invokeMethod(d->m_parser, &Parser::resetDataToCurrentState, Qt::QueuedConnection);
 }
 
 /*!

src/plugins/classview/classviewmanager.h

@@ -60,10 +60,7 @@ signals:
     void treeDataUpdate(QSharedPointer<QStandardItem> result);
 
 private:
-    ParserTreeItem::ConstPtr findItemByRoot(const QStandardItem *item, bool skipRoot = false) const;
     void onProjectListChanged();
-    void resetParser();
-
     void initialize();
 
     inline bool state() const;

src/plugins/classview/classviewparser.cpp

@@ -43,7 +43,6 @@
 #include <QDebug>
 #include <QHash>
 #include <QSet>
-#include <QTimer>
 #include <QElapsedTimer>
 
 enum { debug = false };
@@ -77,15 +76,9 @@ namespace Internal {
 class ParserPrivate
 {
 public:
-    // Keep timer as a child of Parser in order to move it together with its parent
-    // into another thread.
-    ParserPrivate(QObject *parent) : timer(parent) {}
-
     //! Get document from documentList
     CPlusPlus::Document::Ptr document(const QString &fileName) const;
 
-    QTimer timer;
-
     struct DocumentCache {
         unsigned treeRevision = 0;
         ParserTreeItem::ConstPtr tree;
@@ -123,12 +116,8 @@ CPlusPlus::Document::Ptr ParserPrivate::document(const QString &fileName) const
 
 Parser::Parser(QObject *parent)
     : QObject(parent),
-    d(new ParserPrivate(this))
+    d(new ParserPrivate())
 {
-    d->timer.setSingleShot(true);
-
-    // timer for emitting changes
-    connect(&d->timer, &QTimer::timeout, this, &Parser::requestCurrentState);
 }
 
 /*!
@@ -304,25 +293,22 @@ ParserTreeItem::ConstPtr Parser::getCachedOrParseDocumentTree(const CPlusPlus::D
 }
 
 /*!
-    Parses the document \a doc if it is in the project files and adds a tree to
+    Parses the document list \a docs if they are in the project files and adds a tree to
     the internal storage.
 */
 
-void Parser::parseDocument(const CPlusPlus::Document::Ptr &doc)
+void Parser::updateDocuments(const QList<CPlusPlus::Document::Ptr> &docs)
 {
-    if (doc.isNull())
+    for (const CPlusPlus::Document::Ptr &doc: docs) {
-        return;
-
         const QString &name = doc->fileName();
 
         // if it is external file (not in any of our projects)
         if (!d->fileList.contains(name))
-        return;
+            continue;
 
         getParseDocumentTree(doc);
-
+    }
-    if (!d->timer.isActive())
+    requestCurrentState();
-        d->timer.start(400); //! Delay in msecs before an update
 }
 
 /*!
@@ -393,8 +379,6 @@ void Parser::resetDataToCurrentState()
 
 void Parser::requestCurrentState()
 {
-    d->timer.stop();
-
     // TODO: we need to have a fresh SessionManager data here, which we could pass to parse()
     emit treeRegenerated(parse());
 }

src/plugins/classview/classviewparser.h

@@ -55,9 +55,10 @@ public:
     void requestCurrentState();
     void removeFiles(const QStringList &fileList);
     void resetDataToCurrentState();
-    void parseDocument(const CPlusPlus::Document::Ptr &doc);
     void setFlatMode(bool flat);
 
+    void updateDocuments(const QList<CPlusPlus::Document::Ptr> &docs);
+
 signals:
     void treeRegenerated(const ParserTreeItem::ConstPtr &root);