QCompletionEngine: avoid a node allocation on lookup

In matchHint() and lookupCache(), instead of checking for the
existence of 'parent' in 'cache', a new entry was created, if
needed.

However, the remainders of the functions could only fail in
that case, so don't allocate an empty map node here at all.
Wherever something sensible is added to the cache, there will
have to be an insertion of the same node anyway, so there's
no point doing that work here.

Allows to mark these functions const.

Also passed the QString argument by const-reference instead
of by value.

Done-with: Anton Kudryavtsev <a.kudryavtsev@netris.ru>
Change-Id: Ia873f246869a68eecaef83ce4a6b6369187456be
Reviewed-by: Anton Kudryavtsev <antkudr@mail.ru>
Reviewed-by: Edward Welbourne <edward.welbourne@qt.io>
Reviewed-by: Olivier Goffart (Woboq GmbH) <ogoffart@woboq.com>

1 files changed, 32 insertions, 15 deletions

diff --git a/src/widgets/util/qcompleter.cpp b/src/widgets/util/qcompleter.cpp
index 5a31eb4e52..e22f0e7f28 100644
--- a/src/widgets/util/qcompleter.cpp
+++ b/src/widgets/util/qcompleter.cpp
@@ -490,18 +490,25 @@ QMatchData QCompletionEngine::filterHistory()
 }
 
 // Returns a match hint from the cache by chopping the search string
-bool QCompletionEngine::matchHint(QString part, const QModelIndex& parent, QMatchData *hint)
+bool QCompletionEngine::matchHint(const QString &part, const QModelIndex &parent, QMatchData *hint) const
 {
-    if (c->cs == Qt::CaseInsensitive)
-        part = std::move(part).toLower();
+    if (part.isEmpty())
+        return false; // early out to avoid cache[parent] lookup costs
 
-    const CacheItem& map = cache[parent];
+    const auto cit = cache.find(parent);
+    if (cit == cache.end())
+        return false;
+
+    const CacheItem& map = *cit;
+    const auto mapEnd = map.end();
+
+    QString key = c->cs == Qt::CaseInsensitive ? part.toLower() : part;
 
-    QString key = part;
     while (!key.isEmpty()) {
         key.chop(1);
-        if (map.contains(key)) {
-            *hint = map[key];
+        const auto it = map.find(key);
+        if (it != mapEnd) {
+            *hint = *it;
             return true;
         }
     }
@@ -509,15 +516,25 @@ bool QCompletionEngine::matchHint(QString part, const QModelIndex& parent, QMatc
     return false;
 }
 
-bool QCompletionEngine::lookupCache(QString part, const QModelIndex& parent, QMatchData *m)
+bool QCompletionEngine::lookupCache(const QString &part, const QModelIndex &parent, QMatchData *m) const
 {
-   if (c->cs == Qt::CaseInsensitive)
-        part = std::move(part).toLower();
-   const CacheItem& map = cache[parent];
-   if (!map.contains(part))
-       return false;
-   *m = map[part];
-   return true;
+    if (part.isEmpty())
+        return false; // early out to avoid cache[parent] lookup costs
+
+    const auto cit = cache.find(parent);
+    if (cit == cache.end())
+        return false;
+
+    const CacheItem& map = *cit;
+
+    const QString key = c->cs == Qt::CaseInsensitive ? part.toLower() : part;
+
+    const auto it = map.find(key);
+    if (it == map.end())
+        return false;
+
+    *m = it.value();
+    return true;
 }
 
 // When the cache size exceeds 1MB, it clears out about 1/2 of the cache.