QtQml: Use a multihash to store executable CUs

You can produce multiple CUs for the same URL with createQmlObject() and
friends. They need to be marked during garbage collection and therefore
the engine needs to keep track of them.

With the multihash there can be a lot of CUs of the same URL. Searching
through them can take a lot of time. However, there is no point in
searching for an existing executable CU if we've just freshly compiled
the base CU. So, in those cases, insert directly instead.

Fixes: QTBUG-121436
Change-Id: I804dbc74d2ade118f6680a7fbde3f234699ccbc3
Reviewed-by: Fabian Kosmale <fabian.kosmale@qt.io>

src/qml/jsruntime/qv4engine.cpp

@@ -2113,22 +2113,28 @@ QQmlRefPointer<ExecutableCompilationUnit> ExecutionEngine::compileModule(
         }
     }
 
-    return executableCompilationUnit(std::move(unit));
+    return insertCompilationUnit(std::move(unit));
 }
 
 QQmlRefPointer<ExecutableCompilationUnit> ExecutionEngine::compilationUnitForUrl(const QUrl &url) const
 {
+    // Gives the _most recently inserted_ CU of that URL. That's what we want.
     return m_compilationUnits.value(url);
 }
 
 QQmlRefPointer<ExecutableCompilationUnit> ExecutionEngine::executableCompilationUnit(
         QQmlRefPointer<CompiledData::CompilationUnit> &&unit)
 {
-    QQmlRefPointer<QV4::ExecutableCompilationUnit> &result = m_compilationUnits[unit->finalUrl()];
-    if (!result || result->baseCompilationUnit() != unit)
-        result = ExecutableCompilationUnit::create(std::move(unit), this);
+    const QUrl url = unit->finalUrl();
+    auto [begin, end] = std::as_const(m_compilationUnits).equal_range(url);
 
-    return result;
+    for (auto it = begin; it != end; ++it) {
+        if ((*it)->baseCompilationUnit() == unit)
+            return *it;
+    }
+
+    return *m_compilationUnits.insert(
+            url, ExecutableCompilationUnit::create(std::move(unit), this));
 }
 
 void ExecutionEngine::trimCompilationUnits()
@@ -2171,8 +2177,7 @@ ExecutionEngine::Module ExecutionEngine::loadModule(const QUrl &url, const Execu
         return Module { *existingModule, nullptr };
 
     auto newModule = compileModule(resolved);
-    if (newModule)
-        m_compilationUnits.insert(resolved, newModule);
+    Q_ASSERT(!newModule || m_compilationUnits.contains(resolved, newModule));
 
     return Module { newModule, nullptr };
 }

src/qml/jsruntime/qv4engine_p.h

@@ -754,9 +754,18 @@ public:
             const QUrl &url, const QString &sourceCode, const QDateTime &sourceTimeStamp);
 
     QQmlRefPointer<ExecutableCompilationUnit> compilationUnitForUrl(const QUrl &url) const;
+
     QQmlRefPointer<ExecutableCompilationUnit> executableCompilationUnit(
             QQmlRefPointer<QV4::CompiledData::CompilationUnit> &&unit);
-    QHash<QUrl, QQmlRefPointer<ExecutableCompilationUnit>> compilationUnits() const
+
+    QQmlRefPointer<ExecutableCompilationUnit> insertCompilationUnit(
+            QQmlRefPointer<QV4::CompiledData::CompilationUnit> &&unit) {
+        QUrl url = unit->finalUrl();
+        return *m_compilationUnits.insert(
+                std::move(url), ExecutableCompilationUnit::create(std::move(unit), this));
+    }
+
+    QMultiHash<QUrl, QQmlRefPointer<ExecutableCompilationUnit>> compilationUnits() const
     {
         return m_compilationUnits;
     }
@@ -878,7 +887,7 @@ private:
 
     QVector<Deletable *> m_extensionData;
 
-    QHash<QUrl, QQmlRefPointer<ExecutableCompilationUnit>> m_compilationUnits;
+    QMultiHash<QUrl, QQmlRefPointer<ExecutableCompilationUnit>> m_compilationUnits;
 
     // QV4::PersistentValue would be preferred, but using QHash will create copies,
     // and QV4::PersistentValue doesn't like creating copies.

src/qml/jsruntime/qv4executablecompilationunit_p.h

