QtQml: Decouple JavaScript library CUs from engines

In order to re-use the compilation units for JavaScript libraries, we need to eliminate the "m_value" member they are carrying, indirectly. The values are tied to specific engines and invalid in others. Luckily, we already have two suitable places to store such values: 1. In case of a "native" module without a compilation unit we have the nativeModules hash in ExecutionEngine. 2. In case of a module or library backed by a CU we have the "module" member of ExecutableCompilationUnit. This can currently only hold modules but there is no reason why it wouldn't hold JavaScript libraries equally well. By using the "empty" V4 value we can also get rid of the m_loaded bool. As a drive by, correct the QQmlScriptBlob::isNative() misnomer. We don't want to know whether the script is native (for any value of that), but rather whether it has a value. Pick-to: 6.8 Fixes: QTBUG-129052 Change-Id: I284823f4aa26e46dec8328f88f65877f59e40f79 Reviewed-by: Fabian Kosmale <fabian.kosmale@qt.io>
author: Ulf Hermann <ulf.hermann@qt.io> 2024-09-18 10:14:13 +0200
committer: Ulf Hermann <ulf.hermann@qt.io> 2024-09-20 18:55:30 +0200
commit: e1a0eb98e60588862d79fa074b65217a96bdaab5 (patch)
tree: aafe7ef524916441b7a094523d68a8ebdc7d47b4 /src
parent: 9855d7477163cc56742a1766d35346d8058f2794 (diff)
10 files changed, 105 insertions, 56 deletions
diff --git a/src/qml/jsruntime/qv4engine.cpp b/src/qml/jsruntime/qv4engine.cpp
index cd1a4e033c..b392ba4807 100644
--- a/src/qml/jsruntime/qv4engine.cpp
+++ b/src/qml/jsruntime/qv4engine.cpp
@@ -2162,7 +2162,7 @@ QV4::Value *ExecutionEngine::registerNativeModule(const QUrl &url, const QV4::Va
 
     // Make sure the type loader doesn't try to resolve the script anymore.
     if (m_qmlEngine)
-        QQmlEnginePrivate::get(m_qmlEngine)->typeLoader.injectScript(url, *val);
+        QQmlEnginePrivate::get(m_qmlEngine)->typeLoader.injectScript(url);
 
     return val;
 }
diff --git a/src/qml/jsruntime/qv4engine_p.h b/src/qml/jsruntime/qv4engine_p.h
index 6b262cede6..a9f58127e5 100644
--- a/src/qml/jsruntime/qv4engine_p.h
+++ b/src/qml/jsruntime/qv4engine_p.h
@@ -814,6 +814,14 @@ public:
         return ok ? QJSPrimitiveValue(result) : QJSPrimitiveValue(QJSPrimitiveUndefined());
     }
 
+    ReturnedValue nativeModule(const QUrl &url) const
+    {
+        const auto it = nativeModules.find(url);
+        return it == nativeModules.end()
+                ? QV4::Value::emptyValue().asReturnedValue()
+                : (*it)->asReturnedValue();
+    }
+
 private:
     template<int Frames>
     friend struct ExecutionEngineCallDepthRecorder;
diff --git a/src/qml/jsruntime/qv4executablecompilationunit.cpp b/src/qml/jsruntime/qv4executablecompilationunit.cpp
index 5e9361a066..bb05d623e3 100644
--- a/src/qml/jsruntime/qv4executablecompilationunit.cpp
+++ b/src/qml/jsruntime/qv4executablecompilationunit.cpp
@@ -299,8 +299,8 @@ void ExecutableCompilationUnit::markObjects(QV4::MarkStack *markStack) const
             runtimeLookups[i].markObjects(markStack);
     }
 
-    if (auto mod = module())
-        mod->mark(markStack);
+    if (Heap::Base *v = m_valueOrModule.heapObject())
+        v->mark(markStack);
 }
 
 IdentifierHash ExecutableCompilationUnit::createNamedObjectsPerComponent(int componentObjectIndex)
@@ -705,6 +705,18 @@ QString ExecutableCompilationUnit::translateFrom(TranslationDataIndex index) con
 #endif
 }
 
+Heap::Module *ExecutableCompilationUnit::module() const
+{
+    if (const Module *m = m_valueOrModule.as<QV4::Module>())
+        return m->d();
+    return nullptr;
+}
+
+void ExecutableCompilationUnit::setModule(Heap::Module *module)
+{
+    m_valueOrModule = module;
+}
+
 } // namespace QV4
 
 QT_END_NAMESPACE
