From 76bbc42fa6fedbaef7fc1fdd9412049a62293fd2 Mon Sep 17 00:00:00 2001
From: Mikhail Pozdnyakov <mikhail.pozdnyakov@mapbox.com>
Date: Tue, 8 Oct 2019 17:14:37 +0300
Subject: [core] Coalesce requests for the same missing image

This commit coalesces the repeated `onStyleImageMissing` calls
for the same image.

It also simplifies the image manager code.
---
 src/mbgl/renderer/image_manager.cpp | 54 ++++++++++++++++++++++---------------
 src/mbgl/renderer/image_manager.hpp | 18 ++++++++-----
 2 files changed, 43 insertions(+), 29 deletions(-)

diff --git a/src/mbgl/renderer/image_manager.cpp b/src/mbgl/renderer/image_manager.cpp
index d001084f92..a776672edd 100644
--- a/src/mbgl/renderer/image_manager.cpp
+++ b/src/mbgl/renderer/image_manager.cpp
@@ -138,8 +138,9 @@ void ImageManager::removeRequestor(ImageRequestor& requestor) {
 
 void ImageManager::notifyIfMissingImageAdded() {
     for (auto it = missingImageRequestors.begin(); it != missingImageRequestors.end();) {
-        if (it->second.callbacks.empty()) {
-            notify(*it->first, it->second.pair);
+        ImageRequestor& requestor = *it->first;
+        if (!requestor.hasPendingRequests()) {
+            notify(requestor, it->second);
             it = missingImageRequestors.erase(it);
         } else {
             ++it;
@@ -179,28 +180,37 @@ void ImageManager::checkMissingAndNotify(ImageRequestor& requestor, const ImageR
 
     if (!missingImages.empty()) {
         ImageRequestor* requestorPtr = &requestor;
-
-        auto emplaced = missingImageRequestors.emplace(requestorPtr, MissingImageRequestPair { pair, {} });
-        assert(emplaced.second);
+        assert(!missingImageRequestors.count(requestorPtr));
+        missingImageRequestors.emplace(requestorPtr, pair);
 
         for (const auto& missingImage : missingImages) {
             assert(observer != nullptr);
-            requestedImages[missingImage].emplace(&requestor);
-            auto callback = std::make_unique<ActorCallback>(
-                *Scheduler::GetCurrent(),
-                [this, requestorPtr, missingImage] {
-                    auto requestorIt = missingImageRequestors.find(requestorPtr);
-                    if (requestorIt != missingImageRequestors.end()) {
-                        assert(requestorIt->second.callbacks.find(missingImage) != requestorIt->second.callbacks.end());
-                        requestorIt->second.callbacks.erase(missingImage);
-                    }
-            });
-
-            auto actorRef = callback->self();
-            emplaced.first->second.callbacks.emplace(missingImage, std::move(callback));
-            observer->onStyleImageMissing(missingImage, [actorRef] {
-                actorRef.invoke(&Callback::operator());
-            });
+
+            auto existingRequestorsIt = requestedImages.find(missingImage);
+            if (existingRequestorsIt != requestedImages.end()) { // Already asked client about this image.
+                if (!existingRequestorsIt->second.empty()) {     // Still waiting for the client response.
+                    existingRequestorsIt->second.emplace(requestorPtr);
+                    requestor.addPendingRequest(missingImage);
+                }
+                continue;
+            }
+            requestedImages[missingImage].emplace(requestorPtr);
+            requestor.addPendingRequest(missingImage);
+            auto removePendingRequests = [this, missingImage] {
+                auto existingRequest = requestedImages.find(missingImage);
+                if (existingRequest == requestedImages.end()) {
+                    return;
+                }
+
+                for (auto* req : existingRequest->second) {
+                    req->removePendingRequest(missingImage);
+                }
+            };
+
+            Scheduler& scheduler = *Scheduler::GetCurrent();
+            auto callback = [&scheduler, removePendingRequests]() { scheduler.schedule(removePendingRequests); };
+
+            observer->onStyleImageMissing(missingImage, std::move(callback));
         }
     } else {
         // Associate requestor with an image that was provided by the client.
@@ -230,7 +240,7 @@ void ImageManager::notify(ImageRequestor& requestor, const ImageRequestPair& pai
         }
     }
 
-    requestor.onImagesAvailable(iconMap, patternMap, std::move(versionMap), pair.second);
+    requestor.onImagesAvailable(std::move(iconMap), std::move(patternMap), std::move(versionMap), pair.second);
 }
 
 void ImageManager::dumpDebugLogs() const {
diff --git a/src/mbgl/renderer/image_manager.hpp b/src/mbgl/renderer/image_manager.hpp
index 9097418681..98b42da838 100644
--- a/src/mbgl/renderer/image_manager.hpp
+++ b/src/mbgl/renderer/image_manager.hpp
@@ -58,13 +58,7 @@ private:
     bool loaded = false;
 
     std::map<ImageRequestor*, ImageRequestPair> requestors;
-    using Callback = std::function<void()>;
-    using ActorCallback = Actor<Callback>;
-    struct MissingImageRequestPair {
-        ImageRequestPair pair;
-        std::map<std::string, std::unique_ptr<ActorCallback>> callbacks;
-    };
-    std::map<ImageRequestor*, MissingImageRequestPair> missingImageRequestors;
+    std::map<ImageRequestor*, ImageRequestPair> missingImageRequestors;
     std::map<std::string, std::set<ImageRequestor*>> requestedImages;
     std::size_t requestedImagesCacheSize = 0ul;
     ImageMap images;
@@ -77,8 +71,18 @@ public:
     explicit ImageRequestor(ImageManager&);
     virtual ~ImageRequestor();
     virtual void onImagesAvailable(ImageMap icons, ImageMap patterns, ImageVersionMap versionMap, uint64_t imageCorrelationID) = 0;
+
+    void addPendingRequest(const std::string& imageId) { pendingRequests.insert(imageId); }
+
+    bool hasPendingRequests() const { return !pendingRequests.empty(); }
+
+    void removePendingRequest(const std::string& imageId) { pendingRequests.erase(imageId); }
+
 private:
     ImageManager& imageManager;
+
+    // Pending requests are image requests that are waiting to be dispatched to the client.
+    std::set<std::string> pendingRequests;
 };
 
 } // namespace mbgl
-- 
cgit v1.2.1