More changes, render thread task management

include/mbgl/actor/actor.hpp

@@ -62,26 +62,22 @@ class Actor {
     template <class... Args>
     Actor(Scheduler& scheduler, Args&&... args)
         : target(scheduler, parent, std::forward<Args>(args)...) {}
+
+    template <class... Args>
+    Actor(const TaggedScheduler& scheduler, Args&&... args)
+        : retainer(scheduler.get()),
+          target(scheduler, parent, std::forward<Args>(args)...) {}
 
     template <class... Args>
     Actor(std::shared_ptr<Scheduler> scheduler, Args&&... args)
         : retainer(std::move(scheduler)),
           target(*retainer, parent, std::forward<Args>(args)...) {}
 
-    template <class... Args>
-    Actor(TaggedScheduler& scheduler, Args&&... args)
-        : schedulerTag(scheduler.tag()),
-          retainer(scheduler.get()),
-          target(*retainer, parent, std::forward<Args>(args)...) {}
-
-
-
     Actor(const Actor&) = delete;
 
     ActorRef<std::decay_t<Object>> self() { return parent.self(); }
 
 private:
-    const void* schedulerTag = nullptr;
     const std::shared_ptr<Scheduler> retainer;
     AspiringActor<Object> parent;
     EstablishedActor<Object> target;

include/mbgl/actor/established_actor.hpp

@@ -33,7 +33,18 @@ class EstablishedActor {
               class... Args,
               typename std::enable_if_t<std::is_constructible_v<U, Args...> ||
                                         std::is_constructible_v<U, ActorRef<U>, Args...>>* = nullptr>
-    EstablishedActor(Scheduler& scheduler, AspiringActor<Object>& parent_, Args&&... args)
+        EstablishedActor(Scheduler& scheduler, AspiringActor<Object>& parent_, Args&&... args)
+        : parent(parent_) {
+        emplaceObject(std::forward<Args>(args)...);
+        parent.mailbox->open(scheduler);
+    }
+
+    // Construct the Object from a parameter pack `args` (i.e. `Object(args...)`)
+    template <typename U = Object,
+              class... Args,
+              typename std::enable_if_t<std::is_constructible_v<U, Args...> ||
+                                        std::is_constructible_v<U, ActorRef<U>, Args...>>* = nullptr>
+    EstablishedActor(const TaggedScheduler& scheduler, AspiringActor<Object>& parent_, Args&&... args)
         : parent(parent_) {
         emplaceObject(std::forward<Args>(args)...);
         parent.mailbox->open(scheduler);
@@ -48,6 +59,11 @@ class EstablishedActor {
         parent.mailbox->open(scheduler);
     }
 
+    template <class ArgsTuple, std::size_t ArgCount = std::tuple_size<std::decay_t<ArgsTuple>>::value>
+    EstablishedActor(const TaggedScheduler& scheduler, AspiringActor<Object>& parent_, ArgsTuple&& args) {
+        EstablishedActor(*scheduler.get(), parent_, std::forward<ArgsTuple>(args));
+    }
+
     EstablishedActor(const EstablishedActor&) = delete;
 
     ~EstablishedActor() {

include/mbgl/actor/mailbox.hpp

@@ -6,6 +6,7 @@
 #include <queue>
 
 #include <mapbox/std/weak.hpp>
+#include <mbgl/actor/scheduler.hpp>
 
 namespace mbgl {
 
@@ -21,11 +22,13 @@ class Mailbox : public std::enable_shared_from_this<Mailbox> {
     Mailbox();
 
     Mailbox(Scheduler&);
+    Mailbox(const TaggedScheduler&);
 
     /// Attach the given scheduler to this mailbox and begin processing messages
     /// sent to it. The mailbox must be a "holding" mailbox, as created by the
     /// default constructor Mailbox().
-    void open(Scheduler& scheduler_);
+    void open(const TaggedScheduler& scheduler_);
+    void open(Scheduler&);
     void close();
 
     // Indicate this mailbox will no longer be checked for messages
@@ -46,6 +49,7 @@ class Mailbox : public std::enable_shared_from_this<Mailbox> {
         Abandoned
     };
 
+    const void* schedulerTag{nullptr};
     mapbox::base::WeakPtr<Scheduler> weakScheduler;
 
     std::recursive_mutex receivingMutex;

include/mbgl/actor/scheduler.hpp

@@ -47,7 +47,7 @@ class Scheduler {
     virtual void runOnRenderThread(const void*, std::function<void()>&&) {}
     /// Run render thread jobs for the given tag address
     /// @param closeQueue Runs all render jobs and then removes the internal queue.
-    virtual void runRenderJobs(const void*, bool closeQueue = false) {}
+    virtual void runRenderJobs(const void*, [[maybe_unused]] bool closeQueue = false) {}
     /// Returns a closure wrapping the given one.
     ///
     /// When the returned closure is invoked for the first time, it schedules

include/mbgl/gfx/renderer_backend.hpp

@@ -25,7 +25,7 @@ enum class ContextMode : bool {
 
 class RendererBackend {
 protected:
-    explicit RendererBackend(ContextMode, mbgl::Map*);
+    explicit RendererBackend(ContextMode);
 
 public:
     virtual ~RendererBackend();
@@ -70,7 +70,6 @@ class RendererBackend {
 protected:
     std::unique_ptr<Context> context;
     const ContextMode contextMode;
-    const mbgl::Map* owner;
     std::once_flag initialized;
 
     friend class BackendScope;

include/mbgl/gl/renderer_backend.hpp

@@ -16,7 +16,7 @@ using FramebufferID = uint32_t;
 
 class RendererBackend : public gfx::RendererBackend {
 public:
-    RendererBackend(gfx::ContextMode, mbgl::Map*);
+    RendererBackend(gfx::ContextMode);
     ~RendererBackend() override;
 
     /// Called prior to rendering to update the internally assumed OpenGL state.

include/mbgl/mtl/context.hpp

@@ -159,7 +159,7 @@ class Context final : public gfx::Context {
 
 private:
     RendererBackend& backend;
-    TaggedScheduler& scheduler;
+    TaggedScheduler scheduler;
     bool cleanupOnDestruction = true;
 
     std::optional<BufferResource> emptyBuffer;

src/mbgl/actor/mailbox.cpp

@@ -12,6 +12,16 @@ Mailbox::Mailbox() = default;
 Mailbox::Mailbox(Scheduler& scheduler_)
     : weakScheduler(scheduler_.makeWeakPtr()) {}
 
+Mailbox::Mailbox(const TaggedScheduler& scheduler_)
+    : schedulerTag(scheduler_.tag()),
+      weakScheduler(scheduler_.get()->makeWeakPtr()) {}
+
+void Mailbox::open(const TaggedScheduler& scheduler_) {
+    assert(!weakScheduler);
+    schedulerTag = scheduler_.tag();
+    return open(*scheduler_.get());
+}
+
 void Mailbox::open(Scheduler& scheduler_) {
     assert(!weakScheduler);
 
@@ -28,7 +38,7 @@ void Mailbox::open(Scheduler& scheduler_) {
 
     if (!queue.empty()) {
         auto guard = weakScheduler.lock();
-        if (weakScheduler) weakScheduler->schedule(makeClosure(shared_from_this()));
+        if (weakScheduler) weakScheduler->schedule(schedulerTag, makeClosure(shared_from_this()));
     }
 }
 
@@ -86,7 +96,7 @@ void Mailbox::push(std::unique_ptr<Message> message) {
     queue.push(std::move(message));
     auto guard = weakScheduler.lock();
     if (wasEmpty && weakScheduler) {
-        weakScheduler->schedule(makeClosure(shared_from_this()));
+        weakScheduler->schedule(schedulerTag, makeClosure(shared_from_this()));
     }
 }
 
@@ -127,7 +137,7 @@ void Mailbox::receive() {
     (*message)();
 
     if (!wasEmpty) {
-        weakScheduler->schedule(makeClosure(shared_from_this()));
+        weakScheduler->schedule(schedulerTag, makeClosure(shared_from_this()));
     }
 }
 

src/mbgl/gfx/renderer_backend.cpp

@@ -5,9 +5,8 @@
 namespace mbgl {
 namespace gfx {
 
-RendererBackend::RendererBackend(const ContextMode contextMode_, mbgl::Map* map)
-    : contextMode(contextMode_),
-      owner(map) {}
+RendererBackend::RendererBackend(const ContextMode contextMode_)
+    : contextMode(contextMode_) {}
 RendererBackend::~RendererBackend() = default;
 
 gfx::Context& RendererBackend::getContext() {

src/mbgl/gl/renderer_backend.cpp

@@ -15,12 +15,12 @@
 namespace mbgl {
 namespace gl {
 
-RendererBackend::RendererBackend(const gfx::ContextMode contextMode_, mbgl::Map* map)
-    : gfx::RendererBackend(contextMode_, map) {}
+RendererBackend::RendererBackend(const gfx::ContextMode contextMode_)
+    : gfx::RendererBackend(contextMode_) {}
 
 std::unique_ptr<gfx::Context> RendererBackend::createContext() {
     auto result = std::make_unique<gl::Context>(*this,
-        TaggedScheduler{Scheduler::GetBackground(), static_cast<const void*>(owner)});
+        TaggedScheduler{Scheduler::GetBackground(), static_cast<const void*>(this)}); // Tagged background thread pool will be owned by the RendererBackend
     result->enableDebugging();
     result->initializeExtensions(std::bind(&RendererBackend::getExtensionFunctionPointer, this, std::placeholders::_1));
     return result;

src/mbgl/renderer/render_orchestrator.cpp

@@ -187,7 +187,8 @@ std::unique_ptr<RenderTree> RenderOrchestrator::createRenderTree(
                                         updateParameters->annotationManager,
                                         imageManager,
                                         glyphManager,
-                                        updateParameters->prefetchZoomDelta};
+                                        updateParameters->prefetchZoomDelta,
+                                        threadPool};
 
     glyphManager->setURL(updateParameters->glyphURL);
 

src/mbgl/renderer/tile_parameters.hpp

@@ -1,6 +1,7 @@
 #pragma once
 
 #include <mbgl/map/mode.hpp>
+#include <mbgl/actor/scheduler.hpp>
 
 #include <memory>
 
@@ -25,6 +26,7 @@ class TileParameters {
     std::shared_ptr<ImageManager> imageManager;
     std::shared_ptr<GlyphManager> glyphManager;
     const uint8_t prefetchZoomDelta;
+    TaggedScheduler threadPool;
 };
 
 } // namespace mbgl

src/mbgl/tile/geometry_tile.cpp

@@ -156,15 +156,15 @@ const LayerRenderData* GeometryTileRenderData::getLayerRenderData(const style::L
    that could flag the tile as non-pending too early.
  */
 
-GeometryTile::GeometryTile(const OverscaledTileID& id_, std::string sourceID_, const TileParameters& parameters, TaggedScheduler& scheduler)
+GeometryTile::GeometryTile(const OverscaledTileID& id_, std::string sourceID_, const TileParameters& parameters)
     : Tile(Kind::Geometry, id_),
       ImageRequestor(parameters.imageManager),
       sourceID(std::move(sourceID_)),
-      threadPool(scheduler),
+      threadPool(parameters.threadPool),
       mailbox(std::make_shared<Mailbox>(*Scheduler::GetCurrent())),
-      worker(threadPool, // TaggedScheduler reference for the Actor retainer
+      worker(parameters.threadPool, // Scheduler reference for the Actor retainer
              ActorRef<GeometryTile>(*this, mailbox),
-             threadPool, // TaggedScheduler reference for the worker
+             parameters.threadPool,
              id_,
              sourceID,
              obsolete,

src/mbgl/tile/geometry_tile.hpp

@@ -28,7 +28,7 @@ class TileAtlasTextures;
 
 class GeometryTile : public Tile, public GlyphRequestor, public ImageRequestor {
 public:
-    GeometryTile(const OverscaledTileID&, std::string sourceID, const TileParameters&, TaggedScheduler& scheduler);
+    GeometryTile(const OverscaledTileID&, std::string sourceID, const TileParameters&);
 
     ~GeometryTile() override;
 

src/mbgl/tile/geometry_tile_worker.cpp

@@ -30,7 +30,7 @@ using namespace style;
 
 GeometryTileWorker::GeometryTileWorker(ActorRef<GeometryTileWorker> self_,
                                        ActorRef<GeometryTile> parent_,
-                                       TaggedScheduler& scheduler_,
+                                       const TaggedScheduler& scheduler_,
                                        OverscaledTileID id_,
                                        std::string sourceID_,
                                        const std::atomic<bool>& obsolete_,

src/mbgl/tile/geometry_tile_worker.hpp

@@ -30,7 +30,7 @@ class GeometryTileWorker {
 public:
     GeometryTileWorker(ActorRef<GeometryTileWorker> self,
                        ActorRef<GeometryTile> parent,
-                       TaggedScheduler& scheduler_,
+                       const TaggedScheduler& scheduler_,
                        OverscaledTileID,
                        std::string,
                        const std::atomic<bool>&,

src/mbgl/util/thread_pool.cpp

@@ -11,9 +11,6 @@ namespace mbgl {
 ThreadedSchedulerBase::~ThreadedSchedulerBase() = default;
 
 void ThreadedSchedulerBase::terminate() {
-    // Run any leftover render jobs
-    runRenderJobs();
-
     {
         std::lock_guard<std::mutex> lock(workerMutex);
         terminated = true;

src/mbgl/util/thread_pool.hpp

@@ -141,14 +141,15 @@ class ThreadedScheduler : public ThreadedSchedulerBase {
 
 private:
     std::vector<std::thread> threads;
-    mapbox::base::WeakPtrFactory<Scheduler> weakFactory{this};
 
     struct RenderQueue {
         std::queue<std::function<void()>> queue;
         std::mutex mutex;
     };
     mbgl::unordered_map<const void*, std::shared_ptr<RenderQueue>> taggedRenderQueue;
     std::mutex taggedRenderQueueLock;
+
+    mapbox::base::WeakPtrFactory<Scheduler> weakFactory{this};
 };
 
 class SequencedScheduler : public ThreadedScheduler {

test/util/async_task.test.cpp

@@ -104,8 +104,8 @@ TEST(AsyncTask, RequestCoalescingMultithreaded) {
     unsigned count = 0, numThreads = 25;
     AsyncTask async([&count] { ++count; });
 
-    std::shared_ptr<Scheduler> retainer = Scheduler::GetBackground();
-    auto mailbox = std::make_shared<Mailbox>(*retainer);
+    TaggedScheduler retainer = {Scheduler::GetBackground(), &loop};
+    auto mailbox = std::make_shared<Mailbox>(retainer);
 
     TestWorker worker(&async);
     ActorRef<TestWorker> workerRef(worker, mailbox);
@@ -133,8 +133,8 @@ TEST(AsyncTask, ThreadSafety) {
 
     AsyncTask async([&count] { ++count; });
 
-    std::shared_ptr<Scheduler> retainer = Scheduler::GetBackground();
-    auto mailbox = std::make_shared<Mailbox>(*retainer);
+    TaggedScheduler retainer = {Scheduler::GetBackground(), &loop};
+    auto mailbox = std::make_shared<Mailbox>(retainer);
 
     TestWorker worker(&async);
     ActorRef<TestWorker> workerRef(worker, mailbox);