From a513f25c39df21930e823100563553db2a2b65a9 Mon Sep 17 00:00:00 2001 From: Jarek Kobus Date: Tue, 16 Feb 2021 14:01:16 +0100 Subject: [PATCH] ClassView: Move the GUI related code out of Parser Don't create QStandardItem objects inside the non-gui thread, as it was in case of requestCurrentState() which is always called in parser's thread. As a result of parsing send a root's ParserTreeItem::ConstPtr now instead. Store the generated root inside Manager instead of inside Parser. Remove rootItemLocker as it's not needed now anymore. Move the implementation of canFetchMore(), fetchMore() and hasChildren() into Manager class. Now all the API of Parser class is used only in parser's thread (with the exception of constructor and destructor). Task-number: QTCREATORBUG-25317 Change-Id: I2b3c49918bf58266e6bea8acf65c975e19f7d9cb Reviewed-by: Christian Kandeler --- src/plugins/classview/classviewmanager.cpp | 65 +++++++++++--- src/plugins/classview/classviewmanager.h | 4 +- src/plugins/classview/classviewparser.cpp | 89 +------------------ src/plugins/classview/classviewparser.h | 8 +- .../classview/classviewparsertreeitem.h | 2 + 5 files changed, 61 insertions(+), 107 deletions(-) diff --git a/src/plugins/classview/classviewmanager.cpp b/src/plugins/classview/classviewmanager.cpp index 338c1eb3578..53ff9ee142e 100644 --- a/src/plugins/classview/classviewmanager.cpp +++ b/src/plugins/classview/classviewmanager.cpp @@ -96,6 +96,8 @@ public: //! separate thread for the parser QThread parserThread; + ParserTreeItem::ConstPtr m_root; + //! Internal manager state. \sa Manager::state bool state = false; @@ -135,28 +137,68 @@ 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 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 { - return d->parser.canFetchMore(item, skipRoot); + ParserTreeItem::ConstPtr ptr = findItemByRoot(item, skipRoot); + if (ptr.isNull()) + return false; + return ptr->canFetchMore(item); } /*! Checks \a item for lazy data population of a QStandardItemModel. \a skipRoot item is needed for the manual update, call not from model. */ - void Manager::fetchMore(QStandardItem *item, bool skipRoot) { - d->parser.fetchMore(item, skipRoot); + ParserTreeItem::ConstPtr ptr = findItemByRoot(item, skipRoot); + if (ptr.isNull()) + return; + ptr->fetchMore(item); } bool Manager::hasChildren(QStandardItem *item) const { - return d->parser.hasChildren(item); + ParserTreeItem::ConstPtr ptr = findItemByRoot(item); + if (ptr.isNull()) + return false; + return ptr->childCount() != 0; } void Manager::initialize() @@ -195,15 +237,16 @@ void Manager::initialize() resetParser(); }); - // translate data update from the parser to listeners - connect(&d->parser, &Parser::treeDataUpdate, - this, [this](QSharedPointer result) { - // do nothing if Manager is disabled + connect(&d->parser, &Parser::treeRegenerated, + this, [this](const ParserTreeItem::ConstPtr &root) { + d->m_root = root; + if (!state()) return; - emit treeDataUpdate(result); - + QSharedPointer rootItem(new QStandardItem()); + d->m_root->convertTo(rootItem.data()); + emit treeDataUpdate(rootItem); }, Qt::QueuedConnection); // connect to the cpp model manager for signals about document updates diff --git a/src/plugins/classview/classviewmanager.h b/src/plugins/classview/classviewmanager.h index af6d60ef045..4a8046f5ecb 100644 --- a/src/plugins/classview/classviewmanager.h +++ b/src/plugins/classview/classviewmanager.h @@ -30,9 +30,10 @@ #include #include - #include +#include "classviewparsertreeitem.h" + namespace ClassView { namespace Internal { @@ -59,6 +60,7 @@ signals: void treeDataUpdate(QSharedPointer result); private: + ParserTreeItem::ConstPtr findItemByRoot(const QStandardItem *item, bool skipRoot = false) const; void onProjectListChanged(); void resetParser(); diff --git a/src/plugins/classview/classviewparser.cpp b/src/plugins/classview/classviewparser.cpp index 49af9de8767..b3c04420070 100644 --- a/src/plugins/classview/classviewparser.cpp +++ b/src/plugins/classview/classviewparser.cpp @@ -111,12 +111,6 @@ public: //! List for files which has to be parsed QSet fileList; - //! Root item read write lock - QReadWriteLock rootItemLocker; - - //! Parsed root item - ParserTreeItem::ConstPtr rootItem; - //! Flat mode bool flatMode = false; }; @@ -151,39 +145,6 @@ Parser::~Parser() delete d; } -/*! - Checks \a item for lazy data population of a QStandardItemModel. -*/ - -bool Parser::canFetchMore(QStandardItem *item, bool skipRoot) const -{ - ParserTreeItem::ConstPtr ptr = findItemByRoot(item, skipRoot); - if (ptr.isNull()) - return false; - return ptr->canFetchMore(item); -} - -/*! - Checks \a item for lazy data population of a QStandardItemModel. - \a skipRoot skips the root item. -*/ - -void Parser::fetchMore(QStandardItem *item, bool skipRoot) const -{ - ParserTreeItem::ConstPtr ptr = findItemByRoot(item, skipRoot); - if (ptr.isNull()) - return; - ptr->fetchMore(item); -} - -bool Parser::hasChildren(QStandardItem *item) const -{ - ParserTreeItem::ConstPtr ptr = findItemByRoot(item); - if (ptr.isNull()) - return false; - return ptr->childCount() != 0; -} - /*! Switches to flat mode (without subprojects) if \a flat returns \c true. */ @@ -206,45 +167,6 @@ void Parser::aboutToShutdown() d->timer.stop(); } -/*! - Returns the internal tree item for \a item. \a skipRoot skips the root - item. -*/ - -ParserTreeItem::ConstPtr Parser::findItemByRoot(const QStandardItem *item, bool skipRoot) const -{ - if (!item) - return ParserTreeItem::ConstPtr(); - - // go item by item to the root - QList uiList; - const QStandardItem *cur = item; - while (cur) { - uiList.append(cur); - cur = cur->parent(); - } - - if (skipRoot && uiList.count() > 0) - uiList.removeLast(); - - ParserTreeItem::ConstPtr internal; - { - QReadLocker locker(&d->rootItemLocker); - internal = d->rootItem; - } - - 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; -} - /*! Parses the class and produces a new tree. @@ -486,16 +408,7 @@ void Parser::requestCurrentState() d->timer.stop(); // TODO: we need to have a fresh SessionManager data here, which we could pass to parse() - const ParserTreeItem::ConstPtr newRoot = parse(); - { - QWriteLocker locker(&d->rootItemLocker); - d->rootItem = newRoot; - } - - QSharedPointer std(new QStandardItem()); - d->rootItem->convertTo(std.data()); - - emit treeDataUpdate(std); + emit treeRegenerated(parse()); } // TODO: don't use Project class in this thread diff --git a/src/plugins/classview/classviewparser.h b/src/plugins/classview/classviewparser.h index 4d6f75f8fa1..ad114c3ea1c 100644 --- a/src/plugins/classview/classviewparser.h +++ b/src/plugins/classview/classviewparser.h @@ -52,11 +52,6 @@ public: explicit Parser(QObject *parent = nullptr); ~Parser() override; - // TODO: below three methods are called directly from different thread - bool canFetchMore(QStandardItem *item, bool skipRoot = false) const; - void fetchMore(QStandardItem *item, bool skipRoot = false) const; - bool hasChildren(QStandardItem *item) const; - void requestCurrentState(); void removeFiles(const QStringList &fileList); void resetDataToCurrentState(); @@ -65,7 +60,7 @@ public: void aboutToShutdown(); signals: - void treeDataUpdate(QSharedPointer result); + void treeRegenerated(const ParserTreeItem::ConstPtr &root); private: void setFileList(const QStringList &fileList); @@ -77,7 +72,6 @@ private: ParserTreeItem::Ptr getCachedOrParseProjectTree(const QStringList &fileList, const QString &projectId); ParserTreeItem::ConstPtr parse(); - ParserTreeItem::ConstPtr findItemByRoot(const QStandardItem *item, bool skipRoot = false) const; QStringList getAllFiles(const ProjectExplorer::Project *project); ParserTreeItem::Ptr addFlatTree(const ProjectExplorer::Project *project); diff --git a/src/plugins/classview/classviewparsertreeitem.h b/src/plugins/classview/classviewparsertreeitem.h index f34499a0e40..7e6901c3626 100644 --- a/src/plugins/classview/classviewparsertreeitem.h +++ b/src/plugins/classview/classviewparsertreeitem.h @@ -75,3 +75,5 @@ private: } // namespace Internal } // namespace ClassView + +Q_DECLARE_METATYPE(ClassView::Internal::ParserTreeItem::ConstPtr)