From fbb2bd0a25807210ca76022cc1c9e09de206bfc9 Mon Sep 17 00:00:00 2001 From: Marc Mutz Date: Wed, 7 Feb 2024 21:32:20 +0100 Subject: [PATCH] Add fast-path in QLibraryStore::findOrCreate() for empty fileName The LibraryMap can never contain an object whose fileName is empty. To see that, observe that the only insertion into LibraryMap is in findOrCreate() and that refuses to add such objects there. But if LibraryMap cannot contain such an object, and findOrCreate({}) is just an ugly way to get a default-constructed QLibraryPrivate (a new one for each call), then we don't need to lock the qt_library_mutex to produce one, and neither do we need to construct a mapName that we know we'll not find, anyway. So drag this case to before the mutex locking and the construction of mapName. It took me more coffee than I'm ready to admit to figure this out, so leave a comment for the next reader indicating that an empty fileName is actually a valid argument. To avoid repeating the new-expression, wrap it in a lambda, together with the ref() call. Move the remaining ref() call to where it's still needed. The final goal of this exercise is to get rid of the double-lookup in LibraryMap. Pick-to: 6.6 6.5 Change-Id: I781eafdb9516410d7a262ad27f52c38ad2742292 Reviewed-by: Thiago Macieira (cherry picked from commit b28bad0b20719f72fb335a65f76382132e326ad0) Reviewed-by: Qt Cherry-pick Bot --- src/corelib/plugin/qlibrary.cpp | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/src/corelib/plugin/qlibrary.cpp b/src/corelib/plugin/qlibrary.cpp index 1f6d3e9019d..ff1f4513ab7 100644 --- a/src/corelib/plugin/qlibrary.cpp +++ b/src/corelib/plugin/qlibrary.cpp @@ -395,6 +395,15 @@ QLibraryStore *QLibraryStore::instance() inline QLibraryPrivate *QLibraryStore::findOrCreate(const QString &fileName, const QString &version, QLibrary::LoadHints loadHints) { + auto lazyNewLib = [&] { + auto result = new QLibraryPrivate(fileName, version, loadHints); + result->libraryRefCount.ref(); + return result; + }; + + if (fileName.isEmpty()) // request for empty d-pointer in QLibrary::setLoadHints(); + return lazyNewLib(); // must return an independent (new) object + QMutexLocker locker(&qt_library_mutex); QLibraryStore *data = instance(); @@ -404,17 +413,18 @@ inline QLibraryPrivate *QLibraryStore::findOrCreate(const QString &fileName, con QLibraryPrivate *lib = nullptr; if (Q_LIKELY(data)) { lib = data->libraryMap.value(mapName); - if (lib) + if (lib) { + lib->libraryRefCount.ref(); lib->mergeLoadHints(loadHints); + } } if (!lib) - lib = new QLibraryPrivate(fileName, version, loadHints); + lib = lazyNewLib(); // track this library - if (Q_LIKELY(data) && !fileName.isEmpty()) + if (Q_LIKELY(data)) data->libraryMap.insert(mapName, lib); - lib->libraryRefCount.ref(); return lib; }