QFile benchmark: only set up and tear down the data directory once

None of the tests modify the data, so there's no risk that one test
will cause another to fail via that. We can thus avoid the repeated
cost of that set-up and teardown, which was done repeatedly for each
test function since benchmarks get run repeatedly if they're quick.

Use QTemporaryDir to manage the test data, so that it's tidied away
automagically, instead of trying to tidy up at the end of each test
(which was, of course, skipped if the test failed).

As drive-bys, fix a typo in a QFAIL()'s message, change some C casts
that silently bulldozed const away to reinterpret_cast<>s with the
const qualifier and turn some heap buffers into stack buffers to save
the need to delete [] them at the end of their tests (also skipped on
failure).

Inspired by a kindred change by Andreas Buhr and a suggestion on its
review by Friedemann Kleint.

Pick-to: 6.2 6.1 5.15
Change-Id: I6067eb35babfbac02990ef39817b0d5122f563cd
Reviewed-by: Andreas Buhr <andreas.buhr@qt.io>

1 files changed, 73 insertions, 99 deletions

diff --git a/tests/benchmarks/corelib/io/qfile/main.cpp b/tests/benchmarks/corelib/io/qfile/main.cpp
index 5915383888..38c5482d77 100644
--- a/tests/benchmarks/corelib/io/qfile/main.cpp
+++ b/tests/benchmarks/corelib/io/qfile/main.cpp
@@ -110,54 +110,73 @@ private:
     void readFile_data(BenchmarkType type, QIODevice::OpenModeFlag t, QIODevice::OpenModeFlag b);
     void readBigFile();
     void readSmallFiles();
-    void createFile();
-    void fillFile(int factor=FACTOR);
-    void removeFile();
-    void createSmallFiles();
-    void removeSmallFiles();
-    QString filename;
-    QString tmpDirName;
+
+    class TestDataDir : public QTemporaryDir
+    {
+        void createFile();
+        void createSmallFiles();
+    public:
+        TestDataDir() : QTemporaryDir(), fail(errorString().toLocal8Bit())
+        {
+            if (fail.isEmpty() && !QTemporaryDir::isValid())
+                fail = "Failed to create temporary directory for data";
+            if (isValid())
+                createSmallFiles();
+            if (isValid())
+                createFile();
+            if (isValid())
+                QTest::qSleep(2000); // let IO settle
+        }
+        bool isValid() { return QTemporaryDir::isValid() && fail.isEmpty(); }
+        QByteArray fail;
+        QString filename;
+    } tempDir;
 };
 
 Q_DECLARE_METATYPE(tst_qfile::BenchmarkType)
 Q_DECLARE_METATYPE(QIODevice::OpenMode)
 Q_DECLARE_METATYPE(QIODevice::OpenModeFlag)
 
-void tst_qfile::createFile()
+/* None of the tests modify the test data in tempDir, so it's OK to only create
+ * and tear down the directory once.
+ */
+void tst_qfile::TestDataDir::createFile()
 {
-    removeFile();  // Cleanup in case previous test case aborted before cleaning up
-
-    QTemporaryFile tmpFile;
-    tmpFile.setAutoRemove(false);
-    if (!tmpFile.open())
-        ::exit(1);
+    QFile tmpFile(filePath("testFile"));
+    if (!tmpFile.open(QIODevice::WriteOnly)) {
+        fail = "Unable to prepare files for test";
+        return;
+    }
+#if 0 // Varied data, rather than filling with '\0' bytes:
+    for (int row = 0; row < FACTOR; ++row) {
+        tmpFile.write(QByteArray().fill('0' + row % ('0' - 'z'), 80));
+        tmpFile.write("\n");
+    }
+#else
+    tmpFile.seek(FACTOR * 80);
+    tmpFile.putChar('\n');
+#endif
     filename = tmpFile.fileName();
     tmpFile.close();
 }
 
