From af212e5e4edb73978298cd030b15deb8d8c28183 Mon Sep 17 00:00:00 2001
From: Ulf Hermann <ulf.hermann@qt.io>
Date: Thu, 18 Jan 2024 10:04:17 +0100
Subject: [PATCH] QmlCompiler: Use a hash set for the list of conversions

There can be a lot of them and we don't want quadratic behavior. It might be possible to further improve on this using e.g. sorted lists
and algorithms provided by the STL. However, such an implementation
would be more complicated and would require weighing several trade-offs.

Pick-to: 6.7 6.6 6.5
Fixes: QTBUG-121139
Change-Id: I717b49bd4af97abcaae9ae78d1e1b31d5d462952
Reviewed-by: Fabian Kosmale <fabian.kosmale@qt.io>
Reviewed-by: Marc Mutz <marc.mutz@qt.io>
---
 src/qmlcompiler/qqmljsbasicblocks.cpp | 38 +++++++++++++++------------
 src/qmlcompiler/qqmljsbasicblocks_p.h |  4 ++-
 2 files changed, 24 insertions(+), 18 deletions(-)

diff --git a/src/qmlcompiler/qqmljsbasicblocks.cpp b/src/qmlcompiler/qqmljsbasicblocks.cpp
index 0aedcbbd3c..67d331e9ff 100644
--- a/src/qmlcompiler/qqmljsbasicblocks.cpp
+++ b/src/qmlcompiler/qqmljsbasicblocks.cpp
@@ -311,16 +311,6 @@ static bool containsAny(const ContainerA &container, const ContainerB &elements)
     return false;
 }
 
-template<typename ContainerA, typename ContainerB>
-static bool containsAll(const ContainerA &container, const ContainerB &elements)
-{
-    for (const auto &element : elements) {
-        if (!container.contains(element))
-            return false;
-    }
-    return true;
-}
-
 template<class Key, class T, class Compare = std::less<Key>,
          class KeyContainer = QList<Key>, class MappedContainer = QList<T>>
 class NewFlatMap
@@ -347,7 +337,7 @@ private:
 
 struct PendingBlock
 {
-    QList<int> conversions;
+    QQmlJSBasicBlocks::Conversions conversions;
     int start = -1;
     bool registerActive = false;
 };
@@ -434,7 +424,7 @@ void QQmlJSBasicBlocks::populateReaderLocations()
             auto nextBlock = m_basicBlocks.find(block.start);
             auto currentBlock = nextBlock++;
             bool registerActive = block.registerActive;
-            QList<int> conversions = block.conversions;
+            Conversions conversions = block.conversions;
 
             const auto blockEnd = (nextBlock == m_basicBlocks.end())
                     ? m_annotations.end()
@@ -446,7 +436,7 @@ void QQmlJSBasicBlocks::populateReaderLocations()
             for (; blockInstr != blockEnd; ++blockInstr) {
                 if (registerActive
                         && blockInstr->second.typeConversions.contains(writtenRegister)) {
-                    conversions.append(blockInstr.key());
+                    conversions.insert(blockInstr.key());
                 }
 
                 for (auto readIt = blockInstr->second.readRegisters.constBegin(),
@@ -473,12 +463,26 @@ void QQmlJSBasicBlocks::populateReaderLocations()
                 // If we find that an already processed block has the register activated by this jump,
                 // we need to re-evaluate it. We also need to propagate any newly found conversions.
                 const auto processed = processedBlocks.find(blockStart);
-                if (processed == processedBlocks.end())
+                if (processed == processedBlocks.end()) {
                     blocks.append({conversions, blockStart, registerActive});
-                else if (registerActive && !processed->registerActive)
+                } else if (registerActive && !processed->registerActive) {
                     blocks.append({conversions, blockStart, registerActive});
-                else if (!containsAll(processed->conversions, conversions))
-                    blocks.append({processed->conversions + conversions, blockStart, registerActive});
+                } else {
+
+                    // TODO: Use unite() once it is fixed.
+                    // We don't use unite() here since it would be more expensive. unite()
+                    // effectively loops on only insert() and insert() does a number of checks
+                    // each time. We trade those checks for calculating the hash twice on each
+                    // iteration. Calculating the hash is very cheap for integers.
+                    Conversions merged = processed->conversions;
+                    for (const int conversion : std::as_const(conversions)) {
+                        if (!merged.contains(conversion))
+                            merged.insert(conversion);
+                    }
+
+                    if (merged.size() > processed->conversions.size())
+                        blocks.append({std::move(merged), blockStart, registerActive});
+                }
             };
 
             if (!currentBlock->second.jumpIsUnconditional && nextBlock != m_basicBlocks.end())
diff --git a/src/qmlcompiler/qqmljsbasicblocks_p.h b/src/qmlcompiler/qqmljsbasicblocks_p.h
index 303070a2f2..346c91b951 100644
--- a/src/qmlcompiler/qqmljsbasicblocks_p.h
+++ b/src/qmlcompiler/qqmljsbasicblocks_p.h
@@ -24,6 +24,8 @@ QT_BEGIN_NAMESPACE
 class Q_QMLCOMPILER_EXPORT QQmlJSBasicBlocks : public QQmlJSCompilePass
 {
 public:
+    using Conversions = QSet<int>;
+
     struct BasicBlock {
         QList<int> jumpOrigins;
         QList<int> readRegisters;
@@ -55,7 +57,7 @@ private:
     {
         QList<QQmlJSScope::ConstPtr> trackedTypes;
         QHash<int, QQmlJSScope::ConstPtr> typeReaders;
-        QHash<int, QList<int>> registerReadersAndConversions;
+        QHash<int, Conversions> registerReadersAndConversions;
         int trackedRegister;
     };