diff --git a/src/qml/jsruntime/qv4executablecompilationunit_p.h b/src/qml/jsruntime/qv4executablecompilationunit_p.h
index 930e138732..b7a52523fb 100644
--- a/src/qml/jsruntime/qv4executablecompilationunit_p.h
+++ b/src/qml/jsruntime/qv4executablecompilationunit_p.h
@@ -188,8 +188,11 @@ public:
 
     QString translateFrom(TranslationDataIndex index) const;
 
-    Heap::Module *module() const { return m_module; }
-    void setModule(Heap::Module *module) { m_module = module; }
+    Heap::Module *module() const;
+    void setModule(Heap::Module *module);
+
+    ReturnedValue value() const { return m_valueOrModule.asReturnedValue(); }
+    void setValue(const QV4::Value &value) { m_valueOrModule = value; }
 
     const CompiledData::Unit *unitData() const { return m_compilationUnit->data; }
 
@@ -233,7 +236,7 @@ private:
     friend struct ExecutionEngine;
 
     QQmlRefPointer<CompiledData::CompilationUnit> m_compilationUnit;
-    Heap::Module *m_module = nullptr;
+    Value m_valueOrModule = QV4::Value::emptyValue();
 
     struct ResolveSetEntry
     {
diff --git a/src/qml/qml/qqmlscriptblob.cpp b/src/qml/qml/qqmlscriptblob.cpp
index 5f29cd25a9..2932b9289e 100644
--- a/src/qml/qml/qqmlscriptblob.cpp
+++ b/src/qml/qml/qqmlscriptblob.cpp
@@ -31,9 +31,16 @@ QQmlRefPointer<QQmlScriptData> QQmlScriptBlob::scriptData() const
     return m_scriptData;
 }
 
-bool QQmlScriptBlob::isNative() const
+bool QQmlScriptBlob::hasScriptValue() const
 {
-    return m_scriptData && !m_scriptData->m_value.isEmpty();
+    if (!m_scriptData)
+        return false;
+
+    QV4::ExecutionEngine *v4 = m_typeLoader->engine()->handle();
+    Q_ASSERT(v4);
+    QV4::Scope scope(v4);
+    QV4::ScopedValue value(scope, m_scriptData->ownScriptValue(v4));
+    return !value->isEmpty();
 }
 
 void QQmlScriptBlob::dataReceived(const SourceCodeData &data)
@@ -240,14 +247,12 @@ void QQmlScriptBlob::initializeFromCompilationUnit(
 
     \sa QJSEngine::registerModule()
  */
-void QQmlScriptBlob::initializeFromNative(const QV4::Value &value)
+void QQmlScriptBlob::initializeFromNative()
 {
     Q_ASSERT(!m_scriptData);
     m_scriptData.adopt(new QQmlScriptData());
     m_scriptData->url = finalUrl();
     m_scriptData->urlString = finalUrlString();
-    m_scriptData->m_loaded = true;
-    m_scriptData->m_value.set(QQmlEnginePrivate::getV4Engine(typeLoader()->engine()), value);
     m_importCache->setBaseUrl(finalUrl(), finalUrlString());
 }
 
diff --git a/src/qml/qml/qqmlscriptblob_p.h b/src/qml/qml/qqmlscriptblob_p.h
index 9abc843de3..e87dcd8129 100644
--- a/src/qml/qml/qqmlscriptblob_p.h
+++ b/src/qml/qml/qqmlscriptblob_p.h
@@ -40,7 +40,7 @@ public:
     };
 
     QQmlRefPointer<QQmlScriptData> scriptData() const;
-    bool isNative() const;
+    bool hasScriptValue() const;
 
 protected:
     void dataReceived(const SourceCodeData &) override;
@@ -52,7 +52,7 @@ protected:
 private:
     void scriptImported(const QQmlRefPointer<QQmlScriptBlob> &blob, const QV4::CompiledData::Location &location, const QString &qualifier, const QString &nameSpace) override;
     void initializeFromCompilationUnit(QQmlRefPointer<QV4::CompiledData::CompilationUnit> &&cu);
-    void initializeFromNative(const QV4::Value &value);
+    void initializeFromNative();
 
     QList<ScriptReference> m_scripts;
     QQmlRefPointer<QQmlScriptData> m_scriptData;
