From ba423261cd9abedda8b732c82371515003d385ce Mon Sep 17 00:00:00 2001 From: Svenn-Arne Dragly Date: Tue, 5 Sep 2017 15:02:20 +0200 Subject: Improve performance in QThreadPool When many runnables are executed, this improves the performance by not resizing the queue for each runnable, which was the case in the previous version, because of many calls to QVector::takeFirst(). Also add a test that makes sure tryTake() is safe to call and does not leave the queue in a bad state that tries to use nullptr entries. Change-Id: I608134ecfa9cfc03db4878dcbd6f9c1107e13e90 Reviewed-by: Lars Knoll --- .../corelib/thread/qthreadpool/tst_qthreadpool.cpp | 64 ++++++++++++++++++++++ 1 file changed, 64 insertions(+) (limited to 'tests/auto/corelib') diff --git a/tests/auto/corelib/thread/qthreadpool/tst_qthreadpool.cpp b/tests/auto/corelib/thread/qthreadpool/tst_qthreadpool.cpp index fdc4ecb5c8..66853a88d8 100644 --- a/tests/auto/corelib/thread/qthreadpool/tst_qthreadpool.cpp +++ b/tests/auto/corelib/thread/qthreadpool/tst_qthreadpool.cpp @@ -93,6 +93,7 @@ private slots: void waitForDoneTimeout(); void destroyingWaitsForTasksToFinish(); void stressTest(); + void takeAllAndIncreaseMaxThreadCount(); private: QMutex m_functionTestMutex; @@ -1199,5 +1200,68 @@ void tst_QThreadPool::stressTest() } } +void tst_QThreadPool::takeAllAndIncreaseMaxThreadCount() { + class Task : public QRunnable + { + public: + Task(QSemaphore *mainBarrier, QSemaphore *threadBarrier) + : m_mainBarrier(mainBarrier) + , m_threadBarrier(threadBarrier) + { + setAutoDelete(false); + } + + void run() { + m_mainBarrier->release(); + m_threadBarrier->acquire(); + } + private: + QSemaphore *m_mainBarrier; + QSemaphore *m_threadBarrier; + }; + + QSemaphore mainBarrier; + QSemaphore taskBarrier; + + QThreadPool threadPool; + threadPool.setMaxThreadCount(1); + + Task *task1 = new Task(&mainBarrier, &taskBarrier); + Task *task2 = new Task(&mainBarrier, &taskBarrier); + Task *task3 = new Task(&mainBarrier, &taskBarrier); + + threadPool.start(task1); + threadPool.start(task2); + threadPool.start(task3); + + mainBarrier.acquire(1); + + QCOMPARE(threadPool.activeThreadCount(), 1); + + QVERIFY(!threadPool.tryTake(task1)); + QVERIFY(threadPool.tryTake(task2)); + QVERIFY(threadPool.tryTake(task3)); + + // A bad queue implementation can segfault here because two consecutive items in the queue + // have been taken + threadPool.setMaxThreadCount(4); + + // Even though we increase the max thread count, there should only be one job to run + QCOMPARE(threadPool.activeThreadCount(), 1); + + // Make sure jobs 2 and 3 never started + QCOMPARE(mainBarrier.available(), 0); + + taskBarrier.release(1); + + threadPool.waitForDone(); + + QCOMPARE(threadPool.activeThreadCount(), 0); + + delete task1; + delete task2; + delete task3; +} + QTEST_MAIN(tst_QThreadPool); #include "tst_qthreadpool.moc" -- cgit v1.2.3 From f62768d046528636789f901ac79e2cfa1843a7b7 Mon Sep 17 00:00:00 2001 From: Thiago Macieira Date: Thu, 21 Sep 2017 13:27:51 -0700 Subject: QUrl: re-fix the setPath("//path") case leading to scheme://path Commits aba336c2b4ad8926dc8a000718bbb7f8a6d5a72d (in Qt 5.2) and aba336c2b4ad8926dc8a000718bbb7f8a6d5a72d (in 5.6) both tried to deal with this problem, with different levels of success. This is the third attempt (and hopefully the charm). Instead of modifying the path that the user provides, go straight ahead and declare it invalid. This is supported by RFC 3986, which declares this expansion impossible: relative-part = "//" authority path-abempty / path-absolute / path-noscheme / path-empty path-abempty = *( "/" segment ) path-absolute = "/" [ segment-nz *( "/" segment ) ] path-noscheme = segment-nz-nc *( "/" segment ) The "path-abempty" and "path-noscheme" cases are the two issues we already handle. This commit adds the third one: path-absolute, which requires that the first segment of the path be of non-zero length. That is, it is now possible again to have http://example.com//path constructed piece-wise, without it producing http://example.com/path. Additionally, it catches the case of http://example.com//path parsed from full URL then followed by setAuthority(""). Change-Id: I69f37f9304f24709a823fffd14e67a5e7212ddcd Reviewed-by: David Faure --- tests/auto/corelib/io/qurl/tst_qurl.cpp | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) (limited to 'tests/auto/corelib') diff --git a/tests/auto/corelib/io/qurl/tst_qurl.cpp b/tests/auto/corelib/io/qurl/tst_qurl.cpp index c5647752fd..1ff85fc6dc 100644 --- a/tests/auto/corelib/io/qurl/tst_qurl.cpp +++ b/tests/auto/corelib/io/qurl/tst_qurl.cpp @@ -2074,6 +2074,14 @@ void tst_QUrl::isValid() QVERIFY(url.errorString().contains("Path component is relative and authority is present")); } + { + QUrl url("http:"); + url.setPath("//example.com"); + QVERIFY(!url.isValid()); + QVERIFY(url.toString().isEmpty()); + QVERIFY(url.errorString().contains("Path component starts with '//' and authority is absent")); + } + { QUrl url; url.setPath("http://example.com"); @@ -3649,12 +3657,12 @@ void tst_QUrl::setComponents_data() QTest::newRow("path-%3A-before-slash") << QUrl() << int(Path) << "c%3A/" << Tolerant << true << PrettyDecoded << "c%3A/" << "c%3A/"; - QTest::newRow("path-doubleslash") << QUrl("trash:/") + QTest::newRow("path-doubleslash") << QUrl("http://example.com") << int(Path) << "//path" << Tolerant << true - << PrettyDecoded << "/path" << "trash:/path"; + << PrettyDecoded << "//path" << "http://example.com//path"; QTest::newRow("path-withdotdot") << QUrl("file:///tmp") << int(Path) << "//tmp/..///root/." << Tolerant << true - << PrettyDecoded << "/tmp/..///root/." << "file:///tmp/..///root/."; + << PrettyDecoded << "//tmp/..///root/." << "file:////tmp/..///root/."; // the other fields can be present and be empty // that is, their delimiters would be present, but there would be nothing to one side @@ -3777,6 +3785,9 @@ void tst_QUrl::setComponents_data() QTest::newRow("invalid-path-2") << QUrl("http://example.com") << int(Path) << "relative" << Strict << false << PrettyDecoded << "relative" << ""; + QTest::newRow("invalid-path-3") << QUrl("trash:/") + << int(Path) << "//path" << Tolerant << false + << PrettyDecoded << "//path" << ""; // -- test bad percent encoding -- // unnecessary to test the scheme, since percent-decoding is not performed in it; -- cgit v1.2.3