-void tst_qfile::removeFile()
-{
-    if (!filename.isEmpty())
-        QFile::remove(filename);
-}
-
-void tst_qfile::fillFile(int factor)
+void tst_qfile::TestDataDir::createSmallFiles()
 {
-    QFile tmpFile(filename);
-    tmpFile.open(QIODevice::WriteOnly);
-    //for (int row=0; row<factor; ++row) {
-    //    tmpFile.write(QByteArray().fill('0'+row%('0'-'z'), 80));
-    //    tmpFile.write("\n");
-    //}
-    tmpFile.seek(factor*80);
-    tmpFile.putChar('\n');
-    tmpFile.close();
-    // let IO settle
-    QTest::qSleep(2000);
+    for (int i = 0; i < 1000; ++i) {
+        QFile f(filePath(QString::number(i)));
+        if (!f.open(QIODevice::WriteOnly)) {
+            fail = "Unable to prepare small files for test";
+            return;
+        }
+        f.seek(511);
+        f.putChar('\n');
+        f.close();
+    }
 }
 
 void tst_qfile::initTestCase()
 {
+    QVERIFY2(tempDir.isValid(), tempDir.fail.constData());
 }
 
 void tst_qfile::readFile_data(BenchmarkType type, QIODevice::OpenModeFlag t,
@@ -228,13 +247,10 @@ void tst_qfile::readBigFile()
     QFETCH(QFile::OpenModeFlag, textMode);
     QFETCH(QFile::OpenModeFlag, bufferedMode);
 
-    char *buffer = new char[BUFSIZE];
-    createFile();
-    fillFile();
-
+    char buffer[BUFSIZE];
     switch (testType) {
         case(QFileBenchmark): {
-            QFile file(filename);
+            QFile file(tempDir.filename);
             file.open(QIODevice::ReadOnly|textMode|bufferedMode);
             QBENCHMARK {
                 while(!file.atEnd())
@@ -246,7 +262,7 @@ void tst_qfile::readBigFile()
         break;
 #ifdef QT_BUILD_INTERNAL
         case(QFSFileEngineBenchmark): {
-            QFSFileEngine fse(filename);
+            QFSFileEngine fse(tempDir.filename);
             fse.open(QIODevice::ReadOnly|textMode|bufferedMode);
             QBENCHMARK {
                //qWarning() << fse.supportsExtension(QAbstractFileEngine::AtEndExtension);
@@ -258,7 +274,7 @@ void tst_qfile::readBigFile()
         break;
 #endif
         case(PosixBenchmark): {
-            QByteArray data = filename.toLocal8Bit();
+            QByteArray data = tempDir.filename.toLocal8Bit();
             const char* cfilename = data.constData();
             FILE* cfile = ::fopen(cfilename, "rb");
             QBENCHMARK {
@@ -278,7 +294,7 @@ void tst_qfile::readBigFile()
             HANDLE hndl;
 
             // ensure we don't account string conversion
-            wchar_t* cfilename = (wchar_t*)filename.utf16();
+            const wchar_t *cfilename = reinterpret_cast<const wchar_t *>(tempDir.filename.utf16());
 
             hndl = CreateFile(cfilename, GENERIC_READ, 0, 0, OPEN_EXISTING, 0, 0);
             Q_ASSERT(hndl);
@@ -299,9 +315,6 @@ void tst_qfile::readBigFile()
         }
         break;
     }
-
-    removeFile();
-    delete[] buffer;
 }
 
 void tst_qfile::seek_data()
@@ -322,12 +335,9 @@ void tst_qfile::seek()
     QFETCH(tst_qfile::BenchmarkType, testType);
     int i = 0;
 
-    createFile();
-    fillFile();
-
     switch (testType) {
         case(QFileBenchmark): {
-            QFile file(filename);
+            QFile file(tempDir.filename);
             file.open(QIODevice::ReadOnly);
             QBENCHMARK {
                 i=(i+1)%sp_size;
@@ -338,7 +348,7 @@ void tst_qfile::seek()
         break;
 #ifdef QT_BUILD_INTERNAL
         case(QFSFileEngineBenchmark): {
-            QFSFileEngine fse(filename);
+            QFSFileEngine fse(tempDir.filename);
             fse.open(QIODevice::ReadOnly | QIODevice::Unbuffered);
             QBENCHMARK {
                 i=(i+1)%sp_size;
@@ -349,7 +359,7 @@ void tst_qfile::seek()
         break;
 #endif
         case(PosixBenchmark): {
-            QByteArray data = filename.toLocal8Bit();
+            QByteArray data = tempDir.filename.toLocal8Bit();
             const char* cfilename = data.constData();
             FILE* cfile = ::fopen(cfilename, "rb");
             QBENCHMARK {
@@ -368,7 +378,7 @@ void tst_qfile::seek()
             HANDLE hndl;
 
             // ensure we don't account string conversion
-            wchar_t* cfilename = (wchar_t*)filename.utf16();
+            const wchar_t *cfilename = reinterpret_cast<const wchar_t *>(tempDir.filename.utf16());
 
             hndl = CreateFile(cfilename, GENERIC_READ, 0, 0, OPEN_EXISTING, 0, 0);
             Q_ASSERT(hndl);
@@ -378,13 +388,11 @@ void tst_qfile::seek()
             }
             CloseHandle(hndl);
 #else
-            QFAIL("Not running on a Windows plattform!");
+            QFAIL("Not running on a Windows platform!");
 #endif
         }
         break;
     }
-
-    removeFile();
 }
 
 void tst_qfile::open_data()
@@ -405,13 +413,11 @@ void tst_qfile::open()
 {
     QFETCH(tst_qfile::BenchmarkType, testType);
 
-    createFile();
-
     switch (testType) {
         case(QFileBenchmark): {
             QBENCHMARK {
-                QFile file( filename );
-                file.open( QIODevice::ReadOnly );
+                QFile file(tempDir.filename);
+                file.open(QIODevice::ReadOnly);
                 file.close();
             }
         }
@@ -419,7 +425,7 @@ void tst_qfile::open()
 #ifdef QT_BUILD_INTERNAL
         case(QFSFileEngineBenchmark): {
             QBENCHMARK {
-                QFSFileEngine fse(filename);
+                QFSFileEngine fse(tempDir.filename);
                 fse.open(QIODevice::ReadOnly | QIODevice::Unbuffered);
                 fse.close();
             }
@@ -428,7 +434,7 @@ void tst_qfile::open()
 #endif
         case(PosixBenchmark): {
             // ensure we don't account toLocal8Bit()
-            QByteArray data = filename.toLocal8Bit();
+            QByteArray data = tempDir.filename.toLocal8Bit();
             const char* cfilename = data.constData();
 
             QBENCHMARK {
@@ -439,7 +445,7 @@ void tst_qfile::open()
         break;
         case(QFileFromPosixBenchmark): {
             // ensure we don't account toLocal8Bit()
-            QByteArray data = filename.toLocal8Bit();
+            QByteArray data = tempDir.filename.toLocal8Bit();
             const char* cfilename = data.constData();
             FILE* cfile = ::fopen(cfilename, "rb");
 
@@ -456,7 +462,7 @@ void tst_qfile::open()
             HANDLE hndl;
 
             // ensure we don't account string conversion
-            wchar_t* cfilename = (wchar_t*)filename.utf16();
+            const wchar_t *cfilename = reinterpret_cast<const wchar_t *>(tempDir.filename.utf16());
 
             QBENCHMARK {
                 hndl = CreateFile(cfilename, GENERIC_READ, 0, 0, OPEN_EXISTING, 0, 0);
@@ -469,8 +475,6 @@ void tst_qfile::open()
         }
         break;
     }
-
-    removeFile();
 }
 
 void tst_qfile::readSmallFiles_QFile_data()
@@ -508,31 +512,6 @@ void tst_qfile::readSmallFiles_Win32_data()
 #endif
 }
 
-void tst_qfile::createSmallFiles()
-{
-    QDir dir = QDir::temp();
-    dir.mkdir("tst");
-    dir.cd("tst");
-    tmpDirName = dir.absolutePath();
-
-    for (int i = 0; i < 1000; ++i) {
-        QFile f(tmpDirName + QLatin1Char('/') + QString::number(i));
-        f.open(QIODevice::WriteOnly);
-        f.seek(511);
-        f.putChar('\n');
-        f.close();
-    }
-}
-
-void tst_qfile::removeSmallFiles()
-{
-    QDirIterator it(tmpDirName, QDirIterator::FollowSymlinks);
-    while (it.hasNext())
-        QFile::remove(it.next());
-    QDir::temp().rmdir("tst");
-}
-
-
 void tst_qfile::readSmallFiles()
 {
     QFETCH(tst_qfile::BenchmarkType, testType);
@@ -540,17 +519,15 @@ void tst_qfile::readSmallFiles()
     QFETCH(QFile::OpenModeFlag, textMode);
     QFETCH(QFile::OpenModeFlag, bufferedMode);
 
-    createSmallFiles();
-
-    QDir dir(tmpDirName);
+    QDir dir(tempDir.path());
     const QStringList files = dir.entryList(QDir::NoDotAndDotDot|QDir::NoSymLinks|QDir::Files);
-    char *buffer = new char[BUFSIZE];
+    char buffer[BUFSIZE];
 
     switch (testType) {
         case(QFileBenchmark): {
             QList<QFile*> fileList;
             Q_FOREACH(QString file, files) {
-                QFile *f = new QFile(tmpDirName + QLatin1Char('/') + file);
+                QFile *f = new QFile(tempDir.filePath(file));
                 f->open(QIODevice::ReadOnly|textMode|bufferedMode);
                 fileList.append(f);
             }
@@ -573,7 +550,7 @@ void tst_qfile::readSmallFiles()
         case(QFSFileEngineBenchmark): {
             QList<QFSFileEngine*> fileList;
             Q_FOREACH(QString file, files) {
-                QFSFileEngine *fse = new QFSFileEngine(tmpDirName + QLatin1Char('/') + file);
+                QFSFileEngine *fse = new QFSFileEngine(tempDir.filePath(file));
                 fse->open(QIODevice::ReadOnly|textMode|bufferedMode);
                 fileList.append(fse);
             }
@@ -594,7 +571,7 @@ void tst_qfile::readSmallFiles()
         case(PosixBenchmark): {
             QList<FILE*> fileList;
             Q_FOREACH(QString file, files) {
-                fileList.append(::fopen(QFile::encodeName(tmpDirName + QLatin1Char('/') + file).constData(), "rb"));
+                fileList.append(::fopen(QFile::encodeName(tempDir.filePath(file)).constData(), "rb"));
             }
 
             QBENCHMARK {
@@ -619,7 +596,7 @@ void tst_qfile::readSmallFiles()
             HANDLE hndl;
 
             // ensure we don't account string conversion
-            wchar_t* cfilename = (wchar_t*)filename.utf16();
+            const wchar_t *cfilename = reinterpret_cast<const wchar_t *>(tempDir.filename.utf16());
 
             hndl = CreateFile(cfilename, GENERIC_READ, 0, 0, OPEN_EXISTING, 0, 0);
             Q_ASSERT(hndl);
@@ -638,9 +615,6 @@ void tst_qfile::readSmallFiles()
         }
         break;
     }
-
-    removeSmallFiles();
-    delete[] buffer;
 }
 
 QTEST_MAIN(tst_qfile)