From 9462c7a3e90883abc0bcee9b3c9a8d4e80c5b15d Mon Sep 17 00:00:00 2001 From: Marco Bubke Date: Sat, 26 Aug 2023 16:56:13 +0200 Subject: [PATCH] QmlDesigner: Add task queue To share the code TaskQueue is introduced. The Thread and the synchronization code is moved into TaskQueue. Only a dispatch and cean must be implemented in the callback. Change-Id: I19c85c891ef700aae85b54630714555e862200a4 Reviewed-by: Tim Jenssen Reviewed-by: Qt CI Patch Build Bot --- src/plugins/qmldesigner/CMakeLists.txt | 1 + .../asynchronousexplicitimagecache.cpp | 128 ++------------- .../imagecache/asynchronousimagecache.cpp | 150 +++--------------- .../imagecache/asynchronousimagefactory.cpp | 110 +------------ .../imagecache/asynchronousimagefactory.h | 53 ++++--- .../designercore/imagecache/taskqueue.h | 133 ++++++++++++++++ .../include/asynchronousexplicitimagecache.h | 39 +++-- .../include/asynchronousimagecache.h | 47 +++--- .../tests/testdesignercore/CMakeLists.txt | 1 + .../tests/unittests/imagecache/CMakeLists.txt | 1 + .../unittests/imagecache/taskqueue-test.cpp | 103 ++++++++++++ 11 files changed, 365 insertions(+), 401 deletions(-) create mode 100644 src/plugins/qmldesigner/designercore/imagecache/taskqueue.h create mode 100644 tests/unit/tests/unittests/imagecache/taskqueue-test.cpp diff --git a/src/plugins/qmldesigner/CMakeLists.txt b/src/plugins/qmldesigner/CMakeLists.txt index 8939fa7f174..689c6bce9b0 100644 --- a/src/plugins/qmldesigner/CMakeLists.txt +++ b/src/plugins/qmldesigner/CMakeLists.txt @@ -176,6 +176,7 @@ extend_qtc_library(QmlDesignerCore meshimagecachecollector.cpp meshimagecachecollector.h synchronousimagecache.cpp + taskqueue.h textureimagecachecollector.cpp textureimagecachecollector.h timestampprovider.cpp diff --git a/src/plugins/qmldesigner/designercore/imagecache/asynchronousexplicitimagecache.cpp b/src/plugins/qmldesigner/designercore/imagecache/asynchronousexplicitimagecache.cpp index 527ef437b53..70c0bced0e3 100644 --- a/src/plugins/qmldesigner/designercore/imagecache/asynchronousexplicitimagecache.cpp +++ b/src/plugins/qmldesigner/designercore/imagecache/asynchronousexplicitimagecache.cpp @@ -17,8 +17,6 @@ AsynchronousExplicitImageCache::AsynchronousExplicitImageCache(ImageCacheStorage AsynchronousExplicitImageCache::~AsynchronousExplicitImageCache() { - clean(); - wait(); } void AsynchronousExplicitImageCache::request(Utils::SmallStringView name, @@ -57,21 +55,16 @@ void AsynchronousExplicitImageCache::request(Utils::SmallStringView name, } } -void AsynchronousExplicitImageCache::wait() -{ - stopThread(); - m_condition.notify_all(); - if (m_backgroundThread.joinable()) - m_backgroundThread.join(); -} - void AsynchronousExplicitImageCache::requestImage(Utils::SmallStringView name, ImageCache::CaptureImageCallback captureCallback, ImageCache::AbortCallback abortCallback, Utils::SmallStringView extraId) { - addEntry(name, extraId, std::move(captureCallback), std::move(abortCallback), RequestType::Image); - m_condition.notify_all(); + m_taskQueue.addTask(name, + extraId, + std::move(captureCallback), + std::move(abortCallback), + RequestType::Image); } void AsynchronousExplicitImageCache::requestMidSizeImage(Utils::SmallStringView name, @@ -79,8 +72,11 @@ void AsynchronousExplicitImageCache::requestMidSizeImage(Utils::SmallStringView ImageCache::AbortCallback abortCallback, Utils::SmallStringView extraId) { - addEntry(name, extraId, std::move(captureCallback), std::move(abortCallback), RequestType::MidSizeImage); - m_condition.notify_all(); + m_taskQueue.addTask(name, + extraId, + std::move(captureCallback), + std::move(abortCallback), + RequestType::MidSizeImage); } void AsynchronousExplicitImageCache::requestSmallImage(Utils::SmallStringView name, @@ -88,108 +84,16 @@ void AsynchronousExplicitImageCache::requestSmallImage(Utils::SmallStringView na ImageCache::AbortCallback abortCallback, Utils::SmallStringView extraId) { - addEntry(name, extraId, std::move(captureCallback), std::move(abortCallback), RequestType::SmallImage); - m_condition.notify_all(); + m_taskQueue.addTask(name, + extraId, + std::move(captureCallback), + std::move(abortCallback), + RequestType::SmallImage); } void AsynchronousExplicitImageCache::clean() { - clearEntries(); -} - -std::optional AsynchronousExplicitImageCache::getEntry( - std::unique_lock lock) -{ - auto l = std::move(lock); - - if (m_requestEntries.empty()) - return {}; - - RequestEntry entry = m_requestEntries.front(); - m_requestEntries.pop_front(); - - return {entry}; -} - -void AsynchronousExplicitImageCache::addEntry(Utils::PathString &&name, - Utils::SmallString &&extraId, - ImageCache::CaptureImageCallback &&captureCallback, - ImageCache::AbortCallback &&abortCallback, - RequestType requestType) -{ - std::unique_lock lock{m_mutex}; - - ensureThreadIsRunning(); - - m_requestEntries.emplace_back(std::move(name), - std::move(extraId), - std::move(captureCallback), - std::move(abortCallback), - requestType); -} - -void AsynchronousExplicitImageCache::clearEntries() -{ - std::unique_lock lock{m_mutex}; - for (RequestEntry &entry : m_requestEntries) - entry.abortCallback(ImageCache::AbortReason::Abort); - m_requestEntries.clear(); -} - -std::tuple, bool> AsynchronousExplicitImageCache::waitForEntries() -{ - using namespace std::literals::chrono_literals; - std::unique_lock lock{m_mutex}; - if (m_finishing) - return {std::move(lock), true}; - if (m_requestEntries.empty()) { - auto timedOutWithoutEntriesOrFinishing = !m_condition.wait_for(lock, 10min, [&] { - return m_requestEntries.size() || m_finishing; - }); - - if (timedOutWithoutEntriesOrFinishing || m_finishing) { - m_sleeping = true; - return {std::move(lock), true}; - } - } - return {std::move(lock), false}; -} - -void AsynchronousExplicitImageCache::stopThread() -{ - std::unique_lock lock{m_mutex}; - m_finishing = true; -} - -void AsynchronousExplicitImageCache::ensureThreadIsRunning() -{ - if (m_finishing) - return; - - if (!m_sleeping) - return; - - if (m_backgroundThread.joinable()) - m_backgroundThread.join(); - - m_sleeping = false; - - m_backgroundThread = std::thread{[this] { - while (true) { - auto [lock, abort] = waitForEntries(); - if (abort) - return; - - if (auto entry = getEntry(std::move(lock)); entry) { - request(entry->name, - entry->extraId, - entry->requestType, - std::move(entry->captureCallback), - std::move(entry->abortCallback), - m_storage); - } - } - }}; + m_taskQueue.clean(); } } // namespace QmlDesigner diff --git a/src/plugins/qmldesigner/designercore/imagecache/asynchronousimagecache.cpp b/src/plugins/qmldesigner/designercore/imagecache/asynchronousimagecache.cpp index e40d856e532..a18e051c364 100644 --- a/src/plugins/qmldesigner/designercore/imagecache/asynchronousimagecache.cpp +++ b/src/plugins/qmldesigner/designercore/imagecache/asynchronousimagecache.cpp @@ -22,8 +22,7 @@ AsynchronousImageCache::AsynchronousImageCache(ImageCacheStorageInterface &stora AsynchronousImageCache::~AsynchronousImageCache() { - clean(); - wait(); + m_generator.clean(); } void AsynchronousImageCache::request(Utils::SmallStringView name, @@ -93,27 +92,18 @@ void AsynchronousImageCache::request(Utils::SmallStringView name, } } -void AsynchronousImageCache::wait() -{ - stopThread(); - m_condition.notify_all(); - if (m_backgroundThread.joinable()) - m_backgroundThread.join(); -} - void AsynchronousImageCache::requestImage(Utils::SmallStringView name, ImageCache::CaptureImageCallback captureCallback, ImageCache::AbortCallback abortCallback, Utils::SmallStringView extraId, ImageCache::AuxiliaryData auxiliaryData) { - addEntry(std::move(name), - std::move(extraId), - std::move(captureCallback), - std::move(abortCallback), - std::move(auxiliaryData), - RequestType::Image); - m_condition.notify_all(); + m_taskQueue.addTask(std::move(name), + std::move(extraId), + std::move(captureCallback), + std::move(abortCallback), + std::move(auxiliaryData), + RequestType::Image); } void AsynchronousImageCache::requestMidSizeImage(Utils::SmallStringView name, @@ -122,13 +112,12 @@ void AsynchronousImageCache::requestMidSizeImage(Utils::SmallStringView name, Utils::SmallStringView extraId, ImageCache::AuxiliaryData auxiliaryData) { - addEntry(std::move(name), - std::move(extraId), - std::move(captureCallback), - std::move(abortCallback), - std::move(auxiliaryData), - RequestType::MidSizeImage); - m_condition.notify_all(); + m_taskQueue.addTask(std::move(name), + std::move(extraId), + std::move(captureCallback), + std::move(abortCallback), + std::move(auxiliaryData), + RequestType::MidSizeImage); } void AsynchronousImageCache::requestSmallImage(Utils::SmallStringView name, @@ -137,117 +126,18 @@ void AsynchronousImageCache::requestSmallImage(Utils::SmallStringView name, Utils::SmallStringView extraId, ImageCache::AuxiliaryData auxiliaryData) { - addEntry(std::move(name), - std::move(extraId), - std::move(captureCallback), - std::move(abortCallback), - std::move(auxiliaryData), - RequestType::SmallImage); - m_condition.notify_all(); + m_taskQueue.addTask(std::move(name), + std::move(extraId), + std::move(captureCallback), + std::move(abortCallback), + std::move(auxiliaryData), + RequestType::SmallImage); } void AsynchronousImageCache::clean() { - clearEntries(); m_generator.clean(); -} - -std::optional AsynchronousImageCache::getEntry( - std::unique_lock lock) -{ - auto l = std::move(lock); - if (m_entries.empty()) - return {}; - - Entry entry = m_entries.front(); - m_entries.pop_front(); - - return {entry}; -} - -void AsynchronousImageCache::addEntry(Utils::PathString &&name, - Utils::SmallString &&extraId, - ImageCache::CaptureImageCallback &&captureCallback, - ImageCache::AbortCallback &&abortCallback, - ImageCache::AuxiliaryData &&auxiliaryData, - RequestType requestType) -{ - std::unique_lock lock{m_mutex}; - - ensureThreadIsRunning(); - - m_entries.emplace_back(std::move(name), - std::move(extraId), - std::move(captureCallback), - std::move(abortCallback), - std::move(auxiliaryData), - requestType); -} - -void AsynchronousImageCache::clearEntries() -{ - std::unique_lock lock{m_mutex}; - for (Entry &entry : m_entries) - entry.abortCallback(ImageCache::AbortReason::Abort); - m_entries.clear(); -} - -std::tuple, bool> AsynchronousImageCache::waitForEntries() -{ - using namespace std::literals::chrono_literals; - std::unique_lock lock{m_mutex}; - if (m_finishing) - return {std::move(lock), true}; - if (m_entries.empty()) { - auto timedOutWithoutEntriesOrFinishing = !m_condition.wait_for(lock, 10min, [&] { - return m_entries.size() || m_finishing; - }); - - if (timedOutWithoutEntriesOrFinishing || m_finishing) { - m_sleeping = true; - return {std::move(lock), true}; - } - } - return {std::move(lock), false}; -} - -void AsynchronousImageCache::stopThread() -{ - std::unique_lock lock{m_mutex}; - m_finishing = true; -} - -void AsynchronousImageCache::ensureThreadIsRunning() -{ - if (m_finishing) - return; - - if (!m_sleeping) - return; - - if (m_backgroundThread.joinable()) - m_backgroundThread.join(); - - m_sleeping = false; - - m_backgroundThread = std::thread{[this] { - while (true) { - auto [lock, abort] = waitForEntries(); - if (abort) - return; - if (auto entry = getEntry(std::move(lock)); entry) { - request(entry->name, - entry->extraId, - entry->requestType, - std::move(entry->captureCallback), - std::move(entry->abortCallback), - std::move(entry->auxiliaryData), - m_storage, - m_generator, - m_timeStampProvider); - } - } - }}; + m_taskQueue.clean(); } } // namespace QmlDesigner diff --git a/src/plugins/qmldesigner/designercore/imagecache/asynchronousimagefactory.cpp b/src/plugins/qmldesigner/designercore/imagecache/asynchronousimagefactory.cpp index 03bafd18f43..a711f1ad7d8 100644 --- a/src/plugins/qmldesigner/designercore/imagecache/asynchronousimagefactory.cpp +++ b/src/plugins/qmldesigner/designercore/imagecache/asynchronousimagefactory.cpp @@ -20,93 +20,14 @@ AsynchronousImageFactory::AsynchronousImageFactory(ImageCacheStorageInterface &s } -AsynchronousImageFactory::~AsynchronousImageFactory() -{ - clean(); - wait(); -} - void AsynchronousImageFactory::generate(Utils::SmallStringView name, Utils::SmallStringView extraId, ImageCache::AuxiliaryData auxiliaryData) { - addEntry(name, extraId, std::move(auxiliaryData)); - m_condition.notify_all(); + m_taskQueue.addTask(name, extraId, std::move(auxiliaryData)); } -void AsynchronousImageFactory::addEntry(Utils::SmallStringView name, - Utils::SmallStringView extraId, - ImageCache::AuxiliaryData &&auxiliaryData) -{ - std::unique_lock lock{m_mutex}; - - ensureThreadIsRunning(); - - m_entries.emplace_back(std::move(name), std::move(extraId), std::move(auxiliaryData)); -} - -void AsynchronousImageFactory::ensureThreadIsRunning() -{ - if (m_finishing) - return; - - if (!m_sleeping) - return; - - if (m_backgroundThread.joinable()) - m_backgroundThread.join(); - - m_sleeping = false; - - m_backgroundThread = std::thread{[this] { - while (true) { - auto [lock, abort] = waitForEntries(); - if (abort) - return; - if (auto entry = getEntry(std::move(lock)); entry) { - request(entry->name, - entry->extraId, - std::move(entry->auxiliaryData), - m_storage, - m_timeStampProvider, - m_collector); - } - } - }}; -} - -std::tuple, bool> AsynchronousImageFactory::waitForEntries() -{ - using namespace std::literals::chrono_literals; - std::unique_lock lock{m_mutex}; - if (m_finishing) - return {std::move(lock), true}; - if (m_entries.empty()) { - auto timedOutWithoutEntriesOrFinishing = !m_condition.wait_for(lock, 10min, [&] { - return m_entries.size() || m_finishing; - }); - - if (timedOutWithoutEntriesOrFinishing || m_finishing) { - m_sleeping = true; - return {std::move(lock), true}; - } - } - return {std::move(lock), false}; -} - -std::optional AsynchronousImageFactory::getEntry( - std::unique_lock lock) -{ - auto l = std::move(lock); - - if (m_entries.empty()) - return {}; - - Entry entry = m_entries.front(); - m_entries.pop_front(); - - return {entry}; -} +AsynchronousImageFactory::~AsynchronousImageFactory() {} void AsynchronousImageFactory::request(Utils::SmallStringView name, Utils::SmallStringView extraId, @@ -125,8 +46,8 @@ void AsynchronousImageFactory::request(Utils::SmallStringView name, if (currentModifiedTime < (storageModifiedTime + pause)) return; - auto capture = [=](const QImage &image, const QImage &midSizeImage, const QImage &smallImage) { - m_storage.storeImage(id, currentModifiedTime, image, midSizeImage, smallImage); + auto capture = [&](const QImage &image, const QImage &midSizeImage, const QImage &smallImage) { + storage.storeImage(id, currentModifiedTime, image, midSizeImage, smallImage); }; collector.start(name, @@ -138,27 +59,6 @@ void AsynchronousImageFactory::request(Utils::SmallStringView name, void AsynchronousImageFactory::clean() { - clearEntries(); + m_taskQueue.clean(); } - -void AsynchronousImageFactory::wait() -{ - stopThread(); - m_condition.notify_all(); - if (m_backgroundThread.joinable()) - m_backgroundThread.join(); -} - -void AsynchronousImageFactory::clearEntries() -{ - std::unique_lock lock{m_mutex}; - m_entries.clear(); -} - -void AsynchronousImageFactory::stopThread() -{ - std::unique_lock lock{m_mutex}; - m_finishing = true; -} - } // namespace QmlDesigner diff --git a/src/plugins/qmldesigner/designercore/imagecache/asynchronousimagefactory.h b/src/plugins/qmldesigner/designercore/imagecache/asynchronousimagefactory.h index eb39740b8a4..f53ccc18ed7 100644 --- a/src/plugins/qmldesigner/designercore/imagecache/asynchronousimagefactory.h +++ b/src/plugins/qmldesigner/designercore/imagecache/asynchronousimagefactory.h @@ -5,6 +5,8 @@ #include "imagecacheauxiliarydata.h" +#include + #include #include @@ -50,32 +52,41 @@ private: ImageCache::AuxiliaryData auxiliaryData; }; - void addEntry(Utils::SmallStringView name, - Utils::SmallStringView extraId, - ImageCache::AuxiliaryData &&auxiliaryData); - void ensureThreadIsRunning(); - [[nodiscard]] std::tuple, bool> waitForEntries(); - [[nodiscard]] std::optional getEntry(std::unique_lock lock); - void request(Utils::SmallStringView name, - Utils::SmallStringView extraId, - ImageCache::AuxiliaryData auxiliaryData, - ImageCacheStorageInterface &storage, - TimeStampProviderInterface &timeStampProvider, - ImageCacheCollectorInterface &collector); - void wait(); - void clearEntries(); - void stopThread(); + static void request(Utils::SmallStringView name, + Utils::SmallStringView extraId, + ImageCache::AuxiliaryData auxiliaryData, + ImageCacheStorageInterface &storage, + TimeStampProviderInterface &timeStampProvider, + ImageCacheCollectorInterface &collector); + + struct Dispatch + { + void operator()(Entry &entry) + { + request(entry.name, + entry.extraId, + std::move(entry.auxiliaryData), + storage, + timeStampProvider, + collector); + } + + ImageCacheStorageInterface &storage; + TimeStampProviderInterface &timeStampProvider; + ImageCacheCollectorInterface &collector; + }; + + struct Clean + { + void operator()(Entry &) {} + }; private: - std::deque m_entries; - std::mutex m_mutex; - std::condition_variable m_condition; - std::thread m_backgroundThread; ImageCacheStorageInterface &m_storage; TimeStampProviderInterface &m_timeStampProvider; ImageCacheCollectorInterface &m_collector; - bool m_finishing{false}; - bool m_sleeping{true}; + TaskQueue m_taskQueue{Dispatch{m_storage, m_timeStampProvider, m_collector}, + Clean{}}; }; } // namespace QmlDesigner diff --git a/src/plugins/qmldesigner/designercore/imagecache/taskqueue.h b/src/plugins/qmldesigner/designercore/imagecache/taskqueue.h new file mode 100644 index 00000000000..e331b2f5716 --- /dev/null +++ b/src/plugins/qmldesigner/designercore/imagecache/taskqueue.h @@ -0,0 +1,133 @@ +// Copyright (C) 2023 The Qt Company Ltd. +// SPDX-License-Identifier: LicenseRef-Qt-Commercial OR GPL-3.0-only WITH Qt-GPL-exception-1.0 + +#pragma once + +#include +#include +#include +#include + +namespace QmlDesigner { + +template +class TaskQueue +{ +public: + TaskQueue(DispatchCallback dispatchCallback, ClearCallback clearCallback) + : m_dispatchCallback(std::move(dispatchCallback)) + , m_clearCallback(std::move(clearCallback)) + {} + + ~TaskQueue() + { + auto lock = clearTasks(); + stopThread(std::move(lock)); + joinThread(); + } + + template + void addTask(Arguments &&...arguments) + { + { + std::unique_lock lock{m_mutex}; + + ensureThreadIsRunning(); + + m_tasks.emplace_back(std::forward(arguments)...); + } + m_condition.notify_all(); + } + + void clean() { clearTasks(); } + +private: + [[nodiscard]] std::tuple, bool> waitForTasks() + { + using namespace std::literals::chrono_literals; + std::unique_lock lock{m_mutex}; + if (m_finishing) + return {std::move(lock), true}; + if (m_tasks.empty()) { + auto timedOutWithoutEntriesOrFinishing = !m_condition.wait_for(lock, 10min, [&] { + return m_tasks.size() || m_finishing; + }); + + if (timedOutWithoutEntriesOrFinishing || m_finishing) { + m_sleeping = true; + return {std::move(lock), true}; + } + } + return {std::move(lock), false}; + } + + [[nodiscard]] std::optional getTask(std::unique_lock lock) + { + auto l = std::move(lock); + + if (m_tasks.empty()) + return {}; + + Task task = std::move(m_tasks.front()); + m_tasks.pop_front(); + + return {task}; + } + + void ensureThreadIsRunning() + { + if (m_finishing || !m_sleeping) + return; + + if (m_backgroundThread.joinable()) + m_backgroundThread.join(); + + m_sleeping = false; + + m_backgroundThread = std::thread{[this] { + while (true) { + auto [lock, abort] = waitForTasks(); + if (abort) + return; + if (auto task = getTask(std::move(lock)); task) + m_dispatchCallback(*task); + } + }}; + } + + std::unique_lock clearTasks() + { + std::unique_lock lock{m_mutex}; + for (Task &task : m_tasks) + m_clearCallback(task); + m_tasks.clear(); + + return lock; + } + + void stopThread(std::unique_lock lock) + { + { + auto l = std::move(lock); + m_finishing = true; + } + m_condition.notify_all(); + } + + void joinThread() + { + if (m_backgroundThread.joinable()) + m_backgroundThread.join(); + } + +private: + std::deque m_tasks; + std::mutex m_mutex; + std::condition_variable m_condition; + std::thread m_backgroundThread; + DispatchCallback m_dispatchCallback; + ClearCallback m_clearCallback; + bool m_finishing{false}; + bool m_sleeping{true}; +}; +} // namespace QmlDesigner diff --git a/src/plugins/qmldesigner/designercore/include/asynchronousexplicitimagecache.h b/src/plugins/qmldesigner/designercore/include/asynchronousexplicitimagecache.h index 5fe61f4fe39..72d4746944e 100644 --- a/src/plugins/qmldesigner/designercore/include/asynchronousexplicitimagecache.h +++ b/src/plugins/qmldesigner/designercore/include/asynchronousexplicitimagecache.h @@ -5,6 +5,8 @@ #include "asynchronousimagecacheinterface.h" +#include + #include #include #include @@ -62,16 +64,6 @@ private: RequestType requestType = RequestType::Image; }; - [[nodiscard]] std::optional getEntry(std::unique_lock lock); - void addEntry(Utils::PathString &&name, - Utils::SmallString &&extraId, - ImageCache::CaptureImageCallback &&captureCallback, - ImageCache::AbortCallback &&abortCallback, - RequestType requestType); - void clearEntries(); - [[nodiscard]] std::tuple, bool> waitForEntries(); - void stopThread(); - void ensureThreadIsRunning(); static void request(Utils::SmallStringView name, Utils::SmallStringView extraId, AsynchronousExplicitImageCache::RequestType requestType, @@ -79,8 +71,28 @@ private: ImageCache::AbortCallback abortCallback, ImageCacheStorageInterface &storage); -private: - void wait(); + struct Dispatch + { + void operator()(RequestEntry &entry) + { + request(entry.name, + entry.extraId, + entry.requestType, + std::move(entry.captureCallback), + std::move(entry.abortCallback), + storage); + } + + ImageCacheStorageInterface &storage; + }; + + struct Clean + { + void operator()(RequestEntry &entry) + { + entry.abortCallback(ImageCache::AbortReason::Abort); + } + }; private: std::deque m_requestEntries; @@ -88,8 +100,7 @@ private: std::condition_variable m_condition; std::thread m_backgroundThread; ImageCacheStorageInterface &m_storage; - bool m_finishing{false}; - bool m_sleeping{true}; + TaskQueue m_taskQueue{Dispatch{m_storage}, Clean{}}; }; } // namespace QmlDesigner diff --git a/src/plugins/qmldesigner/designercore/include/asynchronousimagecache.h b/src/plugins/qmldesigner/designercore/include/asynchronousimagecache.h index 6435cc7eb43..3eed4a4487c 100644 --- a/src/plugins/qmldesigner/designercore/include/asynchronousimagecache.h +++ b/src/plugins/qmldesigner/designercore/include/asynchronousimagecache.h @@ -5,6 +5,8 @@ #include "asynchronousimagecacheinterface.h" +#include + #include #include #include @@ -73,17 +75,6 @@ private: RequestType requestType = RequestType::Image; }; - [[nodiscard]] std::optional getEntry(std::unique_lock lock); - void addEntry(Utils::PathString &&name, - Utils::SmallString &&extraId, - ImageCache::CaptureImageCallback &&captureCallback, - ImageCache::AbortCallback &&abortCallback, - ImageCache::AuxiliaryData &&auxiliaryData, - RequestType requestType); - void clearEntries(); - [[nodiscard]] std::tuple, bool> waitForEntries(); - void stopThread(); - void ensureThreadIsRunning(); static void request(Utils::SmallStringView name, Utils::SmallStringView extraId, AsynchronousImageCache::RequestType requestType, @@ -94,19 +85,37 @@ private: ImageCacheGeneratorInterface &generator, TimeStampProviderInterface &timeStampProvider); -private: - void wait(); + struct Dispatch + { + void operator()(Entry &entry) + { + request(entry.name, + entry.extraId, + entry.requestType, + std::move(entry.captureCallback), + std::move(entry.abortCallback), + std::move(entry.auxiliaryData), + storage, + generator, + timeStampProvider); + } + + ImageCacheStorageInterface &storage; + ImageCacheGeneratorInterface &generator; + TimeStampProviderInterface &timeStampProvider; + }; + + struct Clean + { + void operator()(Entry &entry) { entry.abortCallback(ImageCache::AbortReason::Abort); } + }; private: - std::deque m_entries; - mutable std::mutex m_mutex; - std::condition_variable m_condition; - std::thread m_backgroundThread; ImageCacheStorageInterface &m_storage; ImageCacheGeneratorInterface &m_generator; TimeStampProviderInterface &m_timeStampProvider; - bool m_finishing{false}; - bool m_sleeping{true}; + TaskQueue m_taskQueue{Dispatch{m_storage, m_generator, m_timeStampProvider}, + Clean{}}; }; } // namespace QmlDesigner diff --git a/tests/unit/tests/testdesignercore/CMakeLists.txt b/tests/unit/tests/testdesignercore/CMakeLists.txt index be7f5376abe..9a10b6d7449 100644 --- a/tests/unit/tests/testdesignercore/CMakeLists.txt +++ b/tests/unit/tests/testdesignercore/CMakeLists.txt @@ -52,6 +52,7 @@ add_qtc_library(TestDesignerCore OBJECT imagecache/imagecachedispatchcollector.h imagecache/imagecachestorageinterface.h imagecache/synchronousimagecache.cpp + imagecache/taskqueue.h imagecache/timestampproviderinterface.h include/abstractproperty.h include/asynchronousexplicitimagecache.h diff --git a/tests/unit/tests/unittests/imagecache/CMakeLists.txt b/tests/unit/tests/unittests/imagecache/CMakeLists.txt index 7f004221d3a..32ae6937a2c 100644 --- a/tests/unit/tests/unittests/imagecache/CMakeLists.txt +++ b/tests/unit/tests/unittests/imagecache/CMakeLists.txt @@ -8,4 +8,5 @@ extend_qtc_test(unittest imagecachegenerator-test.cpp imagecachestorage-test.cpp synchronousimagecache-test.cpp + taskqueue-test.cpp ) diff --git a/tests/unit/tests/unittests/imagecache/taskqueue-test.cpp b/tests/unit/tests/unittests/imagecache/taskqueue-test.cpp new file mode 100644 index 00000000000..537f4b3ea32 --- /dev/null +++ b/tests/unit/tests/unittests/imagecache/taskqueue-test.cpp @@ -0,0 +1,103 @@ +// Copyright (C) 2023 The Qt Company Ltd. +// SPDX-License-Identifier: LicenseRef-Qt-Commercial OR GPL-3.0-only WITH Qt-GPL-exception-1.0 + +#include "../utils/googletest.h" +#include "../utils/notification.h" + +#include + +namespace { +struct Task +{ + Task(int i) + : i{i} + {} + + int i = 5; + + friend bool operator==(Task first, Task second) { return first.i == second.i; } +}; + +template +auto IsTask(Matcher matcher) +{ + return Field(&Task::i, matcher); +} + +class TaskQueue : public testing::Test +{ +protected: + Notification notification; + Notification waitInThread; + NiceMock> mockDispatchCallback; + NiceMock> mockCleanCallback; + using Queue = QmlDesigner::TaskQueue; +}; + +TEST_F(TaskQueue, add_task_dispatches_task) +{ + Queue queue{mockDispatchCallback.AsStdFunction(), mockCleanCallback.AsStdFunction()}; + + EXPECT_CALL(mockDispatchCallback, Call(IsTask(22))).WillRepeatedly([&](Task) { + notification.notify(); + }); + + queue.addTask(22); + notification.wait(); +} + +TEST_F(TaskQueue, depatches_task_in_order) +{ + InSequence s; + Queue queue{mockDispatchCallback.AsStdFunction(), mockCleanCallback.AsStdFunction()}; + + EXPECT_CALL(mockDispatchCallback, Call(IsTask(22))).WillRepeatedly([&](Task) {}); + EXPECT_CALL(mockDispatchCallback, Call(IsTask(32))).WillRepeatedly([&](Task) { + notification.notify(); + }); + + queue.addTask(22); + queue.addTask(32); + notification.wait(); +} + +TEST_F(TaskQueue, cleanup_at_destruction) +{ + InSequence s; + ON_CALL(mockDispatchCallback, Call(IsTask(22))).WillByDefault([&](Task) { + notification.notify(); + waitInThread.wait(); + }); + + EXPECT_CALL(mockCleanCallback, Call(IsTask(32))).WillRepeatedly([&](Task) {}); + + { + Queue queue{mockDispatchCallback.AsStdFunction(), mockCleanCallback.AsStdFunction()}; + queue.addTask(22); + queue.addTask(32); + notification.wait(); + waitInThread.notify(); + } +} + +TEST_F(TaskQueue, clean_task_in_queue) +{ + InSequence s; + ON_CALL(mockDispatchCallback, Call(IsTask(22))).WillByDefault([&](Task) { + notification.notify(); + waitInThread.wait(); + }); + Queue queue{mockDispatchCallback.AsStdFunction(), mockCleanCallback.AsStdFunction()}; + queue.addTask(22); + queue.addTask(32); + + EXPECT_CALL(mockCleanCallback, Call(IsTask(32))).WillRepeatedly([&](Task) {}); + + notification.wait(); + waitInThread.notify(); + queue.clean(); +} + +} // namespace