QmlDesigner: Compress generator tasks

A document change can generate new image request and if the are not in the
database new task for the collector are generated. But if there are already
tasks for an equal id they would generated twice. So the tasks are now
merged. The auxilialry data is not merged because it is expected that
it is not changing.

Task-number: QDS-3388
Change-Id: Id1aceb564dd9dc7c5a1ea5ae4680d7ee36611cda
Reviewed-by: Thomas Hartmann <thomas.hartmann@qt.io>

src/plugins/qmldesigner/designercore/imagecache/imagecachegenerator.cpp

@@ -56,22 +56,47 @@ void ImageCacheGenerator::generateImage(Utils::SmallStringView name,
 {
     {
         std::lock_guard lock{m_mutex};
-        m_tasks.emplace_back(name,
+
-                             extraId,
+        auto found = std::find_if(m_tasks.begin(), m_tasks.end(), [&](const Task &task) {
-                             std::move(auxiliaryData),
+            return task.filePath == name && task.extraId == extraId;
-                             timeStamp,
+        });
-                             std::move(captureCallback),
+
-                             std::move(abortCallback));
+        if (found != m_tasks.end()) {
+            found->timeStamp = timeStamp;
+            found->captureCallbacks.push_back(std::move(captureCallback));
+            found->abortCallbacks.push_back(std::move(abortCallback));
+        } else {
+            m_tasks.emplace_back(name,
+                                 extraId,
+                                 std::move(auxiliaryData),
+                                 timeStamp,
+                                 std::move(captureCallback),
+                                 std::move(abortCallback));
+        }
     }
 
     m_condition.notify_all();
 }
 
+namespace {
+Utils::PathString createId(Utils::SmallStringView name, Utils::SmallStringView extraId)
+{
+    return extraId.empty() ? Utils::PathString{name} : Utils::PathString{name, "+", extraId};
+}
+template<typename Callbacks, typename... Argument>
+void callCallbacks(const Callbacks &callbacks, Argument &&...arguments)
+{
+    for (auto &&callback : callbacks)
+        callback(std::forward<Argument>(arguments)...);
+}
+
+} // namespace
+
 void ImageCacheGenerator::clean()
 {
     std::lock_guard lock{m_mutex};
     for (Task &task : m_tasks)
-        task.abortCallback();
+        callCallbacks(task.abortCallbacks);
     m_tasks.clear();
 }
 
@@ -83,12 +108,6 @@ void ImageCacheGenerator::waitForFinished()
     if (m_backgroundThread)
         m_backgroundThread->wait();
 }
-namespace {
-Utils::PathString createId(Utils::SmallStringView name, Utils::SmallStringView extraId)
-{
-    return extraId.empty() ? Utils::PathString{name} : Utils::PathString{name, "+", extraId};
-}
-} // namespace
 
 void ImageCacheGenerator::startGeneration()
 {
@@ -105,9 +124,9 @@ void ImageCacheGenerator::startGeneration()
                 return;
             }
 
-            task = std::move(m_tasks.back());
+            task = std::move(m_tasks.front());
 
-            m_tasks.pop_back();
+            m_tasks.pop_front();
         }
 
         m_collector.start(
@@ -116,9 +135,9 @@ void ImageCacheGenerator::startGeneration()
             std::move(task.auxiliaryData),
             [this, task](QImage &&image, QImage &&smallImage) {
                 if (image.isNull())
-                    task.abortCallback();
+                    callCallbacks(task.abortCallbacks);
                 else
-                    task.captureCallback(image, smallImage);
+                    callCallbacks(task.captureCallbacks, image, smallImage);
 
                 m_storage.storeImage(createId(task.filePath, task.extraId),
                                      task.timeStamp,
@@ -126,7 +145,7 @@ void ImageCacheGenerator::startGeneration()
                                      smallImage);
             },
             [this, task] {
-                task.abortCallback();
+                callCallbacks(task.abortCallbacks);
                 m_storage.storeImage(createId(task.filePath, task.extraId), task.timeStamp, {}, {});
             });
 

src/plugins/qmldesigner/designercore/imagecache/imagecachegenerator.h

@@ -32,6 +32,7 @@
 
 #include <QThread>
 
+#include <deque>
 #include <memory>
 #include <mutex>
 
@@ -74,16 +75,16 @@ private:
             : filePath(filePath)
             , extraId(std::move(extraId))
             , auxiliaryData(std::move(auxiliaryData))
-            , captureCallback(std::move(captureCallback))
+            , captureCallbacks({std::move(captureCallback)})
-            , abortCallback(std::move(abortCallback))
+            , abortCallbacks({std::move(abortCallback)})
             , timeStamp(timeStamp)
         {}
 
         Utils::PathString filePath;
         Utils::SmallString extraId;
         ImageCache::AuxiliaryData auxiliaryData;
