From 131db085a752469e8f19974c2edb3a138d900249 Mon Sep 17 00:00:00 2001 From: Fabian Kosmale Date: Mon, 13 Feb 2023 12:30:46 +0100 Subject: [PATCH] QML: Don't unconditionally invalidate cache if inline component is used With inline components, we might have the case that an outer components depends on an inline component defined in the same file. In that case, the ResolvedType of the inline component has some very specific characteristics we can recognize when adding to the hash. We don't actually have to add such inline components to the hash since they are just part of the same file. If they change, the file will change, leading to a different timestamp, which is caught earlier. Pick-to: 6.5 6.5.0 6.2 Fixes: QTBUG-111042 Change-Id: I8ae716e55dd783cc8409039bd7baffe051df2b96 Reviewed-by: Fabian Kosmale --- .../jsruntime/qv4resolvedtypereference.cpp | 13 +++- src/qml/qml/qqmltypedata.cpp | 76 ++++++++----------- src/qml/qml/qqmltypedata_p.h | 2 - .../qml/qmldiskcache/tst_qmldiskcache.cpp | 30 ++++++++ 4 files changed, 74 insertions(+), 47 deletions(-) diff --git a/src/qml/jsruntime/qv4resolvedtypereference.cpp b/src/qml/jsruntime/qv4resolvedtypereference.cpp index 0bf04c941a..6eab05fe51 100644 --- a/src/qml/jsruntime/qv4resolvedtypereference.cpp +++ b/src/qml/jsruntime/qv4resolvedtypereference.cpp @@ -67,7 +67,18 @@ QQmlPropertyCache::ConstPtr ResolvedTypeReference::createPropertyCache() bool ResolvedTypeReference::addToHash( QCryptographicHash *hash, QHash *checksums) { - if (m_type.isValid() && !m_type.isInlineComponentType()) { + if (m_type.isInlineComponentType()) { + + // A reference to an inline component in the same file will have + // - no compilation unit since we cannot resolve the compilation unit before it's built. + // - a property cache since we've assigned one in buildMetaObjectsIncrementally(). + // - a QQmlType that says it's an inline component. + // We don't have to add such a thing to the hash since if it changes, the QML document + // itself changes, leading to a new timestamp, which is checked before the checksum. + if (!m_compilationUnit) + return !m_typePropertyCache.isNull(); + + } else if (m_type.isValid()) { bool ok = false; hash->addData(createPropertyCache()->checksum(checksums, &ok)); return ok; diff --git a/src/qml/qml/qqmltypedata.cpp b/src/qml/qml/qqmltypedata.cpp index 460a3440c7..ddfbccbddf 100644 --- a/src/qml/qml/qqmltypedata.cpp +++ b/src/qml/qml/qqmltypedata.cpp @@ -60,24 +60,6 @@ QV4::ExecutableCompilationUnit *QQmlTypeData::compilationUnit() const return m_compiledData.data(); } -QV4::ExecutableCompilationUnit *QQmlTypeData::compilationUnitForInlineComponent(unsigned int icObjectId) const -{ - Q_ASSERT(m_document || m_compiledData); - if (m_compiledData) - return m_compiledData.data(); - for (auto it = m_document->objects.begin(); it != m_document->objects.end(); ++it) { - auto object = *it; - auto icIt = std::find_if(object->inlineComponentsBegin(), object->inlineComponentsEnd(), [&](const QV4::CompiledData::InlineComponent &ic) { - return ic.objectIndex == icObjectId; - }); - if (icIt != object->inlineComponentsEnd()) { - Q_ASSERT(m_inlineComponentToCompiledData.contains(icIt->nameIndex)); - return m_inlineComponentToCompiledData[icIt->nameIndex].data(); - } - } - Q_UNREACHABLE_RETURN(nullptr); -} - void QQmlTypeData::registerCallback(TypeDataCallback *callback) { Q_ASSERT(!m_callbacks.contains(callback)); @@ -198,6 +180,7 @@ void QQmlTypeData::createTypeAndPropertyCaches( Q_ASSERT(m_compiledData); m_compiledData->typeNameCache = typeNameCache; m_compiledData->resolvedTypes = resolvedTypeCache; + m_compiledData->inlineComponentData = m_inlineComponentData; QQmlEnginePrivate * const engine = QQmlEnginePrivate::get(typeLoader()->engine()); @@ -386,27 +369,38 @@ void QQmlTypeData::done() }; // verify if any dependencies changed if we're using a cache - if (m_document.isNull() && !m_compiledData->verifyChecksum(dependencyHasher)) { - qCDebug(DBG_DISK_CACHE) << "Checksum mismatch for cached version of" << m_compiledData->fileName(); - if (!loadFromSource()) + if (m_document.isNull()) { + createTypeAndPropertyCaches(typeNameCache, resolvedTypeCache); + if (isError()) { return; - m_compiledData.reset(); + } + + if (m_compiledData->verifyChecksum(dependencyHasher)) { + setCompileUnit(m_compiledData); + } else { + qCDebug(DBG_DISK_CACHE) << "Checksum mismatch for cached version of" << m_compiledData->fileName(); + if (!loadFromSource()) + return; + + // We want to keep our resolve types ... + m_compiledData->resolvedTypes.clear(); + // ... but we don't want their property caches. + for (QV4::ResolvedTypeReference *ref: std::as_const(resolvedTypeCache)) + ref->setTypePropertyCache(QQmlPropertyCache::ConstPtr()); + + m_compiledData.reset(); + } } if (!m_document.isNull()) { // Compile component compile(typeNameCache, &resolvedTypeCache, dependencyHasher); - if (!isError()) + if (isError()) + return; + else setCompileUnit(m_document); - } else { - createTypeAndPropertyCaches(typeNameCache, resolvedTypeCache); - if (!isError()) - setCompileUnit(m_compiledData); } - if (isError()) - return; - { QQmlEnginePrivate *const enginePrivate = QQmlEnginePrivate::get(typeLoader()->engine()); m_compiledData->inlineComponentData = m_inlineComponentData; @@ -903,22 +897,16 @@ QQmlError QQmlTypeData::buildTypeResolutionCaches( } } else if (resolvedType->type.isInlineComponentType()) { // Inline component, defined in the file we are currently compiling - if (!m_inlineComponentToCompiledData.contains(resolvedType.key())) { - ref->setType(qmlType); - if (qmlType.isValid()) { - // this is required for inline components in singletons - auto type = qmlType.lookupInlineComponentById(qmlType.inlineComponentId()).typeId(); - auto exUnit = QQmlMetaType::obtainExecutableCompilationUnit(type); - if (exUnit) { - ref->setCompilationUnit(exUnit); - ref->setTypePropertyCache(QQmlMetaType::propertyCacheForType(type)); - } + ref->setType(qmlType); + if (qmlType.isValid()) { + // this is required for inline components in singletons + auto type = qmlType.lookupInlineComponentById(qmlType.inlineComponentId()).typeId(); + auto exUnit = QQmlMetaType::obtainExecutableCompilationUnit(type); + if (exUnit) { + ref->setCompilationUnit(exUnit); + ref->setTypePropertyCache(QQmlMetaType::propertyCacheForType(type)); } - } else { - ref->setCompilationUnit(m_inlineComponentToCompiledData[resolvedType.key()]); - ref->setTypePropertyCache(m_inlineComponentToCompiledData[resolvedType.key()]->rootPropertyCache()); } - } else if (qmlType.isValid() && !resolvedType->selfReference) { ref->setType(qmlType); Q_ASSERT(ref->type().isValid()); diff --git a/src/qml/qml/qqmltypedata_p.h b/src/qml/qml/qqmltypedata_p.h index d5398b0b5f..eb39e99348 100644 --- a/src/qml/qml/qqmltypedata_p.h +++ b/src/qml/qml/qqmltypedata_p.h @@ -58,7 +58,6 @@ public: const QList &resolvedScripts() const; QV4::ExecutableCompilationUnit *compilationUnit() const; - QV4::ExecutableCompilationUnit *compilationUnitForInlineComponent(unsigned int icObjectId) const; // Used by QQmlComponent to get notifications struct TypeDataCallback { @@ -130,7 +129,6 @@ private: QHash m_inlineComponentData; ExecutableCompilationUnitPtr m_compiledData; - QHash m_inlineComponentToCompiledData; QList m_callbacks; diff --git a/tests/auto/qml/qmldiskcache/tst_qmldiskcache.cpp b/tests/auto/qml/qmldiskcache/tst_qmldiskcache.cpp index 03bc281617..cec24a5fc5 100644 --- a/tests/auto/qml/qmldiskcache/tst_qmldiskcache.cpp +++ b/tests/auto/qml/qmldiskcache/tst_qmldiskcache.cpp @@ -41,6 +41,7 @@ private slots: void cacheModuleScripts(); void reuseStaticMappings(); void invalidateSaveLoadCache(); + void inlineComponentDoesNotCauseConstantInvalidation(); private: QDir m_qmlCacheDirectory; @@ -1117,6 +1118,35 @@ void tst_qmldiskcache::invalidateSaveLoadCache() QVERIFY(unit->unitData() != oldUnit->unitData()); } +void tst_qmldiskcache::inlineComponentDoesNotCauseConstantInvalidation() +{ + QQmlEngine engine; + + TestCompiler testCompiler(&engine); + QVERIFY(testCompiler.tempDir.isValid()); + + testCompiler.reset(); + QVERIFY(testCompiler.writeTestFile("import QtQml\nQtObject { component Test : QtObject { property int i: 2 }\n property Test test: Test { objectName: 'foobar' } }\n")); + QVERIFY(testCompiler.loadTestFile()); + + const quintptr data1 = testCompiler.unitData(); + QVERIFY(data1 != 0); + QCOMPARE(testCompiler.unitData(), data1); + + engine.clearComponentCache(); + + // inline component does not invalidate cache + QVERIFY(testCompiler.loadTestFile()); + QCOMPARE(testCompiler.unitData(), data1); + + testCompiler.reset(); + QVERIFY(testCompiler.writeTestFile("import QtQml\nQtObject { component Test : QtObject { property double d: 2 }\n property Test test: Test { objectName: 'foobar' } }\n")); + QVERIFY(testCompiler.loadTestFile()); + const quintptr data2 = testCompiler.unitData(); + QVERIFY(data2); + QVERIFY(data1 != data2); +} + QTEST_MAIN(tst_qmldiskcache) #include "tst_qmldiskcache.moc"