From 4a15f6d16bbcd4ec6a75dd03ef38657264982b82 Mon Sep 17 00:00:00 2001 From: Jarek Kobus Date: Tue, 16 Feb 2021 01:37:49 +0100 Subject: [PATCH] ClassView: Optimize operator== of internal data Make all of the fields of SymbolLocation and SymbolInformation const members. Optimize a bit operator== of these structures: check first if hashes are non-equal - in this case return false early. Fix hash type of SymbolInformation for porting to Qt6 purpose. Task-number: QTCREATORBUG-25317 Task-number: QTCREATORBUG-24098 Change-Id: I769f99ff3157093e9f10ee3929bc7f6eb83f34e3 Reviewed-by: Christian Kandeler --- src/plugins/classview/classviewmanager.cpp | 14 ++++++-------- src/plugins/classview/classviewparser.cpp | 1 + .../classview/classviewsymbolinformation.cpp | 18 +++++++++++------- .../classview/classviewsymbolinformation.h | 12 +++++++----- .../classview/classviewsymbollocation.cpp | 18 +++++++----------- .../classview/classviewsymbollocation.h | 12 ++++++------ 6 files changed, 38 insertions(+), 37 deletions(-) diff --git a/src/plugins/classview/classviewmanager.cpp b/src/plugins/classview/classviewmanager.cpp index ce5bfcec204..8284071d217 100644 --- a/src/plugins/classview/classviewmanager.cpp +++ b/src/plugins/classview/classviewmanager.cpp @@ -317,8 +317,7 @@ void Manager::gotoLocations(const QList &list) return; // Default to first known location - SymbolLocation loc = *locations.constBegin(); - + auto locationIt = locations.constBegin(); if (locations.size() > 1) { // The symbol has multiple locations. Check if we are already at one location, // and if so, cycle to the "next" one @@ -329,20 +328,19 @@ void Manager::gotoLocations(const QList &list) int line; int column; textEditor->convertPosition(textEditor->position(), &line, &column); - SymbolLocation current(fileName, line, column); - QSet::const_iterator it = locations.constFind(current); - QSet::const_iterator end = locations.constEnd(); - if (it != end) { + const SymbolLocation current(fileName, line, column); + if (auto it = locations.constFind(current), end = locations.constEnd(); it != end) { // we already are at the symbol, cycle to next location ++it; if (it == end) it = locations.constBegin(); - loc = *it; + locationIt = it; } } } + const SymbolLocation &location = *locationIt; // line is 1-based, column is 0-based - gotoLocation(loc.fileName(), loc.line(), loc.column() - 1); + gotoLocation(location.fileName(), location.line(), location.column() - 1); } /*! diff --git a/src/plugins/classview/classviewparser.cpp b/src/plugins/classview/classviewparser.cpp index 27db704c128..63f3048862f 100644 --- a/src/plugins/classview/classviewparser.cpp +++ b/src/plugins/classview/classviewparser.cpp @@ -479,6 +479,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); diff --git a/src/plugins/classview/classviewsymbolinformation.cpp b/src/plugins/classview/classviewsymbolinformation.cpp index 59d5e24aab7..b1bee4be566 100644 --- a/src/plugins/classview/classviewsymbolinformation.cpp +++ b/src/plugins/classview/classviewsymbolinformation.cpp @@ -49,13 +49,16 @@ SymbolInformation::SymbolInformation() : } SymbolInformation::SymbolInformation(const QString &valueName, const QString &valueType, - int valueIconType) : - m_iconType(valueIconType), - m_name(valueName), - m_type(valueType) + int valueIconType) + : m_iconType(valueIconType) +#if QT_VERSION >= QT_VERSION_CHECK(6, 0, 0) + , m_hash(qHashMulti(0, valueIconType, valueName, valueType)) +#else + , m_hash(qHash(qMakePair(valueIconType, qMakePair(valueName, valueType)))) +#endif + , m_name(valueName) + , m_type(valueType) { - // calculate hash - m_hash = qHash(qMakePair(m_iconType, qMakePair(m_name, m_type))); } /*! @@ -94,7 +97,8 @@ int SymbolInformation::iconTypeSortOrder() const static QHash sortOrder; - // initialization + // TODO: Check if this static initialization is OK when SymbolInformation object are + // instantiated in different threads. if (sortOrder.isEmpty()) { for (int i : IconSortOrder) sortOrder.insert(i, sortOrder.count()); diff --git a/src/plugins/classview/classviewsymbolinformation.h b/src/plugins/classview/classviewsymbolinformation.h index 80d13a873bd..8d5394aaddd 100644 --- a/src/plugins/classview/classviewsymbolinformation.h +++ b/src/plugins/classview/classviewsymbolinformation.h @@ -25,6 +25,8 @@ #pragma once +#include + #include #include @@ -47,17 +49,17 @@ public: inline uint hash() const { return m_hash; } inline bool operator==(const SymbolInformation &other) const { - return iconType() == other.iconType() && name() == other.name() + return hash() == other.hash() && iconType() == other.iconType() && name() == other.name() && type() == other.type(); } int iconTypeSortOrder() const; private: - int m_iconType; //!< icon type - uint m_hash; //!< precalculated hash value - to speed up qHash - QString m_name; //!< symbol name (e.g. SymbolInformation) - QString m_type; //!< symbol type (e.g. (int char)) + const int m_iconType; + const Utils::QHashValueType m_hash; // precalculated hash value - to speed up qHash + const QString m_name; // symbol name (e.g. SymbolInformation) + const QString m_type; // symbol type (e.g. (int char)) }; diff --git a/src/plugins/classview/classviewsymbollocation.cpp b/src/plugins/classview/classviewsymbollocation.cpp index d8b241c333b..a371fc14ed9 100644 --- a/src/plugins/classview/classviewsymbollocation.cpp +++ b/src/plugins/classview/classviewsymbollocation.cpp @@ -45,20 +45,16 @@ SymbolLocation::SymbolLocation() : { } -SymbolLocation::SymbolLocation(QString file, int lineNumber, int columnNumber) : - m_fileName(file), - m_line(lineNumber), - m_column(columnNumber) -{ - if (m_column < 0) - m_column = 0; - - // pre-computate hash value +SymbolLocation::SymbolLocation(const QString &file, int lineNumber, int columnNumber) + : m_fileName(file) + , m_line(lineNumber) + , m_column(qMax(columnNumber, 0)) #if QT_VERSION >= QT_VERSION_CHECK(6, 0, 0) - m_hash = qHashMulti(0, m_fileName, m_line, m_column); + , m_hash(qHashMulti(0, m_fileName, m_line, m_column)) #else - m_hash = qHash(qMakePair(m_fileName, qMakePair(m_line, m_column))); + , m_hash(qHash(qMakePair(m_fileName, qMakePair(m_line, m_column)))) #endif +{ } } // namespace Internal diff --git a/src/plugins/classview/classviewsymbollocation.h b/src/plugins/classview/classviewsymbollocation.h index a97d5ac03ca..bbf854150dd 100644 --- a/src/plugins/classview/classviewsymbollocation.h +++ b/src/plugins/classview/classviewsymbollocation.h @@ -40,7 +40,7 @@ public: SymbolLocation(); //! Constructor - explicit SymbolLocation(QString file, int lineNumber = 0, int columnNumber = 0); + explicit SymbolLocation(const QString &file, int lineNumber = 0, int columnNumber = 0); inline const QString &fileName() const { return m_fileName; } inline int line() const { return m_line; } @@ -48,15 +48,15 @@ public: inline Utils::QHashValueType hash() const { return m_hash; } inline bool operator==(const SymbolLocation &other) const { - return line() == other.line() && column() == other.column() + return hash() == other.hash() && line() == other.line() && column() == other.column() && fileName() == other.fileName(); } private: - QString m_fileName; //!< file name - int m_line; //!< line number - int m_column; //!< column - Utils::QHashValueType m_hash; //!< precalculated hash value for the object, to speed up qHash + const QString m_fileName; + const int m_line; + const int m_column; + const Utils::QHashValueType m_hash; // precalculated hash value - to speed up qHash }; //! qHash overload for QHash/QSet