-        CaptureCallback captureCallback;
+        std::vector<CaptureCallback> captureCallbacks;
-        AbortCallback abortCallback;
+        std::vector<AbortCallback> abortCallbacks;
         Sqlite::TimeStamp timeStamp;
     };
 
@@ -98,7 +99,7 @@ private:
     std::unique_ptr<QThread> m_backgroundThread;
     mutable std::mutex m_mutex;
     std::condition_variable m_condition;
-    std::vector<Task> m_tasks;
+    std::deque<Task> m_tasks;
     ImageCacheCollectorInterface &m_collector;
     ImageCacheStorageInterface &m_storage;
     bool m_finishing{false};

tests/unit/unittest/imagecachegenerator-test.cpp

@@ -78,11 +78,11 @@ TEST_F(ImageCacheGenerator, CallsCollectorWithCaptureCallback)
 
 TEST_F(ImageCacheGenerator, CallsCollectorOnlyIfNotProcessing)
 {
-    EXPECT_CALL(collectorMock, start(Eq("name"), _, _, _, _))
+    EXPECT_CALL(collectorMock, start(AnyOf(Eq("name"), Eq("name2")), _, _, _, _))
         .WillRepeatedly([&](auto, auto, auto, auto, auto) { notification.notify(); });
 
     generator.generateImage("name", {}, {}, imageCallbackMock.AsStdFunction(), {}, {});
-    generator.generateImage("name", {}, {}, imageCallbackMock.AsStdFunction(), {}, {});
+    generator.generateImage("name2", {}, {}, imageCallbackMock.AsStdFunction(), {}, {});
     notification.wait(2);
 }
 
@@ -320,4 +320,134 @@ TEST_F(ImageCacheGenerator, CallsCollectorWithAuxiliaryData)
     notification.wait();
 }
 
