From 7fe5611365aa9d5065fe8ba62fed9a07c722ea1c Mon Sep 17 00:00:00 2001 From: Marc Mutz Date: Tue, 9 Nov 2021 11:04:46 +0100 Subject: [PATCH] QHash: optimize value(key) and key(value) callers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ... by not injecting potentially-expensive temporary objects into the caller's stack frame. Default arguments are a convenient way to avoid overloads, but if the defaulted argument isn't a Trivial Type, and the common use case is not to pass the extra argument explicitly, the construction of the temporary can dominate the call's runtime. Since QHash is generic code, we don't know whether T or Key are expensive or cheap to construct, so use overloading instead of default arguments to avoid injecting needless code into call sites. [ChangeLog][QtCore][Potentially Source-Incompatible Changes][QHash/QMultiHash] The value(key) and key(value) functions are now overloaded on presence of the defaultValue (was: defaulted argument) to avoid injecting temporary objects into the caller's stack frame. Task-number: QTBUG-98117 Change-Id: I80fdd5436f3de3e4bbe20242fe45916aef62ff0c Reviewed-by: Mårten Nordheim Reviewed-by: Edward Welbourne Reviewed-by: Lars Knoll --- src/corelib/tools/qhash.cpp | 31 +++++++------ src/corelib/tools/qhash.h | 93 ++++++++++++++++++++++++++++++++----- 2 files changed, 99 insertions(+), 25 deletions(-) diff --git a/src/corelib/tools/qhash.cpp b/src/corelib/tools/qhash.cpp index 6d7ed95acae..bc8c9490d9e 100644 --- a/src/corelib/tools/qhash.cpp +++ b/src/corelib/tools/qhash.cpp @@ -1865,14 +1865,15 @@ size_t qHash(long double key, size_t seed) noexcept \sa count(), QMultiHash::contains() */ -/*! \fn template T QHash::value(const Key &key, const T &defaultValue = T()) const +/*! \fn template T QHash::value(const Key &key) const + \fn template T QHash::value(const Key &key, const T &defaultValue) const \overload Returns the value associated with the \a key. If the hash contains no item with the \a key, the function - returns \a defaultValue, which is a \l{default-constructed value} if the - parameter has not been specified. + returns \a defaultValue, or a \l{default-constructed value} if this + parameter has not been supplied. */ /*! \fn template T &QHash::operator[](const Key &key) @@ -1935,11 +1936,13 @@ size_t qHash(long double key, size_t seed) noexcept */ /*! - \fn template Key QHash::key(const T &value, const Key &defaultKey = Key()) const + \fn template Key QHash::key(const T &value) const + \fn template Key QHash::key(const T &value, const Key &defaultKey) const \since 4.3 - Returns the first key mapped to \a value, or \a defaultKey if the - hash contains no item mapped to \a value. + Returns the first key mapped to \a value. If the hash contains no item + mapped to \a value, returns \a defaultKey, or a \l{default-constructed + value}{default-constructed key} if this parameter has not been supplied. This function can be slow (\l{linear time}), because QHash's internal data structure is optimized for fast lookup by key, not @@ -2899,14 +2902,14 @@ size_t qHash(long double key, size_t seed) noexcept \sa keys(), values() */ -/*! \fn template T QMultiHash::value(const Key &key, const T &defaultValue = T()) const - \overload +/*! \fn template T QMultiHash::value(const Key &key) const + \fn template T QMultiHash::value(const Key &key, const T &defaultValue) const Returns the value associated with the \a key. If the hash contains no item with the \a key, the function - returns \a defaultValue, which is a \l{default-constructed value} if the - parameter has not been specified. + returns \a defaultValue, or a \l{default-constructed value} if this + parameter has not been supplied. If there are multiple items for the \a key in the hash, the value of the most recently @@ -3054,11 +3057,13 @@ size_t qHash(long double key, size_t seed) noexcept */ /*! - \fn template Key QMultiHash::key(const T &value, const Key &defaultKey = Key()) const + \fn template Key QMultiHash::key(const T &value) const + \fn template Key QMultiHash::key(const T &value, const Key &defaultKey) const \since 4.3 - Returns the first key mapped to \a value, or \a defaultKey if the - hash contains no item mapped to \a value. + Returns the first key mapped to \a value. If the hash contains no item + mapped to \a value, returns \a defaultKey, or a \l{default-constructed + value}{default-constructed key} if this parameter has not been supplied. This function can be slow (\l{linear time}), because QMultiHash's internal data structure is optimized for fast lookup by key, not diff --git a/src/corelib/tools/qhash.h b/src/corelib/tools/qhash.h index 3d92d95e628..0d5fa521112 100644 --- a/src/corelib/tools/qhash.h +++ b/src/corelib/tools/qhash.h @@ -926,28 +926,64 @@ public: return contains(key) ? 1 : 0; } - Key key(const T &value, const Key &defaultKey = Key()) const noexcept +private: + const Key *keyImpl(const T &value) const noexcept { if (d) { const_iterator i = begin(); while (i != end()) { if (i.value() == value) - return i.key(); + return &i.key(); ++i; } } - return defaultKey; + return nullptr; } - T value(const Key &key, const T &defaultValue = T()) const noexcept + +public: + Key key(const T &value) const noexcept + { + if (auto *k = keyImpl(value)) + return *k; + else + return Key(); + } + Key key(const T &value, const Key &defaultKey) const noexcept + { + if (auto *k = keyImpl(value)) + return *k; + else + return defaultKey; + } + +private: + T *valueImpl(const Key &key) const noexcept { if (d) { Node *n = d->findNode(key); if (n) - return n->value; + return &n->value; } - return defaultValue; + return nullptr; } +public: + T value(const Key &key) const noexcept + { + if (T *v = valueImpl(key)) + return *v; + else + return T(); + } + + T value(const Key &key, const T &defaultValue) const noexcept + { + if (T *v = valueImpl(key)) + return *v; + else + return defaultValue; + } + T &operator[](const Key &key) { detach(); @@ -1427,30 +1463,63 @@ public: return d->findNode(key) != nullptr; } - Key key(const T &value, const Key &defaultKey = Key()) const noexcept +private: + const Key *keyImpl(const T &value) const noexcept { if (d) { auto i = d->begin(); while (i != d->end()) { Chain *e = i.node()->value; if (e->contains(value)) - return i.node()->key; + return &i.node()->key; ++i; } } - return defaultKey; + return nullptr; } - T value(const Key &key, const T &defaultValue = T()) const noexcept +public: + Key key(const T &value) const noexcept + { + if (auto *k = keyImpl(value)) + return *k; + else + return Key(); + } + Key key(const T &value, const Key &defaultKey) const noexcept + { + if (auto *k = keyImpl(value)) + return *k; + else + return defaultKey; + } + +private: + T *valueImpl(const Key &key) const noexcept { if (d) { Node *n = d->findNode(key); if (n) { Q_ASSERT(n->value); - return n->value->value; + return &n->value->value; } } - return defaultValue; + return nullptr; + } +public: + T value(const Key &key) const noexcept + { + if (auto *v = valueImpl(key)) + return *v; + else + return T(); + } + T value(const Key &key, const T &defaultValue) const noexcept + { + if (auto *v = valueImpl(key)) + return *v; + else + return defaultValue; } T &operator[](const Key &key)