QtQml: Get rid of the module mutex

It only exists so that the type loader can query pre-compiled and native modules from the loader thread. However, the type loader already has a mutex of its own. We can use that to inject a "native" blob into its script cache for the same effect. We need to get rid of the mutex so that we can use the module map for other compilation units, too. Change-Id: I5a9c266ea36b50f5ea69214110def644f7501674 Reviewed-by: Fabian Kosmale <fabian.kosmale@qt.io>
author: Ulf Hermann <ulf.hermann@qt.io> 2023-12-22 14:06:34 +0100
committer: Ulf Hermann <ulf.hermann@qt.io> 2024-01-10 11:22:43 +0100
commit: 47ddbb72e052989622f5fa23e032c787bed5f2e5 (patch)
tree: 6c18a31045527b7b43be331651607ac34908639a /src
parent: be6d1499af75228341227d15441284e07cfe1e41 (diff)
10 files changed, 129 insertions, 85 deletions
diff --git a/src/qml/common/qv4compileddata.cpp b/src/qml/common/qv4compileddata.cpp
index 4ed53e6907..14315084bd 100644
--- a/src/qml/common/qv4compileddata.cpp
+++ b/src/qml/common/qv4compileddata.cpp
@@ -108,6 +108,15 @@ bool CompilationUnit::saveToDisk(const QUrl &unitUrl, QString *errorString)
             });
 }
 
+QStringList CompilationUnit::moduleRequests() const
+{
+    QStringList requests;
+    requests.reserve(data->moduleRequestTableSize);
+    for (uint i = 0; i < data->moduleRequestTableSize; ++i)
+        requests << stringAt(data->moduleRequestTable()[i]);
+    return requests;
+}
+
 } // namespace CompiledData
 } // namespace QV4
 
diff --git a/src/qml/common/qv4compileddata_p.h b/src/qml/common/qv4compileddata_p.h
index f7654661e2..0c7b07936c 100644
--- a/src/qml/common/qv4compileddata_p.h
+++ b/src/qml/common/qv4compileddata_p.h
@@ -16,14 +16,15 @@
 
 #include <functional>
 
+#include <QtCore/qhash.h>
 #include <QtCore/qhashfunctions.h>
-#include <QtCore/qstring.h>
+#include <QtCore/qlocale.h>
 #include <QtCore/qscopeguard.h>
-#include <QtCore/qvector.h>
+#include <QtCore/qstring.h>
 #include <QtCore/qstringlist.h>
-#include <QtCore/qhash.h>
+#include <QtCore/qurl.h>
+#include <QtCore/qvector.h>
 #include <QtCore/qversionnumber.h>
-#include <QtCore/qlocale.h>
 
 #if QT_CONFIG(temporaryfile)
 #include <QtCore/qsavefile.h>
@@ -33,6 +34,7 @@
 #include <private/qqmlrefcount_p.h>
 #include <private/qv4staticvalue_p.h>
 #include <private/qv4compilationunitmapper_p.h>
+#include <private/qqmlnullablevalue_p.h>
 
 #include <functional>
 #include <limits.h>
@@ -1558,9 +1560,38 @@ public:
             const QUrl &url, const QDateTime &sourceTimeStamp, QString *errorString);
     Q_QML_EXPORT bool saveToDisk(const QUrl &unitUrl, QString *errorString);
 
+    int importCount() const { return qmlData->nImports; }
+    const CompiledData::Import *importAt(int index) const { return qmlData->importAt(index); }
+
+    Q_QML_EXPORT QStringList moduleRequests() const;
+
+    // url() and fileName() shall be used to load the actual QML/JS code or to show errors or
+    // warnings about that code. They include any potential URL interceptions and thus represent the
+    // "physical" location of the code.
+    //
+    // finalUrl() and finalUrlString() shall be used to resolve further URLs referred to in the code
+    // They are _not_ intercepted and thus represent the "logical" name for the code.
+
+    QUrl url() const
+    {
+        if (!m_url.isValid())
+            m_url = QUrl(fileName());
+        return m_url;
+    }
+
+    QUrl finalUrl() const
+    {
+        if (!m_finalUrl.isValid())
+            m_finalUrl = QUrl(finalUrlString());
+        return m_finalUrl;
+    }
+
 private:
     QString m_fileName; // initialized from data->sourceFileIndex
     QString m_finalUrlString; // initialized from data->finalUrlIndex
