QMimeDatabase::mimeTypeForFile: don't create QFileInfo on UNIX

When mode != MatchExtension is passed to
QMimeDatabase::mimeTypeForFile(const QString &fileName, MatchMode mode) const
a QFileInfo object is always created and used to check
QFileInfo::isDir(). Replace this expensive approach with a very cheap
(bitmask) S_ISDIR check under Q_OS_UNIX. QT_STAT is already called for
other reasons, so we just reuse its result in the S_ISDIR check.

Extract almost all code from
QMimeDatabase::mimeTypeForFile(const QString &, MatchMode) const;
QMimeDatabase::mimeTypeForData(QIODevice *) const;
QMimeDatabase::mimeTypeForFile(const QFileInfo &, MatchMode) const
into
QMimeDatabasePrivate::mimeTypeForFileExtension(const QString &);
QMimeDatabasePrivate::mimeTypeForData(QIODevice *);
QMimeDatabasePrivate::mimeTypeForFile(const QString &, const QFileInfo *, QMimeDatabase::MatchMode)
respectively. This refactoring is a less convoluted and more efficient
alternative to adding more mutex-unlocking hacks. The existing
QMutexLocker::unlock() calls are thereby eliminated.

The following table contains the average results of the benchmark
tst_QMimeDatabase::benchMimeTypeForFile() on my GNU/Linux system before
and at this commit. The numbers denote milliseconds per iteration.
This commit does not measurably affect performance when
mode == MatchExtension, therefore the MatchExtension section contains a
single column with results common to both commits (it is interesting to
compare benchmark results across match modes).

        data row tag                        before  at
MatchDefault:
        archive                             0.012   0.010
        OpenDocument Text                   0.012   0.010
        existent archive with extension     0.022   0.018
        existent C with extension           0.016   0.012
        existent text file with extension   0.017   0.012
        existent C w/o extension            0.072   0.067
        existent patch w/o extension        0.10    0.10
        existent archive w/o extension      0.064   0.060
MatchExtension:
        archive                                 0.0081
        OpenDocument Text                       0.0076
        existent archive with extension         0.012
        existent C with extension               0.0075
        existent text file with extension       0.0076
        existent C w/o extension                0.0122
        existent patch w/o extension            0.0091
        existent archive w/o extension          0.0091
MatchContent:
        archive                             0.0098  0.0078
        OpenDocument Text                   0.0098  0.0078
        existent archive with extension     0.051   0.047
        existent C with extension           0.051   0.047
        existent text file with extension   0.053   0.049
        existent C w/o extension            0.058   0.054
        existent patch w/o extension        0.099   0.095
        existent archive w/o extension      0.054   0.050

Change-Id: I01b37c9645f0d8d35fd1a6ea1c7a99be2faeeb73
Reviewed-by: Thiago Macieira <thiago.macieira@intel.com>
Reviewed-by: David Faure <david.faure@kdab.com>

2 files changed, 88 insertions, 68 deletions

diff --git a/src/corelib/mimetypes/qmimedatabase.cpp b/src/corelib/mimetypes/qmimedatabase.cpp
index f7cd926220..24ccdccd69 100644
--- a/src/corelib/mimetypes/qmimedatabase.cpp
+++ b/src/corelib/mimetypes/qmimedatabase.cpp
@@ -437,6 +437,83 @@ QMimeType QMimeDatabasePrivate::mimeTypeForFileNameAndData(const QString &fileNa
     return matchOnContent(&fallbackFile);
 }
 