+TEST_F(ImageCacheGenerator, MergeTasks)
+{
+    EXPECT_CALL(collectorMock, start(Eq("waitDummy"), _, _, _, _))
+        .WillRepeatedly([&](auto, auto, auto, auto, auto) { waitInThread.wait(); });
+    EXPECT_CALL(collectorMock, start(Eq("notificationDummy"), _, _, _, _))
+        .WillRepeatedly([&](auto, auto, auto, auto, auto) { notification.notify(); });
+
+    EXPECT_CALL(collectorMock, start(Eq("name"), _, _, _, _));
+
+    generator.generateImage("waitDummy", {}, {}, {}, {}, {});
+    generator.generateImage("name", {}, {}, {}, {}, {});
+    generator.generateImage("name", {}, {}, {}, {}, {});
+    generator.generateImage("notificationDummy", {}, {}, {}, {}, {});
+    waitInThread.notify();
+    notification.wait();
+}
+
+TEST_F(ImageCacheGenerator, DontMergeTasksWithDifferentId)
+{
+    EXPECT_CALL(collectorMock, start(Eq("waitDummy"), _, _, _, _))
+        .WillRepeatedly([&](auto, auto, auto, auto, auto) { waitInThread.wait(); });
+
+    EXPECT_CALL(collectorMock, start(Eq("name"), _, _, _, _))
+        .WillRepeatedly([&](auto, auto, auto, auto, auto) { notification.notify(); });
+    EXPECT_CALL(collectorMock, start(Eq("name2"), _, _, _, _))
+        .WillRepeatedly([&](auto, auto, auto, auto, auto) { notification.notify(); });
+
+    generator.generateImage("waitDummy", {}, {}, {}, {}, {});
+    generator.generateImage("name", {}, {}, {}, {}, {});
+    generator.generateImage("name2", {}, {}, {}, {}, {});
+    waitInThread.notify();
+    notification.wait(2);
+}
+
+TEST_F(ImageCacheGenerator, MergeTasksWithSameExtraId)
+{
+    EXPECT_CALL(collectorMock, start(Eq("waitDummy"), _, _, _, _))
+        .WillRepeatedly([&](auto, auto, auto, auto, auto) { waitInThread.wait(); });
+    EXPECT_CALL(collectorMock, start(Eq("notificationDummy"), _, _, _, _))
+        .WillRepeatedly([&](auto, auto, auto, auto, auto) { notification.notify(); });
+
+    EXPECT_CALL(collectorMock, start(Eq("name"), _, _, _, _));
+
+    generator.generateImage("waitDummy", {}, {}, {}, {}, {});
+    generator.generateImage("name", "id1", {}, {}, {}, {});
+    generator.generateImage("name", "id1", {}, {}, {}, {});
+    waitInThread.notify();
+    generator.generateImage("notificationDummy", {}, {}, {}, {}, {});
+    notification.wait();
+}
+
+TEST_F(ImageCacheGenerator, DontMergeTasksWithDifferentExtraId)
+{
+    EXPECT_CALL(collectorMock, start(Eq("waitDummy"), _, _, _, _))
+        .WillRepeatedly([&](auto, auto, auto, auto, auto) { waitInThread.wait(); });
+
+    EXPECT_CALL(collectorMock, start(Eq("name"), _, _, _, _))
+        .Times(2)
+        .WillRepeatedly([&](auto, auto, auto, auto, auto) { notification.notify(); });
+
+    generator.generateImage("waitDummy", {}, {}, {}, {}, {});
+    generator.generateImage("name", "id1", {}, {}, {}, {});
+    generator.generateImage("name", "id2", {}, {}, {}, {});
+    waitInThread.notify();
+    notification.wait(2);
+}
+
+TEST_F(ImageCacheGenerator, UseLastTimeStampIfTasksAreMerged)
+{
+    ON_CALL(collectorMock, start(Eq("waitDummy"), _, _, _, _))
+        .WillByDefault([&](auto, auto, auto, auto, auto) { waitInThread.wait(); });
+    ON_CALL(collectorMock, start(Eq("notificationDummy"), _, _, _, _))
+        .WillByDefault([&](auto, auto, auto, auto, auto) { notification.notify(); });
+    ON_CALL(collectorMock, start(Eq("name"), _, _, _, _))
+        .WillByDefault([&](auto, auto, auto, auto, auto abortCallback) { abortCallback(); });
+
+    EXPECT_CALL(storageMock, storeImage(Eq("name"), _, _, _));
+
+    generator.generateImage("waitDummy", {}, {}, {}, {}, {});
+    generator.generateImage("name", {}, {3}, {}, abortCallbackMock.AsStdFunction(), {});
+    generator.generateImage("name", {}, {4}, {}, abortCallbackMock.AsStdFunction(), {});
+    generator.generateImage("notificationDummy", {}, {}, {}, {}, {});
+    waitInThread.notify();
+    notification.wait();
+}
+
+TEST_F(ImageCacheGenerator, MergeCaptureCallbackIfTasksAreMerged)
+{
+    NiceMock<MockFunction<void(const QImage &, const QImage &)>> newerImageCallbackMock;
+    ON_CALL(collectorMock, start(Eq("waitDummy"), _, _, _, _))
+        .WillByDefault([&](auto, auto, auto, auto, auto) { waitInThread.wait(); });
+    ON_CALL(collectorMock, start(Eq("notificationDummy"), _, _, _, _))
+        .WillByDefault([&](auto, auto, auto, auto, auto) { notification.notify(); });
+    ON_CALL(collectorMock, start(Eq("name"), _, _, _, _))
+        .WillByDefault([&](auto, auto, auto, auto imageCallback, auto) {
+            imageCallback(QImage{image1}, QImage{smallImage1});
+        });
+
+    EXPECT_CALL(imageCallbackMock, Call(_, _));
+    EXPECT_CALL(newerImageCallbackMock, Call(_, _));
+
+    generator.generateImage("waitDummy", {}, {}, {}, {}, {});
+    generator.generateImage("name", {}, {}, imageCallbackMock.AsStdFunction(), {}, {});
+    generator.generateImage("name", {}, {}, newerImageCallbackMock.AsStdFunction(), {}, {});
+    generator.generateImage("notificationDummy", {}, {}, {}, {}, {});
+    waitInThread.notify();
+    notification.wait();
+}
+
+TEST_F(ImageCacheGenerator, MergeAbortCallbackIfTasksAreMerged)
+{
+    NiceMock<MockFunction<void()>> newerAbortCallbackMock;
+    ON_CALL(collectorMock, start(Eq("waitDummy"), _, _, _, _))
+        .WillByDefault([&](auto, auto, auto, auto, auto) { waitInThread.wait(); });
+    ON_CALL(collectorMock, start(Eq("notificationDummy"), _, _, _, _))
+        .WillByDefault([&](auto, auto, auto, auto, auto) { notification.notify(); });
+    ON_CALL(collectorMock, start(Eq("name"), _, _, _, _))
+        .WillByDefault([&](auto, auto, auto, auto, auto abortCallback) { abortCallback(); });
+
+    EXPECT_CALL(abortCallbackMock, Call());
+    EXPECT_CALL(newerAbortCallbackMock, Call());
+
+    generator.generateImage("waitDummy", {}, {}, {}, {}, {});
+    generator.generateImage("name", {}, {}, {}, abortCallbackMock.AsStdFunction(), {});
+    generator.generateImage("name", {}, {}, {}, newerAbortCallbackMock.AsStdFunction(), {});
+    generator.generateImage("notificationDummy", {}, {}, {}, {}, {});
+    waitInThread.notify();
+    notification.wait();
+}
+
 } // namespace