From d2c6ac8d5eaa1d4b53b647f01c53d821bbba4665 Mon Sep 17 00:00:00 2001 From: Simon Hausmann Date: Fri, 12 Jul 2019 16:44:03 +0200 Subject: Clean up import resolution handling Instead of storing raw pointers into the compiled data and creating synthetic CompiledData::Import objects for implicit imports, this patch introduces a dedicated PendingImport data structure that removes the need of using raw pointers, reduces the amount of string index to string resolution and incorporates the priority field thus eliminating the need for a hash table. Change-Id: Ia59aad17687bc0935aafd36236bda85d5f4f82a2 Reviewed-by: Fabian Kosmale Reviewed-by: Ulf Hermann --- src/qml/qml/qqmldirdata.cpp | 7 ++-- src/qml/qml/qqmldirdata_p.h | 6 ++-- src/qml/qml/qqmltypedata.cpp | 21 ++++++------ src/qml/qml/qqmltypeloader.cpp | 77 ++++++++++++++++++++++-------------------- src/qml/qml/qqmltypeloader_p.h | 28 +++++++++++++-- 5 files changed, 81 insertions(+), 58 deletions(-) (limited to 'src') diff --git a/src/qml/qml/qqmldirdata.cpp b/src/qml/qml/qqmldirdata.cpp index ec398fa896..7652cec322 100644 --- a/src/qml/qml/qqmldirdata.cpp +++ b/src/qml/qml/qqmldirdata.cpp @@ -51,16 +51,15 @@ const QString &QQmlQmldirData::content() const return m_content; } -const QV4::CompiledData::Import *QQmlQmldirData::import(QQmlTypeLoader::Blob *blob) const +QQmlTypeLoader::Blob::PendingImportPtr QQmlQmldirData::import(QQmlTypeLoader::Blob *blob) const { - QHash::const_iterator it = - m_imports.find(blob); + auto it = m_imports.find(blob); if (it == m_imports.end()) return nullptr; return *it; } -void QQmlQmldirData::setImport(QQmlTypeLoader::Blob *blob, const QV4::CompiledData::Import *import) +void QQmlQmldirData::setImport(QQmlTypeLoader::Blob *blob, QQmlTypeLoader::Blob::PendingImportPtr import) { m_imports[blob] = import; } diff --git a/src/qml/qml/qqmldirdata_p.h b/src/qml/qml/qqmldirdata_p.h index 6af393c47f..34f1ff1678 100644 --- a/src/qml/qml/qqmldirdata_p.h +++ b/src/qml/qml/qqmldirdata_p.h @@ -65,8 +65,8 @@ private: public: const QString &content() const; - const QV4::CompiledData::Import *import(QQmlTypeLoader::Blob *) const; - void setImport(QQmlTypeLoader::Blob *, const QV4::CompiledData::Import *); + PendingImportPtr import(QQmlTypeLoader::Blob *) const; + void setImport(QQmlTypeLoader::Blob *, PendingImportPtr); int priority(QQmlTypeLoader::Blob *) const; void setPriority(QQmlTypeLoader::Blob *, int); @@ -77,7 +77,7 @@ protected: private: QString m_content; - QHash m_imports; + QHash m_imports; QHash m_priorities; }; diff --git a/src/qml/qml/qqmltypedata.cpp b/src/qml/qml/qqmltypedata.cpp index 12d4d8cbad..8d75b57fc1 100644 --- a/src/qml/qml/qqmltypedata.cpp +++ b/src/qml/qml/qqmltypedata.cpp @@ -155,7 +155,8 @@ bool QQmlTypeData::tryLoadFromDiskCache() && import->majorVersion == -1 && import->minorVersion == -1) { QList errors; - if (!fetchQmldir(qmldirUrl, import, 1, &errors)) { + auto pendingImport = std::make_shared(this, import); + if (!fetchQmldir(qmldirUrl, pendingImport, 1, &errors)) { setError(errors); return false; } @@ -522,10 +523,8 @@ void QQmlTypeData::continueLoadFromIR() if (!loadImplicitImport()) return; // This qmldir is for the implicit import - QQmlJS::MemoryPool *pool = m_document->jsParserEngine.pool(); - auto implicitImport = pool->New(); - implicitImport->uriIndex = m_document->registerString(QLatin1String(".")); - implicitImport->qualifierIndex = 0; // empty string + auto implicitImport = std::make_shared(); + implicitImport->uri = QLatin1String("."); implicitImport->majorVersion = -1; implicitImport->minorVersion = -1; QList errors; @@ -560,16 +559,16 @@ void QQmlTypeData::allDependenciesDone() if (!m_typesResolved) { // Check that all imports were resolved QList errors; - QHash::const_iterator it = m_unresolvedImports.constBegin(), end = m_unresolvedImports.constEnd(); + auto it = m_unresolvedImports.constBegin(), end = m_unresolvedImports.constEnd(); for ( ; it != end; ++it) { - if (*it == 0) { + if ((*it)->priority == 0) { // This import was not resolved - for (auto keyIt = m_unresolvedImports.keyBegin(), - keyEnd = m_unresolvedImports.keyEnd(); + for (auto keyIt = m_unresolvedImports.constBegin(), + keyEnd = m_unresolvedImports.constEnd(); keyIt != keyEnd; ++keyIt) { - const QV4::CompiledData::Import *import = *keyIt; + PendingImportPtr import = *keyIt; QQmlError error; - error.setDescription(QQmlTypeLoader::tr("module \"%1\" is not installed").arg(stringAt(import->uriIndex))); + error.setDescription(QQmlTypeLoader::tr("module \"%1\" is not installed").arg(import->uri)); error.setUrl(m_importCache.baseUrl()); error.setLine(import->location.line); error.setColumn(import->location.column); diff --git a/src/qml/qml/qqmltypeloader.cpp b/src/qml/qml/qqmltypeloader.cpp index 4b6c09769a..a1bf9e9201 100644 --- a/src/qml/qml/qqmltypeloader.cpp +++ b/src/qml/qml/qqmltypeloader.cpp @@ -468,6 +468,16 @@ void QQmlTypeLoader::shutdownThread() m_thread->shutdown(); } +QQmlTypeLoader::Blob::PendingImport::PendingImport(QQmlTypeLoader::Blob *blob, const QV4::CompiledData::Import *import) +{ + type = static_cast(quint32(import->type)); + uri = blob->stringAt(import->uriIndex); + qualifier = blob->stringAt(import->qualifierIndex); + majorVersion = import->majorVersion; + minorVersion = import->minorVersion; + location = import->location; +} + QQmlTypeLoader::Blob::Blob(const QUrl &url, QQmlDataBlob::Type type, QQmlTypeLoader *loader) : QQmlDataBlob(url, type, loader), m_importCache(loader) { @@ -477,7 +487,7 @@ QQmlTypeLoader::Blob::~Blob() { } -bool QQmlTypeLoader::Blob::fetchQmldir(const QUrl &url, const QV4::CompiledData::Import *import, int priority, QList *errors) +bool QQmlTypeLoader::Blob::fetchQmldir(const QUrl &url, PendingImportPtr import, int priority, QList *errors) { QQmlRefPointer data = typeLoader()->getQmldir(url); @@ -497,26 +507,22 @@ bool QQmlTypeLoader::Blob::fetchQmldir(const QUrl &url, const QV4::CompiledData: return true; } -bool QQmlTypeLoader::Blob::updateQmldir(const QQmlRefPointer &data, const QV4::CompiledData::Import *import, QList *errors) +bool QQmlTypeLoader::Blob::updateQmldir(const QQmlRefPointer &data, PendingImportPtr import, QList *errors) { QString qmldirIdentifier = data->urlString(); QString qmldirUrl = qmldirIdentifier.left(qmldirIdentifier.lastIndexOf(QLatin1Char('/')) + 1); typeLoader()->setQmldirContent(qmldirIdentifier, data->content()); - if (!m_importCache.updateQmldirContent(typeLoader()->importDatabase(), stringAt(import->uriIndex), stringAt(import->qualifierIndex), qmldirIdentifier, qmldirUrl, errors)) + if (!m_importCache.updateQmldirContent(typeLoader()->importDatabase(), import->uri, import->qualifier, qmldirIdentifier, qmldirUrl, errors)) return false; - QHash::iterator it = m_unresolvedImports.find(import); - if (it != m_unresolvedImports.end()) { - *it = data->priority(this); - } + import->priority = data->priority(this); // Release this reference at destruction m_qmldirs << data; - const QString &importQualifier = stringAt(import->qualifierIndex); - if (!importQualifier.isEmpty()) { + if (!import->qualifier.isEmpty()) { // Does this library contain any qualified scripts? QUrl libraryUrl(qmldirUrl); const QQmlTypeLoaderQmldirContent qmldir = typeLoader()->qmldirContent(qmldirIdentifier); @@ -526,7 +532,7 @@ bool QQmlTypeLoader::Blob::updateQmldir(const QQmlRefPointer &da QQmlRefPointer blob = typeLoader()->getScript(scriptUrl); addDependency(blob.data()); - scriptImported(blob, import->location, script.nameSpace, importQualifier); + scriptImported(blob, import->location, script.nameSpace, import->qualifier); } } @@ -534,37 +540,40 @@ bool QQmlTypeLoader::Blob::updateQmldir(const QQmlRefPointer &da } bool QQmlTypeLoader::Blob::addImport(const QV4::CompiledData::Import *import, QList *errors) +{ + return addImport(std::make_shared(this, import), errors); +} + +bool QQmlTypeLoader::Blob::addImport(QQmlTypeLoader::Blob::PendingImportPtr import, QList *errors) { Q_ASSERT(errors); QQmlImportDatabase *importDatabase = typeLoader()->importDatabase(); - const QString &importUri = stringAt(import->uriIndex); - const QString &importQualifier = stringAt(import->qualifierIndex); if (import->type == QV4::CompiledData::Import::ImportScript) { - QUrl scriptUrl = finalUrl().resolved(QUrl(importUri)); + QUrl scriptUrl = finalUrl().resolved(QUrl(import->uri)); QQmlRefPointer blob = typeLoader()->getScript(scriptUrl); addDependency(blob.data()); - scriptImported(blob, import->location, importQualifier, QString()); + scriptImported(blob, import->location, import->qualifier, QString()); } else if (import->type == QV4::CompiledData::Import::ImportLibrary) { QString qmldirFilePath; QString qmldirUrl; - if (QQmlMetaType::isLockedModule(importUri, import->majorVersion)) { + if (QQmlMetaType::isLockedModule(import->uri, import->majorVersion)) { //Locked modules are checked first, to save on filesystem checks - if (!m_importCache.addLibraryImport(importDatabase, importUri, importQualifier, import->majorVersion, + if (!m_importCache.addLibraryImport(importDatabase, import->uri, import->qualifier, import->majorVersion, import->minorVersion, QString(), QString(), false, errors)) return false; - } else if (m_importCache.locateQmldir(importDatabase, importUri, import->majorVersion, import->minorVersion, + } else if (m_importCache.locateQmldir(importDatabase, import->uri, import->majorVersion, import->minorVersion, &qmldirFilePath, &qmldirUrl)) { // This is a local library import - if (!m_importCache.addLibraryImport(importDatabase, importUri, importQualifier, import->majorVersion, + if (!m_importCache.addLibraryImport(importDatabase, import->uri, import->qualifier, import->majorVersion, import->minorVersion, qmldirFilePath, qmldirUrl, false, errors)) return false; - if (!importQualifier.isEmpty()) { + if (!import->qualifier.isEmpty()) { // Does this library contain any qualified scripts? QUrl libraryUrl(qmldirUrl); const QQmlTypeLoaderQmldirContent qmldir = typeLoader()->qmldirContent(qmldirFilePath); @@ -574,18 +583,18 @@ bool QQmlTypeLoader::Blob::addImport(const QV4::CompiledData::Import *import, QL QQmlRefPointer blob = typeLoader()->getScript(scriptUrl); addDependency(blob.data()); - scriptImported(blob, import->location, script.nameSpace, importQualifier); + scriptImported(blob, import->location, script.nameSpace, import->qualifier); } } } else { // Is this a module? - if (QQmlMetaType::isAnyModule(importUri)) { - if (!m_importCache.addLibraryImport(importDatabase, importUri, importQualifier, import->majorVersion, + if (QQmlMetaType::isAnyModule(import->uri)) { + if (!m_importCache.addLibraryImport(importDatabase, import->uri, import->qualifier, import->majorVersion, import->minorVersion, QString(), QString(), false, errors)) return false; } else { // We haven't yet resolved this import - m_unresolvedImports.insert(import, 0); + m_unresolvedImports << import; QQmlAbstractUrlInterceptor *interceptor = typeLoader()->engine()->urlInterceptor(); @@ -596,13 +605,13 @@ bool QQmlTypeLoader::Blob::addImport(const QV4::CompiledData::Import *import, QL : QQmlImportDatabase::Remote); if (!remotePathList.isEmpty()) { // Add this library and request the possible locations for it - if (!m_importCache.addLibraryImport(importDatabase, importUri, importQualifier, import->majorVersion, + if (!m_importCache.addLibraryImport(importDatabase, import->uri, import->qualifier, import->majorVersion, import->minorVersion, QString(), QString(), true, errors)) return false; // Probe for all possible locations int priority = 0; - const QStringList qmlDirPaths = QQmlImports::completeQmldirPaths(importUri, remotePathList, import->majorVersion, import->minorVersion); + const QStringList qmlDirPaths = QQmlImports::completeQmldirPaths(import->uri, remotePathList, import->majorVersion, import->minorVersion); for (const QString &qmldirPath : qmlDirPaths) { if (interceptor) { QUrl url = interceptor->intercept( @@ -625,7 +634,7 @@ bool QQmlTypeLoader::Blob::addImport(const QV4::CompiledData::Import *import, QL bool incomplete = false; - QUrl importUrl(importUri); + QUrl importUrl(import->uri); QString path = importUrl.path(); path.append(QLatin1String(path.endsWith(QLatin1Char('/')) ? "qmldir" : "/qmldir")); importUrl.setPath(path); @@ -635,7 +644,7 @@ bool QQmlTypeLoader::Blob::addImport(const QV4::CompiledData::Import *import, QL incomplete = true; } - if (!m_importCache.addFileImport(importDatabase, importUri, importQualifier, import->majorVersion, + if (!m_importCache.addFileImport(importDatabase, import->uri, import->qualifier, import->majorVersion, import->minorVersion, incomplete, errors)) return false; @@ -653,7 +662,7 @@ void QQmlTypeLoader::Blob::dependencyComplete(QQmlDataBlob *blob) if (blob->type() == QQmlDataBlob::QmldirFile) { QQmlQmldirData *data = static_cast(blob); - const QV4::CompiledData::Import *import = data->import(this); + PendingImportPtr import = data->import(this); QList errors; if (!qmldirDataAvailable(data, &errors)) { @@ -685,9 +694,7 @@ bool QQmlTypeLoader::Blob::diskCacheForced() bool QQmlTypeLoader::Blob::qmldirDataAvailable(const QQmlRefPointer &data, QList *errors) { - bool resolve = true; - - const QV4::CompiledData::Import *import = data->import(this); + PendingImportPtr import = data->import(this); data->setImport(this, nullptr); int priority = data->priority(this); @@ -695,10 +702,7 @@ bool QQmlTypeLoader::Blob::qmldirDataAvailable(const QQmlRefPointer::iterator it = m_unresolvedImports.find(import); - if (it != m_unresolvedImports.end()) { - resolve = (*it == 0) || (*it > priority); - } + const bool resolve = (import->priority == 0) || (import->priority > priority); if (resolve) { // This is the (current) best resolution for this import @@ -706,8 +710,7 @@ bool QQmlTypeLoader::Blob::qmldirDataAvailable(const QQmlRefPointerpriority = priority; return true; } } diff --git a/src/qml/qml/qqmltypeloader_p.h b/src/qml/qml/qqmltypeloader_p.h index 5710bdba56..f8cec700a5 100644 --- a/src/qml/qml/qqmltypeloader_p.h +++ b/src/qml/qml/qqmltypeloader_p.h @@ -61,6 +61,8 @@ #include #include +#include + QT_BEGIN_NAMESPACE class QQmlScriptBlob; @@ -87,11 +89,31 @@ public: void setCachedUnitStatus(QQmlMetaType::CachedUnitLookupError status) { m_cachedUnitStatus = status; } + struct PendingImport + { + QV4::CompiledData::Import::ImportType type = QV4::CompiledData::Import::ImportType::ImportLibrary; + + QString uri; + QString qualifier; + + int majorVersion = -1; + int minorVersion = -1; + + QV4::CompiledData::Location location; + + int priority = 0; + + PendingImport() = default; + PendingImport(Blob *blob, const QV4::CompiledData::Import *import); + }; + using PendingImportPtr = std::shared_ptr; + protected: bool addImport(const QV4::CompiledData::Import *import, QList *errors); + bool addImport(PendingImportPtr import, QList *errors); - bool fetchQmldir(const QUrl &url, const QV4::CompiledData::Import *import, int priority, QList *errors); - bool updateQmldir(const QQmlRefPointer &data, const QV4::CompiledData::Import *import, QList *errors); + bool fetchQmldir(const QUrl &url, PendingImportPtr import, int priority, QList *errors); + bool updateQmldir(const QQmlRefPointer &data, PendingImportPtr import, QList *errors); private: virtual bool qmldirDataAvailable(const QQmlRefPointer &, QList *); @@ -109,7 +131,7 @@ public: static bool diskCacheForced(); QQmlImports m_importCache; - QHash m_unresolvedImports; + QVector m_unresolvedImports; QVector> m_qmldirs; QQmlMetaType::CachedUnitLookupError m_cachedUnitStatus = QQmlMetaType::CachedUnitLookupError::NoError; }; -- cgit v1.2.3