+QMimeType QMimeDatabasePrivate::mimeTypeForFileExtension(const QString &fileName)
+{
+    const QStringList matches = mimeTypeForFileName(fileName);
+    const int matchCount = matches.count();
+    if (matchCount == 0) {
+        return mimeTypeForName(defaultMimeType());
+    } else if (matchCount == 1) {
+        return mimeTypeForName(matches.first());
+    } else {
+        // We have to pick one.
+        return mimeTypeForName(matches.first());
+    }
+}
+
+QMimeType QMimeDatabasePrivate::mimeTypeForData(QIODevice *device)
+{
+    int accuracy = 0;
+    const bool openedByUs = !device->isOpen() && device->open(QIODevice::ReadOnly);
+    if (device->isOpen()) {
+        // Read 16K in one go (QIODEVICE_BUFFERSIZE in qiodevice_p.h).
+        // This is much faster than seeking back and forth into QIODevice.
+        const QByteArray data = device->peek(16384);
+        const QMimeType result = findByData(data, &accuracy);
+        if (openedByUs)
+            device->close();
+        return result;
+    }
+    return mimeTypeForName(defaultMimeType());
+}
+
+QMimeType QMimeDatabasePrivate::mimeTypeForFile(const QString &fileName,
+                                                [[maybe_unused]] const QFileInfo *fileInfo,
+                                                QMimeDatabase::MatchMode mode)
+{
+#ifdef Q_OS_UNIX
+    // Cannot access statBuf.st_mode from the filesystem engine, so we have to stat again.
+    // In addition we want to follow symlinks.
+    const QByteArray nativeFilePath = QFile::encodeName(fileName);
+    QT_STATBUF statBuffer;
+    if (QT_STAT(nativeFilePath.constData(), &statBuffer) == 0) {
+        if (S_ISDIR(statBuffer.st_mode))
+            return mimeTypeForName(QLatin1String("inode/directory"));
+        if (S_ISCHR(statBuffer.st_mode))
+            return mimeTypeForName(QLatin1String("inode/chardevice"));
+        if (S_ISBLK(statBuffer.st_mode))
+            return mimeTypeForName(QLatin1String("inode/blockdevice"));
+        if (S_ISFIFO(statBuffer.st_mode))
+            return mimeTypeForName(QLatin1String("inode/fifo"));
+        if (S_ISSOCK(statBuffer.st_mode))
+            return mimeTypeForName(QLatin1String("inode/socket"));
+    }
+#else
+    const bool isDirectory = fileInfo ? fileInfo->isDir() : QFileInfo(fileName).isDir();
+    if (isDirectory)
+        return mimeTypeForName(QLatin1String("inode/directory"));
+#endif
+
+    int priority = 0;
+    switch (mode) {
+    case QMimeDatabase::MatchDefault:
+        return mimeTypeForFileNameAndData(fileName, nullptr, &priority);
+    case QMimeDatabase::MatchExtension:
+        return mimeTypeForFileExtension(fileName);
+    case QMimeDatabase::MatchContent: {
+        QFile file(fileName);
+        if (file.open(QIODevice::ReadOnly)) {
+            return mimeTypeForData(&file);
+        } else {
+            return mimeTypeForName(defaultMimeType());
+        }
+    }
+    default:
+        Q_ASSERT(false);
+    }
+    return mimeTypeForName(defaultMimeType());
+}
+
 QList<QMimeType> QMimeDatabasePrivate::allMimeTypes()
 {
     QList<QMimeType> result;
@@ -568,48 +645,7 @@ QMimeType QMimeDatabase::mimeTypeForFile(const QFileInfo &fileInfo, MatchMode mo
 {
     QMutexLocker locker(&d->mutex);
 
-    if (fileInfo.isDir())
-        return d->mimeTypeForName(QLatin1String("inode/directory"));
-
-    const QString filePath = fileInfo.filePath();
-
-#ifdef Q_OS_UNIX
-    // Cannot access statBuf.st_mode from the filesystem engine, so we have to stat again.
-    // In addition we want to follow symlinks.
-    const QByteArray nativeFilePath = QFile::encodeName(filePath);
-    QT_STATBUF statBuffer;
-    if (QT_STAT(nativeFilePath.constData(), &statBuffer) == 0) {
-        if (S_ISCHR(statBuffer.st_mode))
-            return d->mimeTypeForName(QLatin1String("inode/chardevice"));
-        if (S_ISBLK(statBuffer.st_mode))
-            return d->mimeTypeForName(QLatin1String("inode/blockdevice"));
-        if (S_ISFIFO(statBuffer.st_mode))
-            return d->mimeTypeForName(QLatin1String("inode/fifo"));
-        if (S_ISSOCK(statBuffer.st_mode))
-            return d->mimeTypeForName(QLatin1String("inode/socket"));
-    }
-#endif
-
-    int priority = 0;
-    switch (mode) {
-    case MatchDefault:
-        return d->mimeTypeForFileNameAndData(filePath, nullptr, &priority);
-    case MatchExtension:
-        locker.unlock();
-        return mimeTypeForFile(filePath, mode);
-    case MatchContent: {
-        QFile file(filePath);
-        if (file.open(QIODevice::ReadOnly)) {
-            locker.unlock();
-            return mimeTypeForData(&file);
-        } else {
-            return d->mimeTypeForName(d->defaultMimeType());
-        }
-    }
-    default:
-        Q_ASSERT(false);
-    }
-    return d->mimeTypeForName(d->defaultMimeType());
+    return d->mimeTypeForFile(fileInfo.filePath(), &fileInfo, mode);
 }
 
 /*!
@@ -619,22 +655,12 @@ QMimeType QMimeDatabase::mimeTypeForFile(const QFileInfo &fileInfo, MatchMode mo
 */
 QMimeType QMimeDatabase::mimeTypeForFile(const QString &fileName, MatchMode mode) const
 {
+    QMutexLocker locker(&d->mutex);
+
     if (mode == MatchExtension) {
-        QMutexLocker locker(&d->mutex);
-        const QStringList matches = d->mimeTypeForFileName(fileName);
-        const int matchCount = matches.count();
-        if (matchCount == 0) {
-            return d->mimeTypeForName(d->defaultMimeType());
-        } else if (matchCount == 1) {
-            return d->mimeTypeForName(matches.first());
-        } else {
-            // We have to pick one.
-            return d->mimeTypeForName(matches.first());
-        }
+        return d->mimeTypeForFileExtension(fileName);
     } else {
-        // Implemented as a wrapper around mimeTypeForFile(QFileInfo), so no mutex.
-        QFileInfo fileInfo(fileName);
-        return mimeTypeForFile(fileInfo, mode);
+        return d->mimeTypeForFile(fileName, nullptr, mode);
     }
 }
 
@@ -700,18 +726,7 @@ QMimeType QMimeDatabase::mimeTypeForData(QIODevice *device) const
 {
     QMutexLocker locker(&d->mutex);
 
-    int accuracy = 0;
-    const bool openedByUs = !device->isOpen() && device->open(QIODevice::ReadOnly);
-    if (device->isOpen()) {
-        // Read 16K in one go (QIODEVICE_BUFFERSIZE in qiodevice_p.h).
-        // This is much faster than seeking back and forth into QIODevice.
-        const QByteArray data = device->peek(16384);
-        const QMimeType result = d->findByData(data, &accuracy);
-        if (openedByUs)
-            device->close();
-        return result;
-    }
-    return d->mimeTypeForName(d->defaultMimeType());
+    return d->mimeTypeForData(device);
 }
 
 /*!
diff --git a/src/corelib/mimetypes/qmimedatabase_p.h b/src/corelib/mimetypes/qmimedatabase_p.h
index e6af23e7ed..d5ed61b0b9 100644
--- a/src/corelib/mimetypes/qmimedatabase_p.h
+++ b/src/corelib/mimetypes/qmimedatabase_p.h
@@ -52,6 +52,7 @@
 // We mean it.
 //
 
+#include "qmimedatabase.h"
 #include "qmimetype.h"
 
 QT_REQUIRE_CONFIG(mimetype);
@@ -68,6 +69,7 @@ QT_REQUIRE_CONFIG(mimetype);
 
 QT_BEGIN_NAMESPACE
 
+class QFileInfo;
 class QIODevice;
 class QMimeDatabase;
 class QMimeProviderBase;
@@ -92,6 +94,9 @@ public:
     QStringList parents(const QString &mimeName);
     QMimeType mimeTypeForName(const QString &nameOrAlias);
     QMimeType mimeTypeForFileNameAndData(const QString &fileName, QIODevice *device, int *priorityPtr);
+    QMimeType mimeTypeForFileExtension(const QString &fileName);
+    QMimeType mimeTypeForData(QIODevice *device);
+    QMimeType mimeTypeForFile(const QString &fileName, const QFileInfo *fileInfo, QMimeDatabase::MatchMode mode);
     QMimeType findByData(const QByteArray &data, int *priorityPtr);
     QStringList mimeTypeForFileName(const QString &fileName);
     QMimeGlobMatchResult findByFileName(const QString &fileName);