+
+    mutable QQmlNullableValue<QUrl> m_url;
+    mutable QQmlNullableValue<QUrl> m_finalUrl;
 };
 
 class SaveableUnitPointer
diff --git a/src/qml/jsruntime/qv4engine.cpp b/src/qml/jsruntime/qv4engine.cpp
index 378306a7c9..6b01e83bdf 100644
--- a/src/qml/jsruntime/qv4engine.cpp
+++ b/src/qml/jsruntime/qv4engine.cpp
@@ -2124,14 +2124,12 @@ void ExecutionEngine::injectCompiledModule(const QQmlRefPointer<ExecutableCompil
 {
     // Injection can happen from the QML type loader thread for example, but instantiation and
     // evaluation must be limited to the ExecutionEngine's thread.
-    QMutexLocker moduleGuard(&moduleMutex);
     modules.insert(moduleUnit->finalUrl(), moduleUnit);
 }
 
 ExecutionEngine::Module ExecutionEngine::moduleForUrl(
         const QUrl &url, const ExecutableCompilationUnit *referrer) const
 {
-    QMutexLocker moduleGuard(&moduleMutex);
     const auto nativeModule = nativeModules.find(url);
     if (nativeModule != nativeModules.end())
         return Module { nullptr, *nativeModule };
@@ -2147,7 +2145,6 @@ ExecutionEngine::Module ExecutionEngine::moduleForUrl(
 
 ExecutionEngine::Module ExecutionEngine::loadModule(const QUrl &url, const ExecutableCompilationUnit *referrer)
 {
-    QMutexLocker moduleGuard(&moduleMutex);
     const auto nativeModule = nativeModules.find(url);
     if (nativeModule != nativeModules.end())
         return Module { nullptr, *nativeModule };
@@ -2159,20 +2156,15 @@ ExecutionEngine::Module ExecutionEngine::loadModule(const QUrl &url, const Execu
     if (existingModule != modules.end())
         return Module { *existingModule, nullptr };
 
-    moduleGuard.unlock();
-
     auto newModule = compileModule(resolved);
-    if (newModule) {
-        moduleGuard.relock();
+    if (newModule)
         modules.insert(resolved, newModule);
-    }
 
     return Module { newModule, nullptr };
 }
 
 QV4::Value *ExecutionEngine::registerNativeModule(const QUrl &url, const QV4::Value &module)
 {
-    QMutexLocker moduleGuard(&moduleMutex);
     const auto existingModule = nativeModules.find(url);
     if (existingModule != nativeModules.end())
         return nullptr;
@@ -2180,6 +2172,11 @@ QV4::Value *ExecutionEngine::registerNativeModule(const QUrl &url, const QV4::Va
     QV4::Value *val = this->memoryManager->m_persistentValues->allocate();
     *val = module.asReturnedValue();
     nativeModules.insert(url, val);
+
+    // Make sure the type loader doesn't try to resolve the script anymore.
+    if (m_qmlEngine)
+        QQmlEnginePrivate::get(m_qmlEngine)->typeLoader.injectScript(url, *val);
+
     return val;
 }
 
diff --git a/src/qml/jsruntime/qv4engine_p.h b/src/qml/jsruntime/qv4engine_p.h
index 8ab3d62311..25484f2773 100644
--- a/src/qml/jsruntime/qv4engine_p.h
+++ b/src/qml/jsruntime/qv4engine_p.h
@@ -862,7 +862,6 @@ private:
 
     QVector<Deletable *> m_extensionData;
 
-    mutable QMutex moduleMutex;
     QHash<QUrl, QQmlRefPointer<ExecutableCompilationUnit>> modules;
 
     // QV4::PersistentValue would be preferred, but using QHash will create copies,
diff --git a/src/qml/jsruntime/qv4executablecompilationunit.cpp b/src/qml/jsruntime/qv4executablecompilationunit.cpp
index ebd0ae3d4a..7f44182a57 100644
--- a/src/qml/jsruntime/qv4executablecompilationunit.cpp
+++ b/src/qml/jsruntime/qv4executablecompilationunit.cpp
@@ -534,17 +534,6 @@ QQmlType ExecutableCompilationUnit::qmlTypeForComponent(const QString &inlineCom
     return inlineComponentData[inlineComponentName].qmlType;
 }
 
-QStringList ExecutableCompilationUnit::moduleRequests() const
-{
-    const CompiledData::Unit *data = m_compilationUnit->data;
-
-    QStringList requests;
-    requests.reserve(data->moduleRequestTableSize);
-    for (uint i = 0; i < data->moduleRequestTableSize; ++i)
-        requests << stringAt(data->moduleRequestTable()[i]);
-    return requests;
-}
-
 Heap::Module *ExecutableCompilationUnit::instantiate()
 {
     const CompiledData::Unit *data = m_compilationUnit->data;
@@ -565,7 +554,8 @@ Heap::Module *ExecutableCompilationUnit::instantiate()
     if (isESModule())
         setModule(module->d());
 
-    for (const QString &request: moduleRequests()) {
+    const QStringList moduleRequests = m_compilationUnit->moduleRequests();
+    for (const QString &request: moduleRequests) {
         const QUrl url(request);
         const auto dependentModuleUnit = engine->loadModule(url, this);
         if (engine->hasException)
@@ -848,7 +838,8 @@ void ExecutableCompilationUnit::evaluateModuleRequests()
 {
     Q_ASSERT(engine);
 
-    for (const QString &request: moduleRequests()) {
+    const QStringList moduleRequests = m_compilationUnit->moduleRequests();
+    for (const QString &request: moduleRequests) {
         auto dependentModule = engine->loadModule(QUrl(request), this);
         if (dependentModule.native)
             continue;
diff --git a/src/qml/jsruntime/qv4executablecompilationunit_p.h b/src/qml/jsruntime/qv4executablecompilationunit_p.h
index 69cac2974c..4f79f12030 100644
--- a/src/qml/jsruntime/qv4executablecompilationunit_p.h
+++ b/src/qml/jsruntime/qv4executablecompilationunit_p.h
@@ -109,29 +109,8 @@ public:
     QString finalUrlString() const { return m_compilationUnit->finalUrlString(); }
     QString fileName() const { return m_compilationUnit->fileName(); }
 
-    // url() and fileName() shall be used to load the actual QML/JS code or to show errors or
-    // warnings about that code. They include any potential URL interceptions and thus represent the
-    // "physical" location of the code.
-    //
-    // finalUrl() and finalUrlString() shall be used to resolve further URLs referred to in the code
-    // They are _not_ intercepted and thus represent the "logical" name for the code.
-
-    QUrl url() const
-    {
-        if (!m_url.isValid())
-            m_url = QUrl(m_compilationUnit->fileName());
-        return m_url;
-    }
-
-    QUrl finalUrl() const
-    {
-        if (!m_finalUrl.isValid())
-            m_finalUrl = QUrl(m_compilationUnit->finalUrlString());
-        return m_finalUrl;
-    }
-
-    mutable QQmlNullableValue<QUrl> m_url;
-    mutable QQmlNullableValue<QUrl> m_finalUrl;
+    QUrl url() const { return m_compilationUnit->url(); }
+    QUrl finalUrl() const { return m_compilationUnit->finalUrl(); }
 
     // QML specific fields
     QQmlPropertyCacheVector propertyCaches;
@@ -243,10 +222,10 @@ public:
         return qmlData()->objectAt(index);
     }
 
-    int importCount() const { return qmlData()->nImports; }
+    int importCount() const { return m_compilationUnit->importCount(); }
     const CompiledData::Import *importAt(int index) const
     {
-        return qmlData()->importAt(index);
+        return m_compilationUnit->importAt(index);
     }
 
     Heap::Object *templateObjectAt(int index) const;
@@ -289,7 +268,6 @@ public:
         return unitData()->flags & CompiledData::Unit::IsSharedLibrary;
     }
 
-    QStringList moduleRequests() const;
     Heap::Module *instantiate();
     const Value *resolveExport(QV4::String *exportName)
     {
diff --git a/src/qml/qml/qqmlscriptblob.cpp b/src/qml/qml/qqmlscriptblob.cpp
index c0117cb24a..89f9eee1cd 100644
--- a/src/qml/qml/qqmlscriptblob.cpp
+++ b/src/qml/qml/qqmlscriptblob.cpp
@@ -32,15 +32,18 @@ QQmlRefPointer<QQmlScriptData> QQmlScriptBlob::scriptData() const
     return m_scriptData;
 }
 
+bool QQmlScriptBlob::isNative() const
+{
+    return m_scriptData && !m_scriptData->m_value.isEmpty();
+}
+
 void QQmlScriptBlob::dataReceived(const SourceCodeData &data)
 {
-    auto *v4 = QQmlEnginePrivate::getV4Engine(typeLoader()->engine());
     if (readCacheFile()) {
         auto unit = QQml::makeRefPointer<QV4::CompiledData::CompilationUnit>();
         QString error;
         if (unit->loadFromDisk(url(), data.sourceTimeStamp(), &error)) {
-            initializeFromCompilationUnit(
-                    QV4::ExecutableCompilationUnit::create(std::move(unit), v4));
+            initializeFromCompilationUnit(std::move(unit));
             return;
         } else {
             qCDebug(DBG_DISK_CACHE()) << "Error loading" << urlString() << "from disk cache:" << error;
@@ -110,15 +113,13 @@ void QQmlScriptBlob::dataReceived(const SourceCodeData &data)
         }
     }
 
-    initializeFromCompilationUnit(QV4::ExecutableCompilationUnit::create(std::move(unit), v4));
+    initializeFromCompilationUnit(std::move(unit));
 }
 
-void QQmlScriptBlob::initializeFromCachedUnit(const QQmlPrivate::CachedQmlUnit *unit)
+void QQmlScriptBlob::initializeFromCachedUnit(const QQmlPrivate::CachedQmlUnit *cachedUnit)
 {
-    initializeFromCompilationUnit(QV4::ExecutableCompilationUnit::create(
-            QQml::makeRefPointer<QV4::CompiledData::CompilationUnit>(
-                    unit->qmlData, unit->aotCompiledFunctions, urlString(), finalUrlString()),
-            QQmlEnginePrivate::getV4Engine(typeLoader()->engine())));
+    initializeFromCompilationUnit(QQml::makeRefPointer<QV4::CompiledData::CompilationUnit>(
+            cachedUnit->qmlData, cachedUnit->aotCompiledFunctions, urlString(), finalUrlString()));
 }
 
 void QQmlScriptBlob::done()
@@ -167,6 +168,14 @@ void QQmlScriptBlob::done()
     m_scripts.clear();
 }
 
+void QQmlScriptBlob::completed()
+{
+    if (m_scriptData && m_scriptData->m_precompiledScript) {
+        QQmlEnginePrivate::getV4Engine(typeLoader()->engine())
+                ->injectCompiledModule(m_scriptData->m_precompiledScript);
+    }
+}
+
 QString QQmlScriptBlob::stringAt(int index) const
 {
     return m_scriptData->m_precompiledScript->stringAt(index);
@@ -183,17 +192,22 @@ void QQmlScriptBlob::scriptImported(const QQmlRefPointer<QQmlScriptBlob> &blob,
     m_scripts << ref;
 }
 
-void QQmlScriptBlob::initializeFromCompilationUnit(const QQmlRefPointer<QV4::ExecutableCompilationUnit> &unit)
+void QQmlScriptBlob::initializeFromCompilationUnit(
+        QQmlRefPointer<QV4::CompiledData::CompilationUnit> &&unit)
 {
     Q_ASSERT(!m_scriptData);
+    Q_ASSERT(unit);
+
     m_scriptData.adopt(new QQmlScriptData());
     m_scriptData->url = finalUrl();
     m_scriptData->urlString = finalUrlString();
-    m_scriptData->m_precompiledScript = unit;
+    m_scriptData->m_precompiledScript = QV4::ExecutableCompilationUnit::create(
+            std::move(unit), QQmlEnginePrivate::getV4Engine(typeLoader()->engine()));
 
     m_importCache->setBaseUrl(finalUrl(), finalUrlString());
 
-    QQmlRefPointer<QV4::ExecutableCompilationUnit> script = m_scriptData->m_precompiledScript;
+    QQmlRefPointer<QV4::CompiledData::CompilationUnit> script
+            = m_scriptData->m_precompiledScript->baseCompilationUnit();
 
     if (!m_isModule) {
         QList<QQmlError> errors;
@@ -212,22 +226,25 @@ void QQmlScriptBlob::initializeFromCompilationUnit(const QQmlRefPointer<QV4::Exe
         }
     }
 
-    auto *v4 = QQmlEnginePrivate::getV4Engine(typeLoader()->engine());
-
-    v4->injectCompiledModule(unit);
+    const QStringList moduleRequests = script->moduleRequests();
+    for (const QString &request: moduleRequests) {
+        const QUrl relativeRequest = QUrl(request);
+        if (m_typeLoader->injectedScript(relativeRequest))
+            continue;
 
-    for (const QString &request: unit->moduleRequests()) {
-        const auto module = v4->moduleForUrl(QUrl(request), unit.data());
-        if (module.compiled || module.native)
+        const QUrl absoluteRequest = script->finalUrl().resolved(relativeRequest);
+        QQmlRefPointer<QQmlScriptBlob> absoluteBlob = typeLoader()->getScript(absoluteRequest);
+        if (absoluteBlob->m_scriptData && absoluteBlob->m_scriptData->m_precompiledScript)
             continue;
 
-        const QUrl absoluteRequest = unit->finalUrl().resolved(QUrl(request));
-        QQmlRefPointer<QQmlScriptBlob> blob = typeLoader()->getScript(absoluteRequest);
-        addDependency(blob.data());
-        scriptImported(blob, /* ### */QV4::CompiledData::Location(), /*qualifier*/QString(), /*namespace*/QString());
+        addDependency(absoluteBlob.data());
+        scriptImported(
+                absoluteBlob, /* ### */QV4::CompiledData::Location(), /*qualifier*/QString(),
+                /*namespace*/QString());
     }
 }
 
+
 /*!
     \internal
 
diff --git a/src/qml/qml/qqmlscriptblob_p.h b/src/qml/qml/qqmlscriptblob_p.h
index ad9c573400..356fe23675 100644
--- a/src/qml/qml/qqmlscriptblob_p.h
+++ b/src/qml/qml/qqmlscriptblob_p.h
@@ -39,17 +39,19 @@ public:
     };
 
     QQmlRefPointer<QQmlScriptData> scriptData() const;
+    bool isNative() const;
 
 protected:
     void dataReceived(const SourceCodeData &) override;
     void initializeFromCachedUnit(const QQmlPrivate::CachedQmlUnit *unit) override;
     void done() override;
+    void completed() override;
 
     QString stringAt(int index) const override;
 
 private:
     void scriptImported(const QQmlRefPointer<QQmlScriptBlob> &blob, const QV4::CompiledData::Location &location, const QString &qualifier, const QString &nameSpace) override;
-    void initializeFromCompilationUnit(const QQmlRefPointer<QV4::ExecutableCompilationUnit> &unit);
+    void initializeFromCompilationUnit(QQmlRefPointer<QV4::CompiledData::CompilationUnit> &&cu);
     void initializeFromNative(const QV4::Value &value);
 
     QList<ScriptReference> m_scripts;
diff --git a/src/qml/qml/qqmltypeloader.cpp b/src/qml/qml/qqmltypeloader.cpp
index 4f84fa6be4..b76fcd9409 100644
--- a/src/qml/qml/qqmltypeloader.cpp
+++ b/src/qml/qml/qqmltypeloader.cpp
@@ -554,15 +554,12 @@ bool QQmlTypeLoader::Blob::updateQmldir(const QQmlRefPointer<QQmlQmldirData> &da
 bool QQmlTypeLoader::Blob::addScriptImport(const QQmlTypeLoader::Blob::PendingImportPtr &import)
 {
     const QUrl url(import->uri);
-    const auto module = m_typeLoader->engine()->handle()->moduleForUrl(url);
-    QQmlRefPointer<QQmlScriptBlob> blob;
-    if (module.native) {
-        blob.adopt(new QQmlScriptBlob(url, m_typeLoader));
-        blob->initializeFromNative(*module.native);
-        blob->tryDone();
-    } else {
-        blob = typeLoader()->getScript(finalUrl().resolved(url));
-    }
+    QQmlTypeLoader *loader = typeLoader();
+    QQmlRefPointer<QQmlScriptBlob> blob = loader->injectedScript(url);
+    if (!blob)
+        blob = loader->getScript(finalUrl().resolved(url));
+    else
+        Q_ASSERT(blob->status() == QQmlDataBlob::Status::Complete);
     addDependency(blob.data());
     scriptImported(blob, import->location, import->qualifier, QString());
     return true;
@@ -998,6 +995,26 @@ QQmlRefPointer<QQmlTypeData> QQmlTypeLoader::getType(const QByteArray &data, con
     return QQmlRefPointer<QQmlTypeData>(typeData, QQmlRefPointer<QQmlTypeData>::Adopt);
 }
 
+void QQmlTypeLoader::injectScript(const QUrl &relativeUrl, const QV4::Value &value)
+{
+    LockHolder<QQmlTypeLoader> holder(this);
+
+    QQmlScriptBlob *blob = new QQmlScriptBlob(relativeUrl, this);
+    blob->initializeFromNative(value);
+    blob->m_isDone = true;
+    blob->m_data.setStatus(QQmlDataBlob::Complete);
+    m_scriptCache.insert(relativeUrl, blob);
+}
+
+QQmlRefPointer<QQmlScriptBlob> QQmlTypeLoader::injectedScript(const QUrl &relativeUrl)
+{
+    LockHolder<QQmlTypeLoader> holder(this);
+    const auto it = m_scriptCache.constFind(relativeUrl);
+    return (it != m_scriptCache.constEnd() && (*it)->isNative())
+            ? *it
+            : QQmlRefPointer<QQmlScriptBlob>();
+}
+
 /*!
 Return a QQmlScriptBlob for \a url.  The QQmlScriptData may be cached.
 */
diff --git a/src/qml/qml/qqmltypeloader_p.h b/src/qml/qml/qqmltypeloader_p.h
index 6c53437d86..7f6e43bcfb 100644
--- a/src/qml/qml/qqmltypeloader_p.h
+++ b/src/qml/qml/qqmltypeloader_p.h
@@ -131,6 +131,9 @@ public:
     QQmlRefPointer<QQmlTypeData> getType(const QUrl &unNormalizedUrl, Mode mode = PreferSynchronous);
     QQmlRefPointer<QQmlTypeData> getType(const QByteArray &, const QUrl &url, Mode mode = PreferSynchronous);
 
+    void injectScript(const QUrl &relativeUrl, const QV4::Value &value);
+    QQmlRefPointer<QQmlScriptBlob> injectedScript(const QUrl &relativeUrl);
+
     QQmlRefPointer<QQmlScriptBlob> getScript(const QUrl &unNormalizedUrl);
     QQmlRefPointer<QQmlQmldirData> getQmldir(const QUrl &);
author	Ulf Hermann <ulf.hermann@qt.io>	2023-12-22 14:06:34 +0100
committer	Ulf Hermann <ulf.hermann@qt.io>	2024-01-10 11:22:43 +0100
commit	47ddbb72e052989622f5fa23e032c787bed5f2e5 (patch)
tree	6c18a31045527b7b43be331651607ac34908639a /src
parent	be6d1499af75228341227d15441284e07cfe1e41 (diff)