From 2e7d5def1fdabb5949fbffc629da500aa2bb78d7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Abecasis?= Date: Fri, 6 Aug 2010 16:04:27 +0200 Subject: There goes QResourceFileEngineHandler There's no reason why resources can't be handled in the default handler. Reviewed-by: Marius Storm-Olsen --- src/corelib/io/qabstractfileengine.cpp | 7 ++++++- src/corelib/io/qresource.cpp | 23 +---------------------- 2 files changed, 7 insertions(+), 23 deletions(-) diff --git a/src/corelib/io/qabstractfileengine.cpp b/src/corelib/io/qabstractfileengine.cpp index d6b6f11..75eaeb4 100644 --- a/src/corelib/io/qabstractfileengine.cpp +++ b/src/corelib/io/qabstractfileengine.cpp @@ -41,6 +41,9 @@ #include "qabstractfileengine.h" #include "private/qabstractfileengine_p.h" +#ifdef QT_BUILD_CORE_LIB +#include "private/qresource_p.h" +#endif #include "qdatetime.h" #include "qreadwritelock.h" #include "qvariant.h" @@ -179,7 +182,9 @@ QAbstractFileEngine *QAbstractFileEngine::create(const QString &fileName) #ifdef QT_BUILD_CORE_LIB if (!fileName.startsWith(QLatin1Char('/'))) { int prefixSeparator = fileName.indexOf(QLatin1Char(':')); - if (prefixSeparator > 1) { + if (prefixSeparator == 0) { + return new QResourceFileEngine(fileName); + } else if (prefixSeparator > 1) { QString prefix = fileName.left(prefixSeparator); QString fileNameWithoutPrefix = fileName.mid(prefixSeparator + 1).prepend(QLatin1Char('/')); const QStringList &paths = QDir::searchPaths(prefix); diff --git a/src/corelib/io/qresource.cpp b/src/corelib/io/qresource.cpp index ce9c57e..929885c 100644 --- a/src/corelib/io/qresource.cpp +++ b/src/corelib/io/qresource.cpp @@ -1185,21 +1185,6 @@ QResource::unregisterResource(const uchar *rccData, const QString &resourceRoot) return false; } -//file type handler -class QResourceFileEngineHandler : public QAbstractFileEngineHandler -{ -public: - QResourceFileEngineHandler() { } - ~QResourceFileEngineHandler() { } - QAbstractFileEngine *create(const QString &path) const; -}; -QAbstractFileEngine *QResourceFileEngineHandler::create(const QString &path) const -{ - if (path.size() > 0 && path.startsWith(QLatin1Char(':'))) - return new QResourceFileEngine(path); - return 0; -} - //resource engine class QResourceFileEnginePrivate : public QAbstractFileEnginePrivate { @@ -1506,12 +1491,6 @@ bool QResourceFileEnginePrivate::unmap(uchar *ptr) return true; } -//Initialization and cleanup -Q_GLOBAL_STATIC(QResourceFileEngineHandler, resource_file_handler) - -static int qt_force_resource_init() { resource_file_handler(); return 1; } -Q_CORE_EXPORT void qInitResourceIO() { resource_file_handler(); } -static int qt_forced_resource_init = qt_force_resource_init(); -Q_CONSTRUCTOR_FUNCTION(qt_force_resource_init) +Q_CORE_EXPORT void qInitResourceIO() { } // ### Qt 5: remove QT_END_NAMESPACE -- cgit v0.12 From 9a3e7c9c32167b777a090ee13082323d10895694 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Abecasis?= Date: Mon, 9 Aug 2010 13:02:33 +0200 Subject: Fix minor premature pessimizations in file engine creation QAbstractFileEngineHandler ctor adds itself once to handler list, dtor should not try to be any smarter. Repeatedly calling the global static "constructor" fileEngineHandlers() could lead to additional overhead while checking for repeated construction -- that said, compilers can probably figure this one out, but being explicit shouldn't hurt. Reduce string manipulation and copying. Also, prefer QStringBuilder to building strings by hand. Reviewed-by: Marius Storm-Olsen --- src/corelib/io/qabstractfileengine.cpp | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/src/corelib/io/qabstractfileengine.cpp b/src/corelib/io/qabstractfileengine.cpp index 75eaeb4..80bc244 100644 --- a/src/corelib/io/qabstractfileengine.cpp +++ b/src/corelib/io/qabstractfileengine.cpp @@ -50,6 +50,7 @@ // built-in handlers #include "qfsfileengine.h" #include "qdiriterator.h" +#include "qstringbuilder.h" QT_BEGIN_NAMESPACE @@ -138,7 +139,7 @@ QAbstractFileEngineHandler::~QAbstractFileEngineHandler() QWriteLocker locker(fileEngineHandlerMutex()); // Remove this handler from the handler list only if the list is valid. if (!qt_abstractfileenginehandlerlist_shutDown) - fileEngineHandlers()->removeAll(this); + fileEngineHandlers()->removeOne(this); } /*! @@ -173,8 +174,9 @@ QAbstractFileEngine *QAbstractFileEngine::create(const QString &fileName) QReadLocker locker(fileEngineHandlerMutex()); // check for registered handlers that can load the file - for (int i = 0; i < fileEngineHandlers()->size(); i++) { - if (QAbstractFileEngine *ret = fileEngineHandlers()->at(i)->create(fileName)) + QAbstractFileEngineHandlerList *handlers = fileEngineHandlers(); + for (int i = 0; i < handlers->size(); i++) { + if (QAbstractFileEngine *ret = handlers->at(i)->create(fileName)) return ret; } } @@ -185,13 +187,9 @@ QAbstractFileEngine *QAbstractFileEngine::create(const QString &fileName) if (prefixSeparator == 0) { return new QResourceFileEngine(fileName); } else if (prefixSeparator > 1) { - QString prefix = fileName.left(prefixSeparator); - QString fileNameWithoutPrefix = fileName.mid(prefixSeparator + 1).prepend(QLatin1Char('/')); - const QStringList &paths = QDir::searchPaths(prefix); + const QStringList &paths = QDir::searchPaths(fileName.left(prefixSeparator)); for (int i = 0; i < paths.count(); i++) { - QString path = paths.at(i); - path.append(fileNameWithoutPrefix); - QAbstractFileEngine *engine = create(path); + QAbstractFileEngine *engine = create(paths.at(i) % QLatin1Char('/') % fileName.mid(prefixSeparator + 1)); if (engine && (engine->fileFlags(QAbstractFileEngine::FlagsMask) & QAbstractFileEngine::ExistsFlag)) { return engine; } -- cgit v0.12 From 127cab437b53096b18beb0cfe299137dc384060d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Abecasis?= Date: Mon, 9 Aug 2010 17:50:58 +0200 Subject: Make prefix handling in file engine creation more robust ... and make it exit earlier, when feasible. While we could fully validate the prefix at this level, consulting Unicode tables on each character of a file name could quickly become expensive. Furthermore, we can already reap some benefits just by checking for '/' before ':'. Reviewed-by: Marius Storm-Olsen --- src/corelib/io/qabstractfileengine.cpp | 26 +++++++++++++++++++++----- 1 file changed, 21 insertions(+), 5 deletions(-) diff --git a/src/corelib/io/qabstractfileengine.cpp b/src/corelib/io/qabstractfileengine.cpp index 80bc244..6646e4e 100644 --- a/src/corelib/io/qabstractfileengine.cpp +++ b/src/corelib/io/qabstractfileengine.cpp @@ -182,11 +182,18 @@ QAbstractFileEngine *QAbstractFileEngine::create(const QString &fileName) } #ifdef QT_BUILD_CORE_LIB - if (!fileName.startsWith(QLatin1Char('/'))) { - int prefixSeparator = fileName.indexOf(QLatin1Char(':')); - if (prefixSeparator == 0) { - return new QResourceFileEngine(fileName); - } else if (prefixSeparator > 1) { + for (int prefixSeparator = 0; prefixSeparator < fileName.size(); ++prefixSeparator) { + QChar const ch = fileName[prefixSeparator]; + if (ch == QLatin1Char('/')) + break; + + if (ch == QLatin1Char(':')) { + if (prefixSeparator == 0) + return new QResourceFileEngine(fileName); + + if (prefixSeparator == 1) + break; + const QStringList &paths = QDir::searchPaths(fileName.left(prefixSeparator)); for (int i = 0; i < paths.count(); i++) { QAbstractFileEngine *engine = create(paths.at(i) % QLatin1Char('/') % fileName.mid(prefixSeparator + 1)); @@ -195,7 +202,16 @@ QAbstractFileEngine *QAbstractFileEngine::create(const QString &fileName) } delete engine; } + + break; } + + // There's no need to fully validate the prefix here. Consulting the + // unicode tables could be expensive and validation is already + // performed in QDir::setSearchPaths. + // + // if (!ch.isLetterOrNumber()) + // break; } #endif -- cgit v0.12 From 1aca3a1c6d104e52a15039cdbbe88a5e55c9235e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Abecasis?= Date: Mon, 9 Aug 2010 17:56:14 +0200 Subject: Are custom file engines in use? If no custom file engine handlers have been registered, we can avoid using the Read/Write lock and, most of the time, we also avoid the creating the handler list at all. Since the custom handler for QResource's engine has been folded into the default handler, there should be no other internal use of this API, thus triggering this optimization by default for all users. The new flag may also in the future be exported to other modules to trigger bypassing of file engines completely. Reviewed-by: Marius Storm-Olsen --- src/corelib/io/qabstractfileengine.cpp | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/src/corelib/io/qabstractfileengine.cpp b/src/corelib/io/qabstractfileengine.cpp index 6646e4e..37a093c 100644 --- a/src/corelib/io/qabstractfileengine.cpp +++ b/src/corelib/io/qabstractfileengine.cpp @@ -98,6 +98,8 @@ QT_BEGIN_NAMESPACE \sa QAbstractFileEngine, QAbstractFileEngine::create() */ +static bool qt_file_engine_handlers_in_use = false; + /* All application-wide handlers are stored in this list. The mutex must be acquired to ensure thread safety. @@ -127,6 +129,7 @@ Q_GLOBAL_STATIC(QAbstractFileEngineHandlerList, fileEngineHandlers) QAbstractFileEngineHandler::QAbstractFileEngineHandler() { QWriteLocker locker(fileEngineHandlerMutex()); + qt_file_engine_handlers_in_use = true; fileEngineHandlers()->prepend(this); } @@ -138,8 +141,12 @@ QAbstractFileEngineHandler::~QAbstractFileEngineHandler() { QWriteLocker locker(fileEngineHandlerMutex()); // Remove this handler from the handler list only if the list is valid. - if (!qt_abstractfileenginehandlerlist_shutDown) - fileEngineHandlers()->removeOne(this); + if (!qt_abstractfileenginehandlerlist_shutDown) { + QAbstractFileEngineHandlerList *handlers = fileEngineHandlers(); + handlers->removeOne(this); + if (handlers->isEmpty()) + qt_file_engine_handlers_in_use = false; + } } /*! @@ -170,7 +177,7 @@ QAbstractFileEngineHandler::~QAbstractFileEngineHandler() */ QAbstractFileEngine *QAbstractFileEngine::create(const QString &fileName) { - { + if (qt_file_engine_handlers_in_use) { QReadLocker locker(fileEngineHandlerMutex()); // check for registered handlers that can load the file -- cgit v0.12