diff --git a/src/qml/qml/qqmlscriptdata.cpp b/src/qml/qml/qqmlscriptdata.cpp
index 9337f25c6e..f26b7610a6 100644
--- a/src/qml/qml/qqmlscriptdata.cpp
+++ b/src/qml/qml/qqmlscriptdata.cpp
@@ -65,54 +65,57 @@ QQmlRefPointer<QQmlContextData> QQmlScriptData::qmlContextDataForContext(
     return qmlContextData;
 }
 
+QV4::ReturnedValue QQmlScriptData::ownScriptValue(QV4::ExecutionEngine *v4) const
+{
+    return handleOwnScriptValueOrExecutableCU(
+            v4, [](const QQmlRefPointer<QV4::ExecutableCompilationUnit> &executableCU) {
+        return executableCU->value();
+    });
+}
+
 QV4::ReturnedValue QQmlScriptData::scriptValueForContext(
         const QQmlRefPointer<QQmlContextData> &parentQmlContextData)
 {
-    if (m_loaded)
-        return m_value.value();
-
-    Q_ASSERT(parentQmlContextData && parentQmlContextData->engine());
     QV4::ExecutionEngine *v4 = parentQmlContextData->engine()->handle();
-    QV4::Scope scope(v4);
-
-    QV4::Scoped<QV4::QmlContext> qmlExecutionContext(scope);
-    if (auto qmlContextData = qmlContextDataForContext(parentQmlContextData)) {
-        qmlExecutionContext = QV4::QmlContext::create(v4->rootContext(), std::move(qmlContextData),
-                                                      /* scopeObject: */ nullptr);
-    }
+    return handleOwnScriptValueOrExecutableCU(
+            v4, [&](const QQmlRefPointer<QV4::ExecutableCompilationUnit> &executableCU) {
+        QV4::Scope scope(v4);
+        QV4::ScopedValue value(scope, executableCU->value());
+        if (!value->isEmpty())
+            return value->asReturnedValue();
+
+        QV4::Scoped<QV4::QmlContext> qmlExecutionContext(scope);
+        if (auto qmlContextData = qmlContextDataForContext(parentQmlContextData)) {
+            qmlExecutionContext = QV4::QmlContext::create(
+                    v4->rootContext(), std::move(qmlContextData), /* scopeObject: */ nullptr);
+        }
 
-    QV4::Scoped<QV4::Module> module(
-            scope,
-            v4->executableCompilationUnit(QQmlRefPointer<QV4::CompiledData::CompilationUnit>(
-                                                  m_precompiledScript))->instantiate());
+        QV4::Scoped<QV4::Module> module(scope, executableCU->instantiate());
+        if (module) {
+            if (qmlExecutionContext) {
+                module->d()->scope->outer.set(v4, qmlExecutionContext->d());
+                qmlExecutionContext->d()->qml()->module.set(v4, module->d());
+            }
 
-    if (module) {
-        if (qmlExecutionContext) {
-            module->d()->scope->outer.set(v4, qmlExecutionContext->d());
-            qmlExecutionContext->d()->qml()->module.set(v4, module->d());
+            module->evaluate();
         }
 
-        module->evaluate();
-    }
-
-    if (v4->hasException) {
-        QQmlError error = v4->catchExceptionAsQmlError();
-        if (error.isValid())
-            QQmlEnginePrivate::get(v4)->warning(error);
-    }
+        if (v4->hasException) {
+            QQmlError error = v4->catchExceptionAsQmlError();
+            if (error.isValid())
+                QQmlEnginePrivate::get(v4)->warning(error);
+        }
 
-    QV4::ScopedValue value(scope);
-    if (qmlExecutionContext)
-        value = qmlExecutionContext->d()->qml();
-    else if (module)
-        value = module->d();
+        if (qmlExecutionContext)
+            value = qmlExecutionContext->d()->qml();
+        else if (module)
+            value = module->d();
 
-    if (m_precompiledScript->isSharedLibrary() || m_precompiledScript->isESModule()) {
-        m_loaded = true;
-        m_value.set(v4, value);
-    }
+        if (m_precompiledScript->isSharedLibrary() || m_precompiledScript->isESModule())
+            executableCU->setValue(value);
 
-    return value->asReturnedValue();
+        return value->asReturnedValue();
+    });
 }
 
 QT_END_NAMESPACE
diff --git a/src/qml/qml/qqmlscriptdata_p.h b/src/qml/qml/qqmlscriptdata_p.h
index ad5ffb3d07..1361c31608 100644
--- a/src/qml/qml/qqmlscriptdata_p.h
+++ b/src/qml/qml/qqmlscriptdata_p.h
@@ -20,6 +20,7 @@
 #include <private/qv4value_p.h>
 #include <private/qv4persistent_p.h>
 #include <private/qv4compileddata_p.h>
+#include <private/qv4scopedvalue_p.h>
 
 #include <QtCore/qurl.h>
 
@@ -41,6 +42,7 @@ public:
     QQmlRefPointer<QQmlTypeNameCache> typeNameCache;
     QVector<QQmlRefPointer<QQmlScriptBlob>> scripts;
 
+    QV4::ReturnedValue ownScriptValue(QV4::ExecutionEngine *v4) const;
     QV4::ReturnedValue scriptValueForContext(const QQmlRefPointer<QQmlContextData> &parentCtxt);
 
     QQmlRefPointer<QV4::CompiledData::CompilationUnit> compilationUnit() const
@@ -54,9 +56,25 @@ private:
     QQmlRefPointer<QQmlContextData> qmlContextDataForContext(
             const QQmlRefPointer<QQmlContextData> &parentQmlContextData);
 
-    bool m_loaded = false;
+    template<typename WithExecutableCU>
+    QV4::ReturnedValue handleOwnScriptValueOrExecutableCU(
+            QV4::ExecutionEngine *v4,
+            WithExecutableCU &&withExecutableCU) const
+    {
+        QV4::Scope scope(v4);
+
+        QV4::ScopedValue value(scope, v4->nativeModule(url));
+        if (!value->isEmpty())
+            return value->asReturnedValue();
+
+        if (!m_precompiledScript)
+            return QV4::Value::emptyValue().asReturnedValue();
+
+        return withExecutableCU(v4->executableCompilationUnit(
+                QQmlRefPointer<QV4::CompiledData::CompilationUnit>(m_precompiledScript)));
+    }
+
     QQmlRefPointer<QV4::CompiledData::CompilationUnit> m_precompiledScript;
-    QV4::PersistentValue m_value;
 };
 
 QT_END_NAMESPACE
