From 0f7b68218f395641b59b8530fc3479527edca6e2 Mon Sep 17 00:00:00 2001 From: Sona Kurazyan Date: Thu, 10 Jun 2021 17:38:27 +0200 Subject: [PATCH] Remove the dead code for blocking methods from QtConcurrent MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit After 79fd1cb2c631b6084bf10874205d27f5b53c907a the methods for running QtConcurrent algorithms in the blocking mode aren't used anymore. Since ThreadEngineBase and ThreadEngineStarter classes aren't meant to be used externally, it should be fine to remove startBlocking() methods now. Removed the unused code and adjusted the tests accordingly. Change-Id: Ifb13820ce207869d6f720bcb5be8d35bb355fe33 Reviewed-by: Jarek Kobus Reviewed-by: Mårten Nordheim Reviewed-by: Andrei Golubev (cherry picked from commit 1bf75f2a661c05c7f1126187310d7df3f9704af5) --- src/concurrent/qtconcurrentthreadengine.cpp | 33 ----- src/concurrent/qtconcurrentthreadengine.h | 23 ---- .../tst_qtconcurrentiteratekernel.cpp | 23 +++- .../tst_qtconcurrentthreadengine.cpp | 124 ++---------------- 4 files changed, 29 insertions(+), 174 deletions(-) diff --git a/src/concurrent/qtconcurrentthreadengine.cpp b/src/concurrent/qtconcurrentthreadengine.cpp index fcc504a96cc..399afb61953 100644 --- a/src/concurrent/qtconcurrentthreadengine.cpp +++ b/src/concurrent/qtconcurrentthreadengine.cpp @@ -176,39 +176,6 @@ void ThreadEngineBase::startSingleThreaded() finish(); } -void ThreadEngineBase::startBlocking() -{ - start(); - barrier.acquire(); - startThreads(); - - bool throttled = false; -#ifndef QT_NO_EXCEPTIONS - try { -#endif - while (threadFunction() == ThrottleThread) { - if (threadThrottleExit()) { - throttled = true; - break; - } - } -#ifndef QT_NO_EXCEPTIONS - } catch (QException &e) { - handleException(e); - } catch (...) { - handleException(QUnhandledException(std::current_exception())); - } -#endif - - if (throttled == false) { - barrier.release(); - } - - barrier.wait(); - finish(); - exceptionStore.throwPossibleException(); -} - void ThreadEngineBase::startThread() { startThreadInternal(); diff --git a/src/concurrent/qtconcurrentthreadengine.h b/src/concurrent/qtconcurrentthreadengine.h index 4cd5b852107..5a78377e71a 100644 --- a/src/concurrent/qtconcurrentthreadengine.h +++ b/src/concurrent/qtconcurrentthreadengine.h @@ -91,7 +91,6 @@ public: ThreadEngineBase(QThreadPool *pool); virtual ~ThreadEngineBase(); void startSingleThreaded(); - void startBlocking(); void startThread(); bool isCanceled(); void waitForResume(); @@ -153,15 +152,6 @@ public: return result(); } - // Runs the user algorithm using multiple threads. - // This function blocks until the algorithm is finished, - // and then returns the result. - T *startBlocking() - { - ThreadEngineBase::startBlocking(); - return result(); - } - // Runs the user algorithm using multiple threads. // Does not block, returns a future. QFuture startAsynchronously() @@ -242,13 +232,6 @@ class ThreadEngineStarter : public ThreadEngineStarterBase public: ThreadEngineStarter(TypedThreadEngine *eng) : Base(eng) { } - - T startBlocking() - { - T t = *this->threadEngine->startBlocking(); - delete this->threadEngine; - return t; - } }; // Full template specialization where T is void. @@ -258,12 +241,6 @@ class ThreadEngineStarter : public ThreadEngineStarterBase public: ThreadEngineStarter(ThreadEngine *_threadEngine) : ThreadEngineStarterBase(_threadEngine) {} - - void startBlocking() - { - this->threadEngine->startBlocking(); - delete this->threadEngine; - } }; //! [qtconcurrentthreadengine-1] diff --git a/tests/auto/concurrent/qtconcurrentiteratekernel/tst_qtconcurrentiteratekernel.cpp b/tests/auto/concurrent/qtconcurrentiteratekernel/tst_qtconcurrentiteratekernel.cpp index 7fba6ed5801..4163883a5b7 100644 --- a/tests/auto/concurrent/qtconcurrentiteratekernel/tst_qtconcurrentiteratekernel.cpp +++ b/tests/auto/concurrent/qtconcurrentiteratekernel/tst_qtconcurrentiteratekernel.cpp @@ -126,7 +126,8 @@ public: void tst_QtConcurrentIterateKernel::instantiate() { - startThreadEngine(new PrintFor(0, 40)).startBlocking(); + auto future = startThreadEngine(new PrintFor(0, 40)).startAsynchronously(); + future.waitForFinished(); QCOMPARE(iterations.loadRelaxed(), 40); } @@ -165,8 +166,10 @@ void tst_QtConcurrentIterateKernel::stresstest() const int times = 50; for (int i = 0; i < times; ++i) { counter.storeRelaxed(0); - CountFor f(0, iterations); - f.startBlocking(); + // ThreadEngine will delete f when it finishes + auto f = new CountFor(0, iterations); + auto future = f->startAsynchronously(); + future.waitForFinished(); QCOMPARE(counter.loadRelaxed(), iterations); } } @@ -174,8 +177,12 @@ void tst_QtConcurrentIterateKernel::stresstest() void tst_QtConcurrentIterateKernel::noIterations() { const int times = 20000; - for (int i = 0; i < times; ++i) - startThreadEngine(new IterateKernel(QThreadPool::globalInstance(), 0, 0)).startBlocking(); + for (int i = 0; i < times; ++i) { + auto future = startThreadEngine(new IterateKernel( + QThreadPool::globalInstance(), 0, 0)) + .startAsynchronously(); + future.waitForFinished(); + } } QMutex threadsMutex; @@ -230,8 +237,10 @@ void tst_QtConcurrentIterateKernel::throttling() threads.clear(); - ThrottleFor f(0, totalIterations); - f.startBlocking(); + // ThreadEngine will delete f when it finishes + auto f = new ThrottleFor(0, totalIterations); + auto future = f->startAsynchronously(); + future.waitForFinished(); QCOMPARE(iterations.loadRelaxed(), totalIterations); diff --git a/tests/auto/concurrent/qtconcurrentthreadengine/tst_qtconcurrentthreadengine.cpp b/tests/auto/concurrent/qtconcurrentthreadengine/tst_qtconcurrentthreadengine.cpp index c365ec48370..85f83c61f76 100644 --- a/tests/auto/concurrent/qtconcurrentthreadengine/tst_qtconcurrentthreadengine.cpp +++ b/tests/auto/concurrent/qtconcurrentthreadengine/tst_qtconcurrentthreadengine.cpp @@ -66,16 +66,9 @@ public: void tst_QtConcurrentThreadEngine::runDirectly() { - { - PrintUser engine; - engine.startSingleThreaded(); - engine.startBlocking(); - } - { - PrintUser *engine = new PrintUser(); - QFuture f = engine->startAsynchronously(); - f.waitForFinished(); - } + PrintUser *engine = new PrintUser(); + QFuture f = engine->startAsynchronously(); + f.waitForFinished(); } class StringResultUser : public ThreadEngine @@ -108,8 +101,10 @@ public: void tst_QtConcurrentThreadEngine::result() { - StringResultUser engine; - QCOMPARE(*engine.startBlocking(), QString("Foo")); + // ThreadEngine will delete 'engine' when it finishes + auto engine = new StringResultUser(); + auto future = engine->startAsynchronously(); + QCOMPARE(future.result(), QString("Foo")); } class VoidResultUser : public ThreadEngine @@ -137,17 +132,9 @@ public: void tst_QtConcurrentThreadEngine::runThroughStarter() { - { - ThreadEngineStarter starter = startThreadEngine(new StringResultUser()); - QFuture f = starter.startAsynchronously(); - QCOMPARE(f.result(), QString("Foo")); - } - - { - ThreadEngineStarter starter = startThreadEngine(new StringResultUser()); - QString str = starter.startBlocking(); - QCOMPARE(str, QString("Foo")); - } + ThreadEngineStarter starter = startThreadEngine(new StringResultUser()); + QFuture f = starter.startAsynchronously(); + QCOMPARE(f.result(), QString("Foo")); } class CancelUser : public ThreadEngine @@ -233,12 +220,6 @@ void tst_QtConcurrentThreadEngine::throttle() f.waitForFinished(); QCOMPARE(count.loadRelaxed(), 0); } - - for (int i = 0; i < repeats; ++i) { - ThrottleAlwaysUser t; - t.startBlocking(); - QCOMPARE(count.loadRelaxed(), 0); - } } QSet threads; @@ -278,17 +259,9 @@ void tst_QtConcurrentThreadEngine::threadCount() const int repeats = 10; for (int i = 0; i < repeats; ++i) { - ThreadCountUser t; - t.startBlocking(); - int count = threads.count(); - int count_expected = QThreadPool::globalInstance()->maxThreadCount() + 1; // +1 for the main thread. - if (count != count_expected) - QEXPECT_FAIL("", "QTBUG-23333", Abort); - QCOMPARE(count, count_expected); - (new ThreadCountUser())->startAsynchronously().waitForFinished(); - count = threads.count(); - count_expected = QThreadPool::globalInstance()->maxThreadCount(); + const auto count = threads.count(); + const auto count_expected = QThreadPool::globalInstance()->maxThreadCount(); if (count != count_expected) QEXPECT_FAIL("", "QTBUG-23333", Abort); QCOMPARE(count, count_expected); @@ -296,15 +269,8 @@ void tst_QtConcurrentThreadEngine::threadCount() // Set the finish flag immediately, this should give us one thread only. for (int i = 0; i < repeats; ++i) { - ThreadCountUser t(true /*finishImmediately*/); - t.startBlocking(); - int count = threads.count(); - if (count != 1) - QEXPECT_FAIL("", "QTBUG-23333", Abort); - QCOMPARE(count, 1); - (new ThreadCountUser(true /*finishImmediately*/))->startAsynchronously().waitForFinished(); - count = threads.count(); + const auto count = threads.count(); if (count != 1) QEXPECT_FAIL("", "QTBUG-23333", Abort); QCOMPARE(count, 1); @@ -450,7 +416,6 @@ public: void tst_QtConcurrentThreadEngine::exceptions() { - // Asynchronous mode: { bool caught = false; try { @@ -463,32 +428,6 @@ void tst_QtConcurrentThreadEngine::exceptions() QVERIFY2(caught, "did not get exception"); } - // Blocking mode: - // test throwing the exception from a worker thread. - { - bool caught = false; - try { - QtConcurrentExceptionThrower e(QThread::currentThread()); - e.startBlocking(); - } catch (const QException &) { - caught = true; - } - QVERIFY2(caught, "did not get exception"); - } - - // test throwing the exception from the main thread (different code path) - { - bool caught = false; - try { - QtConcurrentExceptionThrower e(nullptr); - e.startBlocking(); - } catch (const QException &) { - caught = true; - } - QVERIFY2(caught, "did not get exception"); - } - - // Asynchronous mode: { bool caught = false; try { @@ -506,43 +445,6 @@ void tst_QtConcurrentThreadEngine::exceptions() } QVERIFY2(caught, "did not get exception"); } - - // Blocking mode: - // test throwing the exception from a worker thread. - { - bool caught = false; - try { - IntExceptionThrower e(QThread::currentThread()); - e.startBlocking(); - } catch (const QUnhandledException &ex) { - // Make sure the exception info is not lost - try { - if (ex.exception()) - std::rethrow_exception(ex.exception()); - } catch (int) { - caught = true; - } - } - QVERIFY2(caught, "did not get exception"); - } - - // test throwing the exception from the main thread (different code path) - { - bool caught = false; - try { - IntExceptionThrower e(nullptr); - e.startBlocking(); - } catch (const QUnhandledException &ex) { - // Make sure the exception info is not lost - try { - if (ex.exception()) - std::rethrow_exception(ex.exception()); - } catch (int) { - caught = true; - } - } - QVERIFY2(caught, "did not get exception"); - } } #endif