@@ -204,7 +204,7 @@ public:
         return &m_compilationUnit->bindingPropertyDataPerObject.at(objectIndex);
     }
 
-    QQmlRefPointer<QV4::CompiledData::CompilationUnit> baseCompilationUnit() const
+    const QQmlRefPointer<QV4::CompiledData::CompilationUnit> &baseCompilationUnit() const
     {
         return m_compilationUnit;
     }

src/qml/jsruntime/qv4functionobject.cpp

@@ -256,7 +256,7 @@ QQmlRefPointer<ExecutableCompilationUnit> FunctionCtor::parse(ExecutionEngine *e
     if (engine->hasException)
         return nullptr;
 
-    return engine->executableCompilationUnit(cg.generateCompilationUnit());
+    return engine->insertCompilationUnit(cg.generateCompilationUnit());
 }
 
 ReturnedValue FunctionCtor::virtualCallAsConstructor(const FunctionObject *f, const Value *argv, int argc, const Value *newTarget)

src/qml/jsruntime/qv4script.cpp

@@ -94,7 +94,7 @@ void Script::parse()
         if (v4->hasException)
             return;
 
-        compilationUnit = v4->executableCompilationUnit(cg.generateCompilationUnit());
+        compilationUnit = v4->insertCompilationUnit(cg.generateCompilationUnit());
         vmFunction = compilationUnit->rootFunction();
     }
 
@@ -197,7 +197,7 @@ Script *Script::createFromFileOrCache(ExecutionEngine *engine, QmlContext *qmlCo
                     &cacheError)
                 : nullptr) {
         QQmlRefPointer<QV4::ExecutableCompilationUnit> jsUnit
-                = engine->executableCompilationUnit(
+                = engine->insertCompilationUnit(
                     QQml::makeRefPointer<QV4::CompiledData::CompilationUnit>(
                         cachedUnit->qmlData, cachedUnit->aotCompiledFunctions));
         return new QV4::Script(engine, qmlContext, jsUnit);

tests/auto/qml/qqmlcomponent/tst_qqmlcomponent.cpp

@@ -141,6 +141,7 @@ private slots:
     void removeBinding();
     void complexObjectArgument();
     void bindingEvaluationOrder();
+    void compilationUnitsWithSameUrl();
 
 private:
     QQmlEngine engine;
@@ -1492,6 +1493,41 @@ void tst_qqmlcomponent::bindingEvaluationOrder()
     QCOMPARE(myList[2].toString(), u"p2"_s);
 }
 
+void tst_qqmlcomponent::compilationUnitsWithSameUrl()
+{
+    QQmlEngine engine;
+    engine.setUiLanguage("de_CH");
+
+    std::vector<std::unique_ptr<QObject>> objects;
+    for (int i = 0; i < 10; ++i) {
+        QQmlComponent component(&engine);
+        component.setData(R"(
+            import QtQml
+            QtObject {
+                function returnThing() : string { return Qt.uiLanguage }
+            }
+        )", QUrl("duplicate.qml"));
+        QVERIFY2(component.isReady(), qPrintable(component.errorString()));
+
+        std::unique_ptr<QObject> o(component.create());
+        QVERIFY(o.get());
+
+        QString result;
+        QMetaObject::invokeMethod(o.get(), "returnThing", Q_RETURN_ARG(QString, result));
+        QCOMPARE(result, "de_CH");
+
+        objects.push_back(std::move(o));
+    }
+
+    gc(engine);
+
+    for (const auto &o: objects) {
+        QString result;
+        QMetaObject::invokeMethod(o.get(), "returnThing", Q_RETURN_ARG(QString, result));
+        QCOMPARE(result, "de_CH");
+    }
+}
+
 QTEST_MAIN(tst_qqmlcomponent)
 
 #include "tst_qqmlcomponent.moc"

tools/qmljs/qmljs.cpp

@@ -124,7 +124,7 @@ int main(int argc, char *argv[])
                 QString error;
                 if (unit->loadFromDisk(QUrl::fromLocalFile(fn), QFileInfo(fn).lastModified(), &error)) {
                     script.reset(new QV4::Script(
-                            &vm, nullptr, vm.executableCompilationUnit(std::move(unit))));
+                            &vm, nullptr, vm.insertCompilationUnit(std::move(unit))));
                 } else {
                     std::cout << "Error loading" << qPrintable(fn) << "from disk cache:" << qPrintable(error) << std::endl;
                 }