diff --git a/src/qml/qml/qqmltypeloader.cpp b/src/qml/qml/qqmltypeloader.cpp
index a74397bd93..a673debd0e 100644
--- a/src/qml/qml/qqmltypeloader.cpp
+++ b/src/qml/qml/qqmltypeloader.cpp
@@ -1006,12 +1006,12 @@ 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)
+void QQmlTypeLoader::injectScript(const QUrl &relativeUrl)
 {
     LockHolder<QQmlTypeLoader> holder(this);
 
     QQmlScriptBlob *blob = new QQmlScriptBlob(relativeUrl, this);
-    blob->initializeFromNative(value);
+    blob->initializeFromNative();
     blob->m_isDone = true;
     blob->m_data.setStatus(QQmlDataBlob::Complete);
     m_scriptCache.insert(relativeUrl, blob);
@@ -1021,7 +1021,7 @@ QQmlRefPointer<QQmlScriptBlob> QQmlTypeLoader::injectedScript(const QUrl &relati
 {
     LockHolder<QQmlTypeLoader> holder(this);
     const auto it = m_scriptCache.constFind(relativeUrl);
-    return (it != m_scriptCache.constEnd() && (*it)->isNative())
+    return (it != m_scriptCache.constEnd() && (*it)->hasScriptValue())
             ? *it
             : QQmlRefPointer<QQmlScriptBlob>();
 }
diff --git a/src/qml/qml/qqmltypeloader_p.h b/src/qml/qml/qqmltypeloader_p.h
index e9c4559527..f9adb9aa62 100644
--- a/src/qml/qml/qqmltypeloader_p.h
+++ b/src/qml/qml/qqmltypeloader_p.h
@@ -149,7 +149,7 @@ 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);
+    void injectScript(const QUrl &relativeUrl);
     QQmlRefPointer<QQmlScriptBlob> injectedScript(const QUrl &relativeUrl);
 
     QQmlRefPointer<QQmlScriptBlob> getScript(const QUrl &unNormalizedUrl);
author	Ulf Hermann <ulf.hermann@qt.io>	2024-09-18 10:14:13 +0200
committer	Ulf Hermann <ulf.hermann@qt.io>	2024-09-20 18:55:30 +0200
commit	e1a0eb98e60588862d79fa074b65217a96bdaab5 (patch)
tree	aafe7ef524916441b7a094523d68a8ebdc7d47b4 /src
parent	9855d7477163cc56742a1766d35346d8058f2794 (diff)