From 7bbe79fe5f54ed7542d935600036d3c8b401505f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Abecasis?= Date: Fri, 17 Feb 2012 20:09:17 +0100 Subject: Drop file-engine abstraction from public API This abstraction imposed serious performance penalties and is being dropped from the public API. In particular, by allowing file names to be arbitrarily hijacked by different file engines, and requiring engines to be instantiated in order to decide, it imposed unnecessary overhead on all file operations. Another flaw in the design with direct impact on performance is how engines have no way to provide (or retain) additional information obtained when querying the filesystem. In many places this has meant repeated operations on the file system, where useful information is immediately discarded to be queried again subsequently. For Qt 4.8 a major refactoring of the code base took place to allow bypassing the file-engine abstraction in select places, with considerable performance gains observed. In Qt 5 it is expected we'll be able to take this further, reaping even more benefits, but the abstraction has to go. [Dropping this now does not preclude that virtual file systems make an appearance in Qt at a later point in Qt 5's lifecycle. Hopefully with a new and improved abstraction.] Forward declarations for QFileExtension(Result) were dropped, as the classes were never used or defined. Tests using "internalized" classes will only fully run on developer builds. QFSFileEngine was removed altogether from exception safety test, as it isn't its intent to test internal API. Change-Id: Ie910e6c2628be202ea9e05366b091d6d529b246b Reviewed-by: Thiago Macieira Reviewed-by: Lars Knoll --- tests/auto/corelib/io/io.pro | 1 + .../io/qabstractfileengine/qabstractfileengine.pro | 2 +- .../tst_qabstractfileengine.cpp | 4 +- tests/auto/corelib/io/qdir/qdir.pro | 2 +- tests/auto/corelib/io/qdir/tst_qdir.cpp | 3 ++ .../auto/corelib/io/qdiriterator/qdiriterator.pro | 2 +- .../corelib/io/qdiriterator/tst_qdiriterator.cpp | 8 ++++ tests/auto/corelib/io/qfile/test/test.pro | 2 +- tests/auto/corelib/io/qfile/tst_qfile.cpp | 18 +++++++-- .../tst_exceptionsafety_objects.cpp | 1 - tests/benchmarks/corelib/io/qfile/main.cpp | 46 ++++++++++++++++++++-- tests/benchmarks/corelib/io/qfile/qfile.pro | 2 +- 12 files changed, 75 insertions(+), 16 deletions(-) (limited to 'tests') diff --git a/tests/auto/corelib/io/io.pro b/tests/auto/corelib/io/io.pro index 095aa7a77d..84a885f5b6 100644 --- a/tests/auto/corelib/io/io.pro +++ b/tests/auto/corelib/io/io.pro @@ -30,6 +30,7 @@ SUBDIRS=\ } !contains(QT_CONFIG, private_tests): SUBDIRS -= \ + qabstractfileengine \ qfileinfo win32:!contains(QT_CONFIG, private_tests): SUBDIRS -= \ diff --git a/tests/auto/corelib/io/qabstractfileengine/qabstractfileengine.pro b/tests/auto/corelib/io/qabstractfileengine/qabstractfileengine.pro index d7565b5429..641bb7341b 100644 --- a/tests/auto/corelib/io/qabstractfileengine/qabstractfileengine.pro +++ b/tests/auto/corelib/io/qabstractfileengine/qabstractfileengine.pro @@ -1,5 +1,5 @@ CONFIG += testcase TARGET = tst_qabstractfileengine -QT = core testlib +QT = core-private core testlib SOURCES = tst_qabstractfileengine.cpp RESOURCES += qabstractfileengine.qrc diff --git a/tests/auto/corelib/io/qabstractfileengine/tst_qabstractfileengine.cpp b/tests/auto/corelib/io/qabstractfileengine/tst_qabstractfileengine.cpp index 776ad4d0a7..c6ccc9308a 100644 --- a/tests/auto/corelib/io/qabstractfileengine/tst_qabstractfileengine.cpp +++ b/tests/auto/corelib/io/qabstractfileengine/tst_qabstractfileengine.cpp @@ -39,8 +39,8 @@ ** ****************************************************************************/ -#include -#include +#include +#include #include #include diff --git a/tests/auto/corelib/io/qdir/qdir.pro b/tests/auto/corelib/io/qdir/qdir.pro index 14f2d8812a..94ee14b51e 100644 --- a/tests/auto/corelib/io/qdir/qdir.pro +++ b/tests/auto/corelib/io/qdir/qdir.pro @@ -1,6 +1,6 @@ CONFIG += testcase parallel_test TARGET = tst_qdir -QT = core testlib +QT = core core-private testlib SOURCES = tst_qdir.cpp RESOURCES += qdir.qrc diff --git a/tests/auto/corelib/io/qdir/tst_qdir.cpp b/tests/auto/corelib/io/qdir/tst_qdir.cpp index 6a48d7e60f..04967d8313 100644 --- a/tests/auto/corelib/io/qdir/tst_qdir.cpp +++ b/tests/auto/corelib/io/qdir/tst_qdir.cpp @@ -49,6 +49,7 @@ #include #if (defined(Q_OS_WIN) && !defined(Q_OS_WINCE)) +#include #include "../../../network-settings.h" #endif @@ -471,6 +472,7 @@ void tst_QDir::exists_data() #if (defined(Q_OS_WIN) && !defined(Q_OS_WINCE)) QTest::newRow("This drive should exist") << "C:/" << true; // find a non-existing drive and check if it does not exist +#ifdef QT_BUILD_INTERNAL QFileInfoList drives = QFSFileEngine::drives(); QStringList driveLetters; for (int i = 0; i < drives.count(); ++i) { @@ -487,6 +489,7 @@ void tst_QDir::exists_data() QTest::newRow("This drive should not exist") << driv << false; } #endif +#endif } void tst_QDir::exists() diff --git a/tests/auto/corelib/io/qdiriterator/qdiriterator.pro b/tests/auto/corelib/io/qdiriterator/qdiriterator.pro index cfb3201c43..ef59fc48e3 100644 --- a/tests/auto/corelib/io/qdiriterator/qdiriterator.pro +++ b/tests/auto/corelib/io/qdiriterator/qdiriterator.pro @@ -1,6 +1,6 @@ CONFIG += testcase parallel_test TARGET = tst_qdiriterator -QT = core testlib +QT = core-private core testlib SOURCES = tst_qdiriterator.cpp RESOURCES += qdiriterator.qrc diff --git a/tests/auto/corelib/io/qdiriterator/tst_qdiriterator.cpp b/tests/auto/corelib/io/qdiriterator/tst_qdiriterator.cpp index d5d490dc29..37d3e1ab68 100644 --- a/tests/auto/corelib/io/qdiriterator/tst_qdiriterator.cpp +++ b/tests/auto/corelib/io/qdiriterator/tst_qdiriterator.cpp @@ -48,6 +48,8 @@ #include #include +#include + #if defined(Q_OS_VXWORKS) #define Q_NO_SYMLINKS #endif @@ -454,6 +456,7 @@ void tst_QDirIterator::stopLinkLoop() // The goal of this test is only to ensure that the test above don't malfunction } +#ifdef QT_BUILD_INTERNAL class EngineWithNoIterator : public QFSFileEngine { public: @@ -473,13 +476,18 @@ public: return new EngineWithNoIterator(fileName); } }; +#endif void tst_QDirIterator::engineWithNoIterator() { +#ifdef QT_BUILD_INTERNAL EngineWithNoIteratorHandler handler; QDir("entrylist").entryList(); QVERIFY(true); // test that the above line doesn't crash +#else + QSKIP("This test requires -developer-build."); +#endif } void tst_QDirIterator::absoluteFilePathsFromRelativeIteratorPath() diff --git a/tests/auto/corelib/io/qfile/test/test.pro b/tests/auto/corelib/io/qfile/test/test.pro index 2611ff39bd..dab5e4a3a5 100644 --- a/tests/auto/corelib/io/qfile/test/test.pro +++ b/tests/auto/corelib/io/qfile/test/test.pro @@ -1,5 +1,5 @@ CONFIG += testcase -QT = core network testlib +QT = core-private core network testlib TARGET = ../tst_qfile SOURCES = ../tst_qfile.cpp RESOURCES += ../qfile.qrc ../rename-fallback.qrc ../copy-fallback.qrc diff --git a/tests/auto/corelib/io/qfile/tst_qfile.cpp b/tests/auto/corelib/io/qfile/tst_qfile.cpp index d1a0debf4d..87d2cf83f7 100644 --- a/tests/auto/corelib/io/qfile/tst_qfile.cpp +++ b/tests/auto/corelib/io/qfile/tst_qfile.cpp @@ -42,13 +42,15 @@ #include #include -#include -#include #include #include #include #include #include + +#include +#include + #if !defined(Q_OS_WINCE) #include #endif @@ -70,7 +72,6 @@ # include #elif defined(Q_OS_WINCE) # include -# include #endif #include @@ -1943,6 +1944,7 @@ void tst_QFile::longFileName() QVERIFY(QFile::remove(newName)); } +#ifdef QT_BUILD_INTERNAL class MyEngine : public QAbstractFileEngine { public: @@ -1998,6 +2000,7 @@ public: return new MyEngine(2); } }; +#endif void tst_QFile::fileEngineHandler() { @@ -2006,6 +2009,7 @@ void tst_QFile::fileEngineHandler() QFile file("ole.bull"); QCOMPARE(file.size(), qint64(0)); +#ifdef QT_BUILD_INTERNAL // Instantiating our handler will enable the new engine. MyHandler handler; file.setFileName("ole.bull"); @@ -2015,9 +2019,10 @@ void tst_QFile::fileEngineHandler() MyHandler2 handler2; file.setFileName("ole.bull"); QCOMPARE(file.size(), qint64(125)); - +#endif } +#ifdef QT_BUILD_INTERNAL class MyRecursiveHandler : public QAbstractFileEngineHandler { public: @@ -2032,13 +2037,18 @@ public: return 0; } }; +#endif void tst_QFile::useQFileInAFileHandler() { +#ifdef QT_BUILD_INTERNAL // This test should not dead-lock MyRecursiveHandler handler; QFile file(":!tst_qfile.cpp"); QVERIFY(file.exists()); +#else + QSKIP("This test requires -developer-build."); +#endif } void tst_QFile::getCharFF() diff --git a/tests/auto/other/exceptionsafety_objects/tst_exceptionsafety_objects.cpp b/tests/auto/other/exceptionsafety_objects/tst_exceptionsafety_objects.cpp index 71a70384fc..a426a90976 100644 --- a/tests/auto/other/exceptionsafety_objects/tst_exceptionsafety_objects.cpp +++ b/tests/auto/other/exceptionsafety_objects/tst_exceptionsafety_objects.cpp @@ -166,7 +166,6 @@ void tst_ExceptionSafety_Objects::objects_data() NEWROW(QObject); NEWROW(QBuffer); NEWROW(QFile); - NEWROW(QFSFileEngine); NEWROW(QProcess); NEWROW(QSettings); NEWROW(QThread); diff --git a/tests/benchmarks/corelib/io/qfile/main.cpp b/tests/benchmarks/corelib/io/qfile/main.cpp index 3d16921b7f..02922e0bbd 100644 --- a/tests/benchmarks/corelib/io/qfile/main.cpp +++ b/tests/benchmarks/corelib/io/qfile/main.cpp @@ -41,10 +41,11 @@ #include #include -#include #include #include +#include + #include #include @@ -79,7 +80,9 @@ Q_OBJECT public: enum BenchmarkType { QFileBenchmark = 1, +#ifdef QT_BUILD_INTERNAL QFSFileEngineBenchmark, +#endif Win32Benchmark, PosixBenchmark, QFileFromPosixBenchmark @@ -173,7 +176,14 @@ void tst_qfile::cleanupTestCase() } void tst_qfile::readBigFile_QFile() { readBigFile(); } -void tst_qfile::readBigFile_QFSFileEngine() { readBigFile(); } +void tst_qfile::readBigFile_QFSFileEngine() +{ +#ifdef QT_BUILD_INTERNAL + readBigFile(); +#else + QSKIP("This test requires -developer-build."); +#endif +} void tst_qfile::readBigFile_posix() { readBigFile(); @@ -191,10 +201,15 @@ void tst_qfile::readBigFile_QFile_data() void tst_qfile::readBigFile_QFSFileEngine_data() { +#ifdef QT_BUILD_INTERNAL readBigFile_data(QFSFileEngineBenchmark, QIODevice::NotOpen, QIODevice::NotOpen); readBigFile_data(QFSFileEngineBenchmark, QIODevice::NotOpen, QIODevice::Unbuffered); readBigFile_data(QFSFileEngineBenchmark, QIODevice::Text, QIODevice::NotOpen); readBigFile_data(QFSFileEngineBenchmark, QIODevice::Text, QIODevice::Unbuffered); +#else + QTest::addColumn("dummy"); + QTest::newRow("Test will be skipped") << -1; +#endif } void tst_qfile::readBigFile_posix_data() @@ -255,6 +270,7 @@ void tst_qfile::readBigFile() file.close(); } break; +#ifdef QT_BUILD_INTERNAL case(QFSFileEngineBenchmark): { QFSFileEngine fse(filename); fse.open(QIODevice::ReadOnly|textMode|bufferedMode); @@ -266,6 +282,7 @@ void tst_qfile::readBigFile() fse.close(); } break; +#endif case(PosixBenchmark): { QByteArray data = filename.toLocal8Bit(); const char* cfilename = data.constData(); @@ -317,7 +334,9 @@ void tst_qfile::seek_data() { QTest::addColumn("testType"); QTest::newRow("QFile") << QFileBenchmark; +#ifdef QT_BUILD_INTERNAL QTest::newRow("QFSFileEngine") << QFSFileEngineBenchmark; +#endif QTest::newRow("Posix FILE*") << PosixBenchmark; #ifdef Q_OS_WIN QTest::newRow("Win32 API") << Win32Benchmark; @@ -343,6 +362,7 @@ void tst_qfile::seek() file.close(); } break; +#ifdef QT_BUILD_INTERNAL case(QFSFileEngineBenchmark): { QFSFileEngine fse(filename); fse.open(QIODevice::ReadOnly); @@ -353,6 +373,7 @@ void tst_qfile::seek() fse.close(); } break; +#endif case(PosixBenchmark): { QByteArray data = filename.toLocal8Bit(); const char* cfilename = data.constData(); @@ -396,7 +417,9 @@ void tst_qfile::open_data() { QTest::addColumn("testType"); QTest::newRow("QFile") << QFileBenchmark; +#ifdef QT_BUILD_INTERNAL QTest::newRow("QFSFileEngine") << QFSFileEngineBenchmark; +#endif QTest::newRow("Posix FILE*") << PosixBenchmark; QTest::newRow("QFile from FILE*") << QFileFromPosixBenchmark; #ifdef Q_OS_WIN @@ -419,6 +442,7 @@ void tst_qfile::open() } } break; +#ifdef QT_BUILD_INTERNAL case(QFSFileEngineBenchmark): { QBENCHMARK { QFSFileEngine fse(filename); @@ -427,7 +451,7 @@ void tst_qfile::open() } } break; - +#endif case(PosixBenchmark): { // ensure we don't account toLocal8Bit() QByteArray data = filename.toLocal8Bit(); @@ -477,7 +501,14 @@ void tst_qfile::open() void tst_qfile::readSmallFiles_QFile() { readSmallFiles(); } -void tst_qfile::readSmallFiles_QFSFileEngine() { readSmallFiles(); } +void tst_qfile::readSmallFiles_QFSFileEngine() +{ +#ifdef QT_BUILD_INTERNAL + readSmallFiles(); +#else + QSKIP("This test requires -developer-build."); +#endif +} void tst_qfile::readSmallFiles_posix() { readSmallFiles(); @@ -498,10 +529,15 @@ void tst_qfile::readSmallFiles_QFile_data() void tst_qfile::readSmallFiles_QFSFileEngine_data() { +#ifdef QT_BUILD_INTERNAL readSmallFiles_data(QFSFileEngineBenchmark, QIODevice::NotOpen, QIODevice::NotOpen); readSmallFiles_data(QFSFileEngineBenchmark, QIODevice::NotOpen, QIODevice::Unbuffered); readSmallFiles_data(QFSFileEngineBenchmark, QIODevice::Text, QIODevice::NotOpen); readSmallFiles_data(QFSFileEngineBenchmark, QIODevice::Text, QIODevice::Unbuffered); +#else + QTest::addColumn("dummy"); + QTest::newRow("Test will be skipped") << -1; +#endif } void tst_qfile::readSmallFiles_posix_data() @@ -606,6 +642,7 @@ void tst_qfile::readSmallFiles() } } break; +#ifdef QT_BUILD_INTERNAL case(QFSFileEngineBenchmark): { QList fileList; Q_FOREACH(QString file, files) { @@ -626,6 +663,7 @@ void tst_qfile::readSmallFiles() } } break; +#endif case(PosixBenchmark): { QList fileList; Q_FOREACH(QString file, files) { diff --git a/tests/benchmarks/corelib/io/qfile/qfile.pro b/tests/benchmarks/corelib/io/qfile/qfile.pro index 933469071d..5f7b9af73f 100644 --- a/tests/benchmarks/corelib/io/qfile/qfile.pro +++ b/tests/benchmarks/corelib/io/qfile/qfile.pro @@ -1,6 +1,6 @@ TEMPLATE = app TARGET = tst_bench_qfile -QT = core testlib +QT = core core-private testlib win32: DEFINES+= _CRT_SECURE_NO_WARNINGS SOURCES += main.cpp -- cgit v1.2.3