From 4e71b380e442b6814f3d7fbe501cb965132ad6c2 Mon Sep 17 00:00:00 2001 From: mwilsnd Date: Wed, 22 May 2024 15:56:09 -0500 Subject: [PATCH] Add TaggedScheduler --- include/mbgl/actor/actor.hpp | 7 + include/mbgl/actor/established_actor.hpp | 18 ++- include/mbgl/actor/mailbox.hpp | 6 +- include/mbgl/actor/scheduler.hpp | 40 ++++- include/mbgl/gfx/context.hpp | 5 +- include/mbgl/gfx/renderer_backend.hpp | 6 + include/mbgl/mtl/renderer_backend.hpp | 2 +- include/mbgl/util/run_loop.hpp | 3 +- .../MapLibreAndroid/src/cpp/map_renderer.cpp | 9 +- .../MapLibreAndroid/src/cpp/map_renderer.hpp | 5 +- .../android/maps/renderer/MapRenderer.java | 5 - .../maps/renderer/MapRendererScheduler.java | 3 - .../GLSurfaceViewMapRenderer.java | 4 +- .../glsurfaceview/MapLibreGLSurfaceView.java | 29 +--- .../textureview/TextureViewMapRenderer.java | 4 +- .../textureview/TextureViewRenderThread.java | 25 +-- .../android/maps/NativeMapViewTest.kt | 3 +- platform/android/src/run_loop.cpp | 13 +- platform/android/src/run_loop_impl.hpp | 2 +- platform/darwin/src/run_loop.cpp | 9 +- platform/default/src/mbgl/util/run_loop.cpp | 9 +- platform/qt/src/mbgl/run_loop.cpp | 9 +- platform/qt/src/utils/scheduler.cpp | 12 +- platform/qt/src/utils/scheduler.hpp | 2 +- src/mbgl/actor/mailbox.cpp | 16 +- .../annotation/render_annotation_source.cpp | 4 +- .../annotation/render_annotation_source.hpp | 2 +- src/mbgl/gfx/renderer_backend.cpp | 3 +- src/mbgl/gl/context.cpp | 4 +- src/mbgl/gl/renderer_backend.cpp | 2 +- src/mbgl/mtl/context.cpp | 4 +- src/mbgl/mtl/renderer_backend.cpp | 7 +- src/mbgl/renderer/render_orchestrator.cpp | 11 +- src/mbgl/renderer/render_orchestrator.hpp | 5 +- src/mbgl/renderer/render_source.cpp | 2 +- src/mbgl/renderer/render_source.hpp | 3 +- src/mbgl/renderer/renderer_impl.cpp | 2 +- .../sources/render_custom_geometry_source.cpp | 4 +- .../sources/render_custom_geometry_source.hpp | 2 +- .../sources/render_geojson_source.cpp | 4 +- .../sources/render_geojson_source.hpp | 2 +- .../sources/render_raster_dem_source.cpp | 4 +- .../sources/render_raster_dem_source.hpp | 2 +- .../renderer/sources/render_raster_source.cpp | 5 +- .../renderer/sources/render_raster_source.hpp | 2 +- .../renderer/sources/render_tile_source.cpp | 8 +- .../renderer/sources/render_tile_source.hpp | 4 +- .../renderer/sources/render_vector_source.cpp | 5 +- .../renderer/sources/render_vector_source.hpp | 2 +- src/mbgl/renderer/tile_parameters.hpp | 2 + src/mbgl/renderer/tile_pyramid.cpp | 4 +- src/mbgl/renderer/tile_pyramid.hpp | 2 +- src/mbgl/tile/geometry_tile.cpp | 7 +- src/mbgl/tile/geometry_tile.hpp | 2 +- src/mbgl/tile/geometry_tile_worker.cpp | 4 +- src/mbgl/tile/geometry_tile_worker.hpp | 2 + src/mbgl/tile/tile_cache.cpp | 2 +- src/mbgl/tile/tile_cache.hpp | 9 +- src/mbgl/util/thread_pool.cpp | 148 +++++++++++------- src/mbgl/util/thread_pool.hpp | 102 +++++++++--- test/actor/actor.test.cpp | 8 +- test/api/custom_drawable_layer.test.cpp | 2 +- test/include/mbgl/test/vector_tile_test.hpp | 9 +- test/style/source.test.cpp | 15 +- test/tile/tile_cache.test.cpp | 3 +- test/util/async_task.test.cpp | 8 +- test/util/thread.test.cpp | 29 +--- 67 files changed, 395 insertions(+), 302 deletions(-) diff --git a/include/mbgl/actor/actor.hpp b/include/mbgl/actor/actor.hpp index af42bec7b97..f5a4bfa10d4 100644 --- a/include/mbgl/actor/actor.hpp +++ b/include/mbgl/actor/actor.hpp @@ -14,6 +14,8 @@ namespace mbgl { +class TaggedScheduler; + /** An `Actor` is an owning reference to an asynchronous object of type `O`: an "actor". Communication with an actor happens via message passing: you send @@ -60,6 +62,11 @@ class Actor { template Actor(Scheduler& scheduler, Args&&... args) : target(scheduler, parent, std::forward(args)...) {} + + template + Actor(const TaggedScheduler& scheduler, Args&&... args) + : retainer(scheduler.get()), + target(scheduler, parent, std::forward(args)...) {} template Actor(std::shared_ptr scheduler, Args&&... args) diff --git a/include/mbgl/actor/established_actor.hpp b/include/mbgl/actor/established_actor.hpp index 3a60033672f..5246e1c61b6 100644 --- a/include/mbgl/actor/established_actor.hpp +++ b/include/mbgl/actor/established_actor.hpp @@ -33,7 +33,18 @@ class EstablishedActor { class... Args, typename std::enable_if_t || std::is_constructible_v, Args...>>* = nullptr> - EstablishedActor(Scheduler& scheduler, AspiringActor& parent_, Args&&... args) + EstablishedActor(Scheduler& scheduler, AspiringActor& parent_, Args&&... args) + : parent(parent_) { + emplaceObject(std::forward(args)...); + parent.mailbox->open(scheduler); + } + + // Construct the Object from a parameter pack `args` (i.e. `Object(args...)`) + template || + std::is_constructible_v, Args...>>* = nullptr> + EstablishedActor(const TaggedScheduler& scheduler, AspiringActor& parent_, Args&&... args) : parent(parent_) { emplaceObject(std::forward(args)...); parent.mailbox->open(scheduler); @@ -48,6 +59,11 @@ class EstablishedActor { parent.mailbox->open(scheduler); } + template >::value> + EstablishedActor(const TaggedScheduler& scheduler, AspiringActor& parent_, ArgsTuple&& args) { + EstablishedActor(*scheduler.get(), parent_, std::forward(args)); + } + EstablishedActor(const EstablishedActor&) = delete; ~EstablishedActor() { diff --git a/include/mbgl/actor/mailbox.hpp b/include/mbgl/actor/mailbox.hpp index b558062aeb3..123fa019c6d 100644 --- a/include/mbgl/actor/mailbox.hpp +++ b/include/mbgl/actor/mailbox.hpp @@ -6,6 +6,7 @@ #include #include +#include namespace mbgl { @@ -21,11 +22,13 @@ class Mailbox : public std::enable_shared_from_this { 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 { Abandoned }; + const void* schedulerTag{nullptr}; mapbox::base::WeakPtr weakScheduler; std::recursive_mutex receivingMutex; diff --git a/include/mbgl/actor/scheduler.hpp b/include/mbgl/actor/scheduler.hpp index 4ebb017cb13..badae746338 100644 --- a/include/mbgl/actor/scheduler.hpp +++ b/include/mbgl/actor/scheduler.hpp @@ -39,12 +39,15 @@ class Scheduler { /// Enqueues a function for execution. virtual void schedule(std::function&&) = 0; + virtual void schedule(const void*, std::function&&) = 0; + /// Makes a weak pointer to this Scheduler. virtual mapbox::base::WeakPtr makeWeakPtr() = 0; - /// Enqueues a function for execution on the render thread. - virtual void runOnRenderThread(std::function&&) {}; - virtual void runRenderJobs() {} - + /// Enqueues a function for execution on the render thread owned by the given tag. + virtual void runOnRenderThread(const void*, std::function&&) {} + /// 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*, [[maybe_unused]] bool closeQueue = false) {} /// Returns a closure wrapping the given one. /// /// When the returned closure is invoked for the first time, it schedules @@ -69,8 +72,7 @@ class Scheduler { /// Wait until there's nothing pending or in process /// Must not be called from a task provided to this scheduler. - /// @param timeout Time to wait, or zero to wait forever. - virtual std::size_t waitForEmpty(Milliseconds timeout = Milliseconds{0}) = 0; + virtual void waitForEmpty(const void* tag = nullptr) = 0; /// Set/Get the current Scheduler for this thread static Scheduler* GetCurrent(); @@ -116,4 +118,30 @@ class Scheduler { std::function handler; }; +/// @brief A TaggedScheduler pairs a scheduler with a memory address. Tasklets submitted via a TaggedScheduler +/// are bucketed with the tag address to enable queries on tasks related to that tag. This allows multiple map +/// instances to all use the same scheduler and await processing of all their tasks prior to map deletion. +class TaggedScheduler { +public: + TaggedScheduler() = delete; + TaggedScheduler(std::shared_ptr scheduler_, const void* tagAddr_) + : scheduler(std::move(scheduler_)), + tagAddr(tagAddr_) {} + TaggedScheduler(const TaggedScheduler&) = default; + + /// @brief Get the wrapped scheduler + /// @return + const std::shared_ptr& get() const noexcept { return scheduler; } + const void* tag() const noexcept { return tagAddr; } + + void schedule(std::function&& fn) { scheduler->schedule(tagAddr, std::move(fn)); } + void runOnRenderThread(std::function&& fn) { scheduler->runOnRenderThread(tagAddr, std::move(fn)); } + void runRenderJobs(bool closeQueue = false) { scheduler->runRenderJobs(tagAddr, closeQueue); } + void waitForEmpty() const noexcept { scheduler->waitForEmpty(tagAddr); } + +private: + std::shared_ptr scheduler; + const void* tagAddr; +}; + } // namespace mbgl diff --git a/include/mbgl/gfx/context.hpp b/include/mbgl/gfx/context.hpp index d2300a4d3c2..652deb3cbde 100644 --- a/include/mbgl/gfx/context.hpp +++ b/include/mbgl/gfx/context.hpp @@ -54,8 +54,7 @@ using VertexAttributeArrayPtr = std::shared_ptr; class Context { protected: Context(uint32_t maximumVertexBindingCount_) - : maximumVertexBindingCount(maximumVertexBindingCount_), - backgroundScheduler(Scheduler::GetBackground()) {} + : maximumVertexBindingCount(maximumVertexBindingCount_) {} public: static constexpr const uint32_t minimumRequiredVertexBindingCount = 8; @@ -174,8 +173,6 @@ class Context { virtual std::unique_ptr createDrawScopeResource() = 0; gfx::RenderingStats stats; - - std::shared_ptr backgroundScheduler; }; } // namespace gfx diff --git a/include/mbgl/gfx/renderer_backend.hpp b/include/mbgl/gfx/renderer_backend.hpp index 5d8b74f00d0..d4daae04d92 100644 --- a/include/mbgl/gfx/renderer_backend.hpp +++ b/include/mbgl/gfx/renderer_backend.hpp @@ -1,5 +1,6 @@ #pragma once +#include #include #include @@ -8,6 +9,7 @@ namespace mbgl { class ProgramParameters; +class Map; namespace gfx { @@ -31,6 +33,9 @@ class RendererBackend { RendererBackend(const RendererBackend&) = delete; RendererBackend& operator=(const RendererBackend&) = delete; + // Return the background thread pool assigned to this backend + TaggedScheduler& getThreadPool() noexcept { return threadPool; } + /// Returns the device's context. Context& getContext(); @@ -70,6 +75,7 @@ class RendererBackend { std::unique_ptr context; const ContextMode contextMode; std::once_flag initialized; + TaggedScheduler threadPool; friend class BackendScope; }; diff --git a/include/mbgl/mtl/renderer_backend.hpp b/include/mbgl/mtl/renderer_backend.hpp index b519bb69d48..b4d22cddc67 100644 --- a/include/mbgl/mtl/renderer_backend.hpp +++ b/include/mbgl/mtl/renderer_backend.hpp @@ -21,7 +21,7 @@ using FramebufferID = uint32_t; class RendererBackend : public gfx::RendererBackend { public: - RendererBackend(gfx::ContextMode); + RendererBackend(gfx::ContextMode, mbgl::Map*); ~RendererBackend() override; /// Called prior to rendering to update the internally assumed MetalMetal state. diff --git a/include/mbgl/util/run_loop.hpp b/include/mbgl/util/run_loop.hpp index 841814638ab..ee230314ede 100644 --- a/include/mbgl/util/run_loop.hpp +++ b/include/mbgl/util/run_loop.hpp @@ -79,9 +79,10 @@ class RunLoop : public Scheduler, private util::noncopyable { } void schedule(std::function&& fn) override { invoke(std::move(fn)); } + void schedule(const void*, std::function&& fn) override { schedule(std::move(fn)); } ::mapbox::base::WeakPtr makeWeakPtr() override { return weakFactory.makeWeakPtr(); } - std::size_t waitForEmpty(Milliseconds timeout) override; + void waitForEmpty(const void* tag = nullptr) override; class Impl; diff --git a/platform/android/MapLibreAndroid/src/cpp/map_renderer.cpp b/platform/android/MapLibreAndroid/src/cpp/map_renderer.cpp index 472b96d14c2..0af25b6e100 100644 --- a/platform/android/MapLibreAndroid/src/cpp/map_renderer.cpp +++ b/platform/android/MapLibreAndroid/src/cpp/map_renderer.cpp @@ -84,20 +84,17 @@ void MapRenderer::schedule(std::function&& scheduled) { } } -std::size_t MapRenderer::waitForEmpty(Milliseconds timeout) { +void MapRenderer::waitForEmpty([[maybe_unused]] const void* tag) { try { android::UniqueEnv _env = android::AttachEnv(); static auto& javaClass = jni::Class::Singleton(*_env); - static auto waitForEmpty = javaClass.GetMethod(*_env, "waitForEmpty"); + static auto waitForEmpty = javaClass.GetMethod(*_env, "waitForEmpty"); if (auto weakReference = javaPeer.get(*_env)) { - return weakReference.Call(*_env, waitForEmpty, static_cast(timeout.count())); + return weakReference.Call(*_env, waitForEmpty); } - // If the peer is already cleaned up, there's nothing to wait for - return 0; } catch (...) { Log::Error(Event::Android, "MapRenderer::waitForEmpty failed"); jni::ThrowJavaError(*android::AttachEnv(), std::current_exception()); - return 0; } } diff --git a/platform/android/MapLibreAndroid/src/cpp/map_renderer.hpp b/platform/android/MapLibreAndroid/src/cpp/map_renderer.hpp index 4ecb8f5d94b..40d09fb9f03 100644 --- a/platform/android/MapLibreAndroid/src/cpp/map_renderer.hpp +++ b/platform/android/MapLibreAndroid/src/cpp/map_renderer.hpp @@ -67,11 +67,12 @@ class MapRenderer : public Scheduler { // From Scheduler. Schedules by using callbacks to the // JVM to process the mailbox on the right thread. void schedule(std::function&& scheduled) override; + void schedule(const void*, std::function&& fn) override { schedule(std::move(fn)); }; + mapbox::base::WeakPtr makeWeakPtr() override { return weakFactory.makeWeakPtr(); } // Wait for the queue to be empty - // A timeout of zero results in an unbounded wait - std::size_t waitForEmpty(Milliseconds timeout) override; + void waitForEmpty(const void*) override; void requestRender(); diff --git a/platform/android/MapLibreAndroid/src/main/java/org/maplibre/android/maps/renderer/MapRenderer.java b/platform/android/MapLibreAndroid/src/main/java/org/maplibre/android/maps/renderer/MapRenderer.java index 72c539040de..7be40232479 100644 --- a/platform/android/MapLibreAndroid/src/main/java/org/maplibre/android/maps/renderer/MapRenderer.java +++ b/platform/android/MapLibreAndroid/src/main/java/org/maplibre/android/maps/renderer/MapRenderer.java @@ -120,11 +120,6 @@ void queueEvent(MapRendererRunnable runnable) { this.queueEvent((Runnable) runnable); } - /// Wait indefinitely for the queue to become empty - public void waitForEmpty() { - waitForEmpty(0); - } - private native void nativeInitialize(MapRenderer self, float pixelRatio, String localIdeographFontFamily); diff --git a/platform/android/MapLibreAndroid/src/main/java/org/maplibre/android/maps/renderer/MapRendererScheduler.java b/platform/android/MapLibreAndroid/src/main/java/org/maplibre/android/maps/renderer/MapRendererScheduler.java index f1e17e658f7..15184736f53 100644 --- a/platform/android/MapLibreAndroid/src/main/java/org/maplibre/android/maps/renderer/MapRendererScheduler.java +++ b/platform/android/MapLibreAndroid/src/main/java/org/maplibre/android/maps/renderer/MapRendererScheduler.java @@ -16,7 +16,4 @@ public interface MapRendererScheduler { @Keep void waitForEmpty(); - - @Keep - long waitForEmpty(long timeoutMillis); } diff --git a/platform/android/MapLibreAndroid/src/main/java/org/maplibre/android/maps/renderer/glsurfaceview/GLSurfaceViewMapRenderer.java b/platform/android/MapLibreAndroid/src/main/java/org/maplibre/android/maps/renderer/glsurfaceview/GLSurfaceViewMapRenderer.java index a8a3c3fb41c..b47fa38e35d 100644 --- a/platform/android/MapLibreAndroid/src/main/java/org/maplibre/android/maps/renderer/glsurfaceview/GLSurfaceViewMapRenderer.java +++ b/platform/android/MapLibreAndroid/src/main/java/org/maplibre/android/maps/renderer/glsurfaceview/GLSurfaceViewMapRenderer.java @@ -120,7 +120,7 @@ public void queueEvent(Runnable runnable) { * {@inheritDoc} */ @Override - public long waitForEmpty(long timeoutMillis) { - return glSurfaceView.waitForEmpty(timeoutMillis); + public void waitForEmpty() { + glSurfaceView.waitForEmpty(); } } \ No newline at end of file diff --git a/platform/android/MapLibreAndroid/src/main/java/org/maplibre/android/maps/renderer/glsurfaceview/MapLibreGLSurfaceView.java b/platform/android/MapLibreAndroid/src/main/java/org/maplibre/android/maps/renderer/glsurfaceview/MapLibreGLSurfaceView.java index a78a8a907a4..a58ba011307 100644 --- a/platform/android/MapLibreAndroid/src/main/java/org/maplibre/android/maps/renderer/glsurfaceview/MapLibreGLSurfaceView.java +++ b/platform/android/MapLibreAndroid/src/main/java/org/maplibre/android/maps/renderer/glsurfaceview/MapLibreGLSurfaceView.java @@ -322,8 +322,8 @@ public void queueEvent(Runnable r) { * @param timeoutMillis Timeout in milliseconds * @return Number of queue items remaining */ - public long waitForEmpty(long timeoutMillis) { - return glThread.waitForEmpty(timeoutMillis); + public void waitForEmpty() { + glThread.waitForEmpty(); } @@ -1038,31 +1038,16 @@ public void queueEvent(@NonNull Runnable r) { * @param timeoutMillis Timeout in milliseconds, zero for indefinite wait * @return Number of queue items remaining */ - public int waitForEmpty(long timeoutMillis) { - final long startTime = System.nanoTime(); + public void waitForEmpty() { synchronized (glThreadManager) { // Wait for the queue to be empty while (!this.eventQueue.isEmpty()) { - if (timeoutMillis > 0) { - final long elapsedMillis = (System.nanoTime() - startTime) / 1000 / 1000; - if (elapsedMillis < timeoutMillis) { - try { - glThreadManager.wait(timeoutMillis - elapsedMillis); - } catch (InterruptedException ex) { - Thread.currentThread().interrupt(); - } - } else { - break; - } - } else { - try { - glThreadManager.wait(); - } catch (InterruptedException ex) { - Thread.currentThread().interrupt(); - } + try { + glThreadManager.wait(); + } catch (InterruptedException ex) { + Thread.currentThread().interrupt(); } } - return this.eventQueue.size(); } } diff --git a/platform/android/MapLibreAndroid/src/main/java/org/maplibre/android/maps/renderer/textureview/TextureViewMapRenderer.java b/platform/android/MapLibreAndroid/src/main/java/org/maplibre/android/maps/renderer/textureview/TextureViewMapRenderer.java index ee20aae6089..22a9415a6e0 100644 --- a/platform/android/MapLibreAndroid/src/main/java/org/maplibre/android/maps/renderer/textureview/TextureViewMapRenderer.java +++ b/platform/android/MapLibreAndroid/src/main/java/org/maplibre/android/maps/renderer/textureview/TextureViewMapRenderer.java @@ -91,8 +91,8 @@ public void queueEvent(Runnable runnable) { * {@inheritDoc} */ @Override - public long waitForEmpty(long timeoutMillis) { - return renderThread.waitForEmpty(timeoutMillis); + public void waitForEmpty() { + renderThread.waitForEmpty(); } /** diff --git a/platform/android/MapLibreAndroid/src/main/java/org/maplibre/android/maps/renderer/textureview/TextureViewRenderThread.java b/platform/android/MapLibreAndroid/src/main/java/org/maplibre/android/maps/renderer/textureview/TextureViewRenderThread.java index 51598b967ee..11d8a8d918f 100644 --- a/platform/android/MapLibreAndroid/src/main/java/org/maplibre/android/maps/renderer/textureview/TextureViewRenderThread.java +++ b/platform/android/MapLibreAndroid/src/main/java/org/maplibre/android/maps/renderer/textureview/TextureViewRenderThread.java @@ -141,31 +141,16 @@ void queueEvent(@NonNull Runnable runnable) { * @return The number of items remaining in the queue */ @UiThread - int waitForEmpty(long timeoutMillis) { - final long startTime = System.nanoTime(); + void waitForEmpty() { synchronized (lock) { // Wait for the queue to be empty while (!this.eventQueue.isEmpty()) { - if (timeoutMillis > 0) { - final long elapsedMillis = (System.nanoTime() - startTime) / 1000 / 1000; - if (elapsedMillis < timeoutMillis) { - try { - lock.wait(timeoutMillis - elapsedMillis); - } catch (InterruptedException ex) { - Thread.currentThread().interrupt(); - } - } else { - break; - } - } else { - try { - lock.wait(0); - } catch (InterruptedException ex) { - Thread.currentThread().interrupt(); - } + try { + lock.wait(0); + } catch (InterruptedException ex) { + Thread.currentThread().interrupt(); } } - return this.eventQueue.size(); } } diff --git a/platform/android/MapLibreAndroidTestApp/src/androidTest/java/org/maplibre/android/maps/NativeMapViewTest.kt b/platform/android/MapLibreAndroidTestApp/src/androidTest/java/org/maplibre/android/maps/NativeMapViewTest.kt index 307bd4a8611..49e2bb3f824 100644 --- a/platform/android/MapLibreAndroidTestApp/src/androidTest/java/org/maplibre/android/maps/NativeMapViewTest.kt +++ b/platform/android/MapLibreAndroidTestApp/src/androidTest/java/org/maplibre/android/maps/NativeMapViewTest.kt @@ -443,9 +443,8 @@ class NativeMapViewTest : AppCenter() { // no-op } - override fun waitForEmpty(timeoutMillis: Long): Long { + override fun waitForEmpty() { // no-op - return 0 } } } diff --git a/platform/android/src/run_loop.cpp b/platform/android/src/run_loop.cpp index a26c91ead0d..f8adf1ed0a2 100644 --- a/platform/android/src/run_loop.cpp +++ b/platform/android/src/run_loop.cpp @@ -216,8 +216,7 @@ Milliseconds RunLoop::Impl::processRunnables() { return timeout; } -std::size_t RunLoop::Impl::waitForEmpty(Milliseconds timeout) { - const auto startTime = mbgl::util::MonotonicTimer::now(); +void RunLoop::Impl::waitForEmpty() { while (true) { std::size_t remaining; { @@ -225,10 +224,8 @@ std::size_t RunLoop::Impl::waitForEmpty(Milliseconds timeout) { remaining = runnables.size(); } - const auto elapsed = mbgl::util::MonotonicTimer::now() - startTime; - const auto elapsedMillis = std::chrono::duration_cast(elapsed); - if (remaining == 0 || (Milliseconds::zero() < timeout && timeout <= elapsedMillis)) { - return remaining; + if (remaining == 0) { + return; } runLoop->runOnce(); @@ -257,8 +254,8 @@ void RunLoop::wake() { impl->wake(); } -std::size_t RunLoop::waitForEmpty(std::chrono::milliseconds timeout) { - return impl->waitForEmpty(timeout); +void RunLoop::waitForEmpty([[maybe_unused]] const void* tag) { + impl->waitForEmpty(); } void RunLoop::run() { diff --git a/platform/android/src/run_loop_impl.hpp b/platform/android/src/run_loop_impl.hpp index ff2420cd02e..4f0eff4103a 100644 --- a/platform/android/src/run_loop_impl.hpp +++ b/platform/android/src/run_loop_impl.hpp @@ -42,7 +42,7 @@ class RunLoop::Impl { Milliseconds processRunnables(); - std::size_t waitForEmpty(Milliseconds timeout); + void waitForEmpty(); ALooper* loop = nullptr; RunLoop* runLoop = nullptr; diff --git a/platform/darwin/src/run_loop.cpp b/platform/darwin/src/run_loop.cpp index bf5bf71d0bb..8243c48e680 100644 --- a/platform/darwin/src/run_loop.cpp +++ b/platform/darwin/src/run_loop.cpp @@ -47,8 +47,7 @@ void RunLoop::stop() { invoke([&] { CFRunLoopStop(CFRunLoopGetCurrent()); }); } -std::size_t RunLoop::waitForEmpty(Milliseconds timeout) { - const auto startTime = mbgl::util::MonotonicTimer::now(); +void RunLoop::waitForEmpty([[maybe_unused]] const void* tag) { while (true) { std::size_t remaining; { @@ -56,10 +55,8 @@ std::size_t RunLoop::waitForEmpty(Milliseconds timeout) { remaining = defaultQueue.size() + highPriorityQueue.size(); } - const auto elapsed = mbgl::util::MonotonicTimer::now() - startTime; - const auto elapsedMillis = std::chrono::duration_cast(elapsed); - if (remaining == 0 || (Milliseconds::zero() < timeout && timeout <= elapsedMillis)) { - return remaining; + if (remaining == 0) { + return; } runOnce(); diff --git a/platform/default/src/mbgl/util/run_loop.cpp b/platform/default/src/mbgl/util/run_loop.cpp index 39d31d87b74..f69b830a9dc 100644 --- a/platform/default/src/mbgl/util/run_loop.cpp +++ b/platform/default/src/mbgl/util/run_loop.cpp @@ -149,8 +149,7 @@ void RunLoop::stop() { invoke([&] { uv_unref(impl->holderHandle()); }); } -std::size_t RunLoop::waitForEmpty(Milliseconds timeout) { - const auto startTime = mbgl::util::MonotonicTimer::now(); +void RunLoop::waitForEmpty([[maybe_unused]] const void* tag) { while (true) { std::size_t remaining; { @@ -158,10 +157,8 @@ std::size_t RunLoop::waitForEmpty(Milliseconds timeout) { remaining = defaultQueue.size() + highPriorityQueue.size(); } - const auto elapsed = mbgl::util::MonotonicTimer::now() - startTime; - const auto elapsedMillis = std::chrono::duration_cast(elapsed); - if (remaining == 0 || (Milliseconds::zero() < timeout && timeout <= elapsedMillis)) { - return remaining; + if (remaining == 0) { + return; } runOnce(); diff --git a/platform/qt/src/mbgl/run_loop.cpp b/platform/qt/src/mbgl/run_loop.cpp index c978bd81ea5..3f26c806e6e 100644 --- a/platform/qt/src/mbgl/run_loop.cpp +++ b/platform/qt/src/mbgl/run_loop.cpp @@ -91,8 +91,7 @@ void RunLoop::runOnce() { } } -std::size_t RunLoop::waitForEmpty(std::chrono::milliseconds timeout) { - const auto startTime = mbgl::util::MonotonicTimer::now(); +void RunLoop::waitForEmpty([[maybe_unused]] const void* tag) { while (true) { std::size_t remaining; { @@ -100,10 +99,8 @@ std::size_t RunLoop::waitForEmpty(std::chrono::milliseconds timeout) { remaining = defaultQueue.size() + highPriorityQueue.size(); } - const auto elapsed = mbgl::util::MonotonicTimer::now() - startTime; - const auto elapsedMillis = std::chrono::duration_cast(elapsed); - if (remaining == 0 || timeout <= elapsedMillis) { - return remaining; + if (remaining == 0) { + return; } runOnce(); diff --git a/platform/qt/src/utils/scheduler.cpp b/platform/qt/src/utils/scheduler.cpp index 45e83926151..4d8157c0985 100644 --- a/platform/qt/src/utils/scheduler.cpp +++ b/platform/qt/src/utils/scheduler.cpp @@ -42,23 +42,19 @@ void Scheduler::processEvents() { cvEmpty.notify_all(); } -std::size_t Scheduler::waitForEmpty(std::chrono::milliseconds timeout) { +void Scheduler::waitForEmpty([[maybe_unused]] const void* tag) { MBGL_VERIFY_THREAD(tid); - const auto startTime = mbgl::util::MonotonicTimer::now(); std::unique_lock lock(m_taskQueueMutex); const auto isDone = [&] { return m_taskQueue.empty() && pendingItems == 0; }; + while (!isDone()) { - const auto elapsed = mbgl::util::MonotonicTimer::now() - startTime; - if (timeout <= elapsed || !cvEmpty.wait_for(lock, timeout - elapsed, isDone)) { - assert(isDone()); - break; - } + cvEmpty.wait(lock); } - return m_taskQueue.size() + pendingItems; + assert(m_taskQueue.size() + pendingItems == 0); } } // namespace QMapLibre diff --git a/platform/qt/src/utils/scheduler.hpp b/platform/qt/src/utils/scheduler.hpp index 56f9fd954d7..e23f0b589a1 100644 --- a/platform/qt/src/utils/scheduler.hpp +++ b/platform/qt/src/utils/scheduler.hpp @@ -22,7 +22,7 @@ class Scheduler : public QObject, public mbgl::Scheduler { // mbgl::Scheduler implementation. void schedule(std::function&& function) final; - std::size_t waitForEmpty(std::chrono::milliseconds timeout) override; + void waitForEmpty(const void* tag = nullptr) override; mapbox::base::WeakPtr makeWeakPtr() override { return weakFactory.makeWeakPtr(); } diff --git a/src/mbgl/actor/mailbox.cpp b/src/mbgl/actor/mailbox.cpp index ad63f4bc5d3..85c236354db 100644 --- a/src/mbgl/actor/mailbox.cpp +++ b/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) { 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())); } } diff --git a/src/mbgl/annotation/render_annotation_source.cpp b/src/mbgl/annotation/render_annotation_source.cpp index ec60eecbe68..10421d62dea 100644 --- a/src/mbgl/annotation/render_annotation_source.cpp +++ b/src/mbgl/annotation/render_annotation_source.cpp @@ -10,8 +10,8 @@ namespace mbgl { using namespace style; RenderAnnotationSource::RenderAnnotationSource(Immutable impl_, - std::shared_ptr threadPool_) - : RenderTileSource(std::move(impl_), std::move(threadPool_)) { + const TaggedScheduler& threadPool_) + : RenderTileSource(std::move(impl_), threadPool_) { assert(LayerManager::annotationsEnabled); tilePyramid.setObserver(this); } diff --git a/src/mbgl/annotation/render_annotation_source.hpp b/src/mbgl/annotation/render_annotation_source.hpp index d195ea9cb27..f8065b9221a 100644 --- a/src/mbgl/annotation/render_annotation_source.hpp +++ b/src/mbgl/annotation/render_annotation_source.hpp @@ -7,7 +7,7 @@ namespace mbgl { class RenderAnnotationSource final : public RenderTileSource { public: - explicit RenderAnnotationSource(Immutable, std::shared_ptr); + explicit RenderAnnotationSource(Immutable, const TaggedScheduler&); void update(Immutable, const std::vector>&, diff --git a/src/mbgl/gfx/renderer_backend.cpp b/src/mbgl/gfx/renderer_backend.cpp index b24a66a27be..15cb6c446b0 100644 --- a/src/mbgl/gfx/renderer_backend.cpp +++ b/src/mbgl/gfx/renderer_backend.cpp @@ -6,7 +6,8 @@ namespace mbgl { namespace gfx { RendererBackend::RendererBackend(const ContextMode contextMode_) - : contextMode(contextMode_) {} + : contextMode(contextMode_), + threadPool(Scheduler::GetBackground(), static_cast(this)) {} RendererBackend::~RendererBackend() = default; gfx::Context& RendererBackend::getContext() { diff --git a/src/mbgl/gl/context.cpp b/src/mbgl/gl/context.cpp index 8632134d25a..0ff2dc5caa4 100644 --- a/src/mbgl/gl/context.cpp +++ b/src/mbgl/gl/context.cpp @@ -82,7 +82,7 @@ Context::Context(RendererBackend& backend_) Context::~Context() noexcept { if (cleanupOnDestruction) { - Scheduler::GetBackground()->runRenderJobs(); + backend.getThreadPool().runRenderJobs(true /* closeQueue */); reset(); #if !defined(NDEBUG) @@ -93,7 +93,7 @@ Context::~Context() noexcept { } void Context::beginFrame() { - Scheduler::GetBackground()->runRenderJobs(); + backend.getThreadPool().runRenderJobs(); #if MLN_DRAWABLE_RENDERER frameInFlightFence = std::make_shared(); diff --git a/src/mbgl/gl/renderer_backend.cpp b/src/mbgl/gl/renderer_backend.cpp index c1b61afc1b0..9b10b502b2c 100644 --- a/src/mbgl/gl/renderer_backend.cpp +++ b/src/mbgl/gl/renderer_backend.cpp @@ -19,7 +19,7 @@ RendererBackend::RendererBackend(const gfx::ContextMode contextMode_) : gfx::RendererBackend(contextMode_) {} std::unique_ptr RendererBackend::createContext() { - auto result = std::make_unique(*this); + auto result = std::make_unique(*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; diff --git a/src/mbgl/mtl/context.cpp b/src/mbgl/mtl/context.cpp index b6470ec1335..b26f2b93d8b 100644 --- a/src/mbgl/mtl/context.cpp +++ b/src/mbgl/mtl/context.cpp @@ -44,7 +44,7 @@ Context::Context(RendererBackend& backend_) Context::~Context() noexcept { if (cleanupOnDestruction) { - Scheduler::GetBackground()->runRenderJobs(); + backend.getThreadPool().runRenderJobs(true /* closeQueue */); performCleanup(); emptyVertexBuffer.reset(); @@ -68,7 +68,7 @@ Context::~Context() noexcept { } void Context::beginFrame() { - Scheduler::GetBackground()->runRenderJobs(); + backend.getThreadPool().runRenderJobs(); } void Context::endFrame() {} diff --git a/src/mbgl/mtl/renderer_backend.cpp b/src/mbgl/mtl/renderer_backend.cpp index 6af61d9dc09..83b4b9dfc76 100644 --- a/src/mbgl/mtl/renderer_backend.cpp +++ b/src/mbgl/mtl/renderer_backend.cpp @@ -40,8 +40,8 @@ namespace mbgl { namespace mtl { -RendererBackend::RendererBackend(const gfx::ContextMode contextMode_) - : gfx::RendererBackend(contextMode_), +RendererBackend::RendererBackend(const gfx::ContextMode contextMode_, mbgl::Map* map) + : gfx::RendererBackend(contextMode_, map), device(NS::TransferPtr(MTL::CreateSystemDefaultDevice())), commandQueue(NS::TransferPtr(device->newCommandQueue())) { assert(device); @@ -51,7 +51,8 @@ RendererBackend::RendererBackend(const gfx::ContextMode contextMode_) RendererBackend::~RendererBackend() = default; std::unique_ptr RendererBackend::createContext() { - return std::make_unique(*this); + return std::make_unique(*this, + TaggedScheduler{Scheduler::GetBackground(), static_cast(owner)}); } PremultipliedImage RendererBackend::readFramebuffer(const Size& size) { diff --git a/src/mbgl/renderer/render_orchestrator.cpp b/src/mbgl/renderer/render_orchestrator.cpp index f06cc50d7eb..515f2eb8bd3 100644 --- a/src/mbgl/renderer/render_orchestrator.cpp +++ b/src/mbgl/renderer/render_orchestrator.cpp @@ -111,7 +111,7 @@ class RenderTreeImpl final : public RenderTree { } // namespace -RenderOrchestrator::RenderOrchestrator(bool backgroundLayerAsColor_, const std::optional& localFontFamily_) +RenderOrchestrator::RenderOrchestrator(bool backgroundLayerAsColor_, TaggedScheduler& threadPool_, const std::optional& localFontFamily_) : observer(&nullObserver()), glyphManager(std::make_unique(std::make_unique(localFontFamily_))), imageManager(std::make_unique()), @@ -122,7 +122,7 @@ RenderOrchestrator::RenderOrchestrator(bool backgroundLayerAsColor_, const std:: layerImpls(makeMutable>>()), renderLight(makeMutable()), backgroundLayerAsColor(backgroundLayerAsColor_), - threadPool(Scheduler::GetBackground()) { + threadPool(threadPool_) { glyphManager->setObserver(this); imageManager->setObserver(this); } @@ -142,9 +142,7 @@ RenderOrchestrator::~RenderOrchestrator() { // Wait for any deferred cleanup tasks to complete before releasing and potentially // destroying the scheduler. Those cleanup tasks must not hold the final reference // to the scheduler because it cannot be destroyed from one of its own pool threads. - constexpr auto deferredCleanupTimeout = Milliseconds{1000}; - [[maybe_unused]] const auto remaining = threadPool->waitForEmpty(deferredCleanupTimeout); - assert(remaining == 0); + threadPool.waitForEmpty(); } void RenderOrchestrator::setObserver(RendererObserver* observer_) { @@ -189,7 +187,8 @@ std::unique_ptr RenderOrchestrator::createRenderTree( updateParameters->annotationManager, imageManager, glyphManager, - updateParameters->prefetchZoomDelta}; + updateParameters->prefetchZoomDelta, + threadPool}; glyphManager->setURL(updateParameters->glyphURL); diff --git a/src/mbgl/renderer/render_orchestrator.hpp b/src/mbgl/renderer/render_orchestrator.hpp index 69ff4667e6b..e035a817dce 100644 --- a/src/mbgl/renderer/render_orchestrator.hpp +++ b/src/mbgl/renderer/render_orchestrator.hpp @@ -2,6 +2,7 @@ #if MLN_DRAWABLE_RENDERER #include #endif +#include #include #include #include @@ -55,7 +56,7 @@ using ImmutableLayer = Immutable; class RenderOrchestrator final : public GlyphManagerObserver, public ImageManagerObserver, public RenderSourceObserver { public: - RenderOrchestrator(bool backgroundLayerAsColor_, const std::optional& localFontFamily_); + RenderOrchestrator(bool backgroundLayerAsColor_, TaggedScheduler& threadPool_, const std::optional& localFontFamily_); ~RenderOrchestrator() override; void markContextLost() { contextLost = true; }; @@ -210,7 +211,7 @@ class RenderOrchestrator final : public GlyphManagerObserver, public ImageManage RenderLayerReferences orderedLayers; RenderLayerReferences layersNeedPlacement; - std::shared_ptr threadPool; + TaggedScheduler threadPool; #if MLN_DRAWABLE_RENDERER std::vector> pendingChanges; diff --git a/src/mbgl/renderer/render_source.cpp b/src/mbgl/renderer/render_source.cpp index 2eb5ce900e2..8c0481e38b4 100644 --- a/src/mbgl/renderer/render_source.cpp +++ b/src/mbgl/renderer/render_source.cpp @@ -21,7 +21,7 @@ namespace mbgl { using namespace style; std::unique_ptr RenderSource::create(const Immutable& impl, - std::shared_ptr threadPool_) { + const TaggedScheduler& threadPool_) { switch (impl->type) { case SourceType::Vector: return std::make_unique(staticImmutableCast(impl), diff --git a/src/mbgl/renderer/render_source.hpp b/src/mbgl/renderer/render_source.hpp index 4b46e07756b..d113f22745c 100644 --- a/src/mbgl/renderer/render_source.hpp +++ b/src/mbgl/renderer/render_source.hpp @@ -1,5 +1,6 @@ #pragma once +#include #include #include #include @@ -48,7 +49,7 @@ using RenderTiles = std::shared_ptr create(const Immutable&, std::shared_ptr); + static std::unique_ptr create(const Immutable&, const TaggedScheduler&); ~RenderSource() override; bool isEnabled() const; diff --git a/src/mbgl/renderer/renderer_impl.cpp b/src/mbgl/renderer/renderer_impl.cpp index 2477b1d63e4..c27bef787f1 100644 --- a/src/mbgl/renderer/renderer_impl.cpp +++ b/src/mbgl/renderer/renderer_impl.cpp @@ -58,7 +58,7 @@ RendererObserver& nullObserver() { Renderer::Impl::Impl(gfx::RendererBackend& backend_, float pixelRatio_, const std::optional& localFontFamily_) - : orchestrator(!backend_.contextIsShared(), localFontFamily_), + : orchestrator(!backend_.contextIsShared(), backend_.getThreadPool(), localFontFamily_), backend(backend_), observer(&nullObserver()), pixelRatio(pixelRatio_) {} diff --git a/src/mbgl/renderer/sources/render_custom_geometry_source.cpp b/src/mbgl/renderer/sources/render_custom_geometry_source.cpp index 7f054bfa2c2..13377ef42fd 100644 --- a/src/mbgl/renderer/sources/render_custom_geometry_source.cpp +++ b/src/mbgl/renderer/sources/render_custom_geometry_source.cpp @@ -8,8 +8,8 @@ namespace mbgl { using namespace style; RenderCustomGeometrySource::RenderCustomGeometrySource(Immutable impl_, - std::shared_ptr threadPool_) - : RenderTileSource(std::move(impl_), std::move(threadPool_)) { + const TaggedScheduler& threadPool_) + : RenderTileSource(std::move(impl_), threadPool_) { tilePyramid.setObserver(this); } diff --git a/src/mbgl/renderer/sources/render_custom_geometry_source.hpp b/src/mbgl/renderer/sources/render_custom_geometry_source.hpp index 4c77c6a1973..9ed8dc5f6b7 100644 --- a/src/mbgl/renderer/sources/render_custom_geometry_source.hpp +++ b/src/mbgl/renderer/sources/render_custom_geometry_source.hpp @@ -7,7 +7,7 @@ namespace mbgl { class RenderCustomGeometrySource final : public RenderTileSource { public: - explicit RenderCustomGeometrySource(Immutable, std::shared_ptr); + explicit RenderCustomGeometrySource(Immutable, const TaggedScheduler&); void update(Immutable, const std::vector>&, diff --git a/src/mbgl/renderer/sources/render_geojson_source.cpp b/src/mbgl/renderer/sources/render_geojson_source.cpp index 633df708879..1e91e5f8c7b 100644 --- a/src/mbgl/renderer/sources/render_geojson_source.cpp +++ b/src/mbgl/renderer/sources/render_geojson_source.cpp @@ -66,8 +66,8 @@ MAPBOX_ETERNAL_CONSTEXPR const auto extensionGetters = } // namespace RenderGeoJSONSource::RenderGeoJSONSource(Immutable impl_, - std::shared_ptr threadPool_) - : RenderTileSource(std::move(impl_), std::move(threadPool_)) {} + const TaggedScheduler& threadPool_) + : RenderTileSource(std::move(impl_), threadPool_) {} RenderGeoJSONSource::~RenderGeoJSONSource() = default; diff --git a/src/mbgl/renderer/sources/render_geojson_source.hpp b/src/mbgl/renderer/sources/render_geojson_source.hpp index f0c41c2ab50..d7c9c4b8cda 100644 --- a/src/mbgl/renderer/sources/render_geojson_source.hpp +++ b/src/mbgl/renderer/sources/render_geojson_source.hpp @@ -11,7 +11,7 @@ class GeoJSONData; class RenderGeoJSONSource final : public RenderTileSource { public: - explicit RenderGeoJSONSource(Immutable, std::shared_ptr); + explicit RenderGeoJSONSource(Immutable, const TaggedScheduler&); ~RenderGeoJSONSource() override; void update(Immutable, diff --git a/src/mbgl/renderer/sources/render_raster_dem_source.cpp b/src/mbgl/renderer/sources/render_raster_dem_source.cpp index 6f41426141e..8cb322ea20f 100644 --- a/src/mbgl/renderer/sources/render_raster_dem_source.cpp +++ b/src/mbgl/renderer/sources/render_raster_dem_source.cpp @@ -11,8 +11,8 @@ namespace mbgl { using namespace style; RenderRasterDEMSource::RenderRasterDEMSource(Immutable impl_, - std::shared_ptr threadPool_) - : RenderTileSetSource(std::move(impl_), std::move(threadPool_)) {} + const TaggedScheduler& threadPool_) + : RenderTileSetSource(std::move(impl_), threadPool_) {} const style::RasterSource::Impl& RenderRasterDEMSource::impl() const { return static_cast(*baseImpl); diff --git a/src/mbgl/renderer/sources/render_raster_dem_source.hpp b/src/mbgl/renderer/sources/render_raster_dem_source.hpp index 1599b76ef5b..8531c6be86c 100644 --- a/src/mbgl/renderer/sources/render_raster_dem_source.hpp +++ b/src/mbgl/renderer/sources/render_raster_dem_source.hpp @@ -7,7 +7,7 @@ namespace mbgl { class RenderRasterDEMSource final : public RenderTileSetSource { public: - explicit RenderRasterDEMSource(Immutable, std::shared_ptr); + explicit RenderRasterDEMSource(Immutable, const TaggedScheduler&); std::unordered_map> queryRenderedFeatures( const ScreenLineString& geometry, diff --git a/src/mbgl/renderer/sources/render_raster_source.cpp b/src/mbgl/renderer/sources/render_raster_source.cpp index db99831e02c..30bf341512a 100644 --- a/src/mbgl/renderer/sources/render_raster_source.cpp +++ b/src/mbgl/renderer/sources/render_raster_source.cpp @@ -8,9 +8,8 @@ namespace mbgl { using namespace style; -RenderRasterSource::RenderRasterSource(Immutable impl_, - std::shared_ptr threadPool_) - : RenderTileSetSource(std::move(impl_), std::move(threadPool_)) {} +RenderRasterSource::RenderRasterSource(Immutable impl_, const TaggedScheduler& threadPool_) + : RenderTileSetSource(std::move(impl_), threadPool_) {} inline const style::RasterSource::Impl& RenderRasterSource::impl() const { return static_cast(*baseImpl); diff --git a/src/mbgl/renderer/sources/render_raster_source.hpp b/src/mbgl/renderer/sources/render_raster_source.hpp index c827c8a7e0f..d650e974eee 100644 --- a/src/mbgl/renderer/sources/render_raster_source.hpp +++ b/src/mbgl/renderer/sources/render_raster_source.hpp @@ -7,7 +7,7 @@ namespace mbgl { class RenderRasterSource final : public RenderTileSetSource { public: - explicit RenderRasterSource(Immutable, std::shared_ptr); + explicit RenderRasterSource(Immutable, const TaggedScheduler&); private: void prepare(const SourcePrepareParameters&) final; diff --git a/src/mbgl/renderer/sources/render_tile_source.cpp b/src/mbgl/renderer/sources/render_tile_source.cpp index 61761204c42..af7e137a10b 100644 --- a/src/mbgl/renderer/sources/render_tile_source.cpp +++ b/src/mbgl/renderer/sources/render_tile_source.cpp @@ -365,9 +365,9 @@ void TileSourceRenderItem::updateDebugDrawables(DebugLayerGroupMap& debugLayerGr } #endif -RenderTileSource::RenderTileSource(Immutable impl_, std::shared_ptr threadPool_) +RenderTileSource::RenderTileSource(Immutable impl_, const TaggedScheduler& threadPool_) : RenderSource(std::move(impl_)), - tilePyramid(std::move(threadPool_)), + tilePyramid(threadPool_), renderTiles(makeMutable>()) { tilePyramid.setObserver(this); } @@ -486,8 +486,8 @@ void RenderTileSource::dumpDebugLogs() const { // RenderTileSetSource implementation -RenderTileSetSource::RenderTileSetSource(Immutable impl_, std::shared_ptr threadPool_) - : RenderTileSource(std::move(impl_), std::move(threadPool_)) {} +RenderTileSetSource::RenderTileSetSource(Immutable impl_, const TaggedScheduler& threadPool_) + : RenderTileSource(std::move(impl_), threadPool_) {} RenderTileSetSource::~RenderTileSetSource() = default; diff --git a/src/mbgl/renderer/sources/render_tile_source.hpp b/src/mbgl/renderer/sources/render_tile_source.hpp index 160ed9541da..a755bdd41bc 100644 --- a/src/mbgl/renderer/sources/render_tile_source.hpp +++ b/src/mbgl/renderer/sources/render_tile_source.hpp @@ -51,7 +51,7 @@ class RenderTileSource : public RenderSource { void dumpDebugLogs() const override; protected: - RenderTileSource(Immutable, std::shared_ptr); + RenderTileSource(Immutable, const TaggedScheduler&); TilePyramid tilePyramid; Immutable> renderTiles; mutable RenderTiles filteredRenderTiles; @@ -67,7 +67,7 @@ class RenderTileSource : public RenderSource { */ class RenderTileSetSource : public RenderTileSource { protected: - RenderTileSetSource(Immutable, std::shared_ptr); + RenderTileSetSource(Immutable, const TaggedScheduler&); ~RenderTileSetSource() override; virtual void updateInternal(const Tileset&, diff --git a/src/mbgl/renderer/sources/render_vector_source.cpp b/src/mbgl/renderer/sources/render_vector_source.cpp index 212338db670..c53447eb839 100644 --- a/src/mbgl/renderer/sources/render_vector_source.cpp +++ b/src/mbgl/renderer/sources/render_vector_source.cpp @@ -8,9 +8,8 @@ namespace mbgl { using namespace style; -RenderVectorSource::RenderVectorSource(Immutable impl_, - std::shared_ptr threadPool_) - : RenderTileSetSource(std::move(impl_), std::move(threadPool_)) {} +RenderVectorSource::RenderVectorSource(Immutable impl_, const TaggedScheduler& threadPool_) + : RenderTileSetSource(std::move(impl_), threadPool_) {} const std::optional& RenderVectorSource::getTileset() const { return static_cast(*baseImpl).tileset; diff --git a/src/mbgl/renderer/sources/render_vector_source.hpp b/src/mbgl/renderer/sources/render_vector_source.hpp index 326b392b698..fff332c4529 100644 --- a/src/mbgl/renderer/sources/render_vector_source.hpp +++ b/src/mbgl/renderer/sources/render_vector_source.hpp @@ -8,7 +8,7 @@ namespace mbgl { class RenderVectorSource final : public RenderTileSetSource { public: - explicit RenderVectorSource(Immutable, std::shared_ptr); + explicit RenderVectorSource(Immutable, const TaggedScheduler&); private: void updateInternal(const Tileset&, diff --git a/src/mbgl/renderer/tile_parameters.hpp b/src/mbgl/renderer/tile_parameters.hpp index b7ada67d636..75c1c152f4f 100644 --- a/src/mbgl/renderer/tile_parameters.hpp +++ b/src/mbgl/renderer/tile_parameters.hpp @@ -1,6 +1,7 @@ #pragma once #include +#include #include @@ -25,6 +26,7 @@ class TileParameters { std::shared_ptr imageManager; std::shared_ptr glyphManager; const uint8_t prefetchZoomDelta; + TaggedScheduler threadPool; }; } // namespace mbgl diff --git a/src/mbgl/renderer/tile_pyramid.cpp b/src/mbgl/renderer/tile_pyramid.cpp index 55466aa3ada..82753ccc473 100644 --- a/src/mbgl/renderer/tile_pyramid.cpp +++ b/src/mbgl/renderer/tile_pyramid.cpp @@ -24,8 +24,8 @@ using namespace style; static TileObserver nullObserver; -TilePyramid::TilePyramid(std::shared_ptr threadPool_) - : cache(std::move(threadPool_)), +TilePyramid::TilePyramid(const TaggedScheduler& threadPool_) + : cache(threadPool_), observer(&nullObserver) {} TilePyramid::~TilePyramid() = default; diff --git a/src/mbgl/renderer/tile_pyramid.hpp b/src/mbgl/renderer/tile_pyramid.hpp index bfae55f0952..5068093ffcb 100644 --- a/src/mbgl/renderer/tile_pyramid.hpp +++ b/src/mbgl/renderer/tile_pyramid.hpp @@ -29,7 +29,7 @@ class SourcePrepareParameters; class TilePyramid { public: - TilePyramid(std::shared_ptr threadPool_); + TilePyramid(const TaggedScheduler& threadPool_); ~TilePyramid(); bool isLoaded() const; diff --git a/src/mbgl/tile/geometry_tile.cpp b/src/mbgl/tile/geometry_tile.cpp index 1649b69f926..242082308fd 100644 --- a/src/mbgl/tile/geometry_tile.cpp +++ b/src/mbgl/tile/geometry_tile.cpp @@ -160,10 +160,11 @@ GeometryTile::GeometryTile(const OverscaledTileID& id_, std::string sourceID_, c : Tile(Kind::Geometry, id_), ImageRequestor(parameters.imageManager), sourceID(std::move(sourceID_)), - threadPool(Scheduler::GetBackground()), + threadPool(parameters.threadPool), mailbox(std::make_shared(*Scheduler::GetCurrent())), - worker(threadPool, + worker(parameters.threadPool, // Scheduler reference for the Actor retainer ActorRef(*this, mailbox), + parameters.threadPool, id_, sourceID, obsolete, @@ -183,7 +184,7 @@ GeometryTile::~GeometryTile() { imageManager->removeRequestor(*this); if (layoutResult) { - threadPool->runOnRenderThread( + threadPool.runOnRenderThread( [layoutResult_{std::move(layoutResult)}, atlasTextures_{std::move(atlasTextures)}]() {}); } } diff --git a/src/mbgl/tile/geometry_tile.hpp b/src/mbgl/tile/geometry_tile.hpp index 31aa70b668f..7c4a0d286d3 100644 --- a/src/mbgl/tile/geometry_tile.hpp +++ b/src/mbgl/tile/geometry_tile.hpp @@ -106,7 +106,7 @@ class GeometryTile : public Tile, public GlyphRequestor, public ImageRequestor { // Used to signal the worker that it should abandon parsing this tile as soon as possible. std::atomic obsolete{false}; - const std::shared_ptr threadPool; + TaggedScheduler threadPool; const std::shared_ptr mailbox; Actor worker; diff --git a/src/mbgl/tile/geometry_tile_worker.cpp b/src/mbgl/tile/geometry_tile_worker.cpp index ec1c444f57f..2c0adf1dedd 100644 --- a/src/mbgl/tile/geometry_tile_worker.cpp +++ b/src/mbgl/tile/geometry_tile_worker.cpp @@ -30,6 +30,7 @@ using namespace style; GeometryTileWorker::GeometryTileWorker(ActorRef self_, ActorRef parent_, + const TaggedScheduler& scheduler_, OverscaledTileID id_, std::string sourceID_, const std::atomic& obsolete_, @@ -38,6 +39,7 @@ GeometryTileWorker::GeometryTileWorker(ActorRef self_, const bool showCollisionBoxes_) : self(std::move(self_)), parent(std::move(parent_)), + scheduler(scheduler_), id(id_), sourceID(std::move(sourceID_)), obsolete(obsolete_), @@ -46,7 +48,7 @@ GeometryTileWorker::GeometryTileWorker(ActorRef self_, showCollisionBoxes(showCollisionBoxes_) {} GeometryTileWorker::~GeometryTileWorker() { - Scheduler::GetBackground()->runOnRenderThread([renderData_{std::move(renderData)}]() {}); + scheduler.runOnRenderThread([renderData_{std::move(renderData)}]() {}); } /* diff --git a/src/mbgl/tile/geometry_tile_worker.hpp b/src/mbgl/tile/geometry_tile_worker.hpp index bb169a0e846..ccd54652009 100644 --- a/src/mbgl/tile/geometry_tile_worker.hpp +++ b/src/mbgl/tile/geometry_tile_worker.hpp @@ -30,6 +30,7 @@ class GeometryTileWorker { public: GeometryTileWorker(ActorRef self, ActorRef parent, + const TaggedScheduler& scheduler_, OverscaledTileID, std::string, const std::atomic&, @@ -71,6 +72,7 @@ class GeometryTileWorker { ActorRef self; ActorRef parent; + TaggedScheduler scheduler; const OverscaledTileID id; const std::string sourceID; diff --git a/src/mbgl/tile/tile_cache.cpp b/src/mbgl/tile/tile_cache.cpp index b4d65990fcf..4dfa603a24a 100644 --- a/src/mbgl/tile/tile_cache.cpp +++ b/src/mbgl/tile/tile_cache.cpp @@ -49,7 +49,7 @@ void TileCache::deferredRelease(std::unique_ptr&& tile) { std::function func{[tile_{CaptureWrapper{std::move(tile)}}]() { }}; - threadPool->schedule(std::move(func)); + threadPool.schedule(std::move(func)); } void TileCache::add(const OverscaledTileID& key, std::unique_ptr&& tile) { diff --git a/src/mbgl/tile/tile_cache.hpp b/src/mbgl/tile/tile_cache.hpp index fe7f41f7b9f..02ad51258b1 100644 --- a/src/mbgl/tile/tile_cache.hpp +++ b/src/mbgl/tile/tile_cache.hpp @@ -1,5 +1,6 @@ #pragma once +#include #include #include @@ -9,12 +10,10 @@ namespace mbgl { -class Scheduler; - class TileCache { public: - TileCache(std::shared_ptr threadPool_, size_t size_ = 0) - : threadPool(std::move(threadPool_)), + TileCache(const TaggedScheduler& threadPool_, size_t size_ = 0) + : threadPool(threadPool_), size(size_) {} /// Change the maximum size of the cache. @@ -38,7 +37,7 @@ class TileCache { private: std::map> tiles; std::list orderedKeys; - std::shared_ptr threadPool; + TaggedScheduler threadPool; size_t size; }; diff --git a/src/mbgl/util/thread_pool.cpp b/src/mbgl/util/thread_pool.cpp index d02963bf8f9..0ddf8acdf51 100644 --- a/src/mbgl/util/thread_pool.cpp +++ b/src/mbgl/util/thread_pool.cpp @@ -11,11 +11,8 @@ namespace mbgl { ThreadedSchedulerBase::~ThreadedSchedulerBase() = default; void ThreadedSchedulerBase::terminate() { - // Run any leftover render jobs - runRenderJobs(); - { - std::lock_guard lock(mutex); + std::lock_guard lock(workerMutex); terminated = true; } @@ -36,49 +33,69 @@ std::thread ThreadedSchedulerBase::makeSchedulerThread(size_t index) { owningThreadPool.set(this); + bool didWork = true; while (true) { - std::unique_lock lock(mutex); - if (queue.empty() && !pendingItems) { - cvEmpty.notify_all(); + std::unique_lock conditionLock(workerMutex); + if (!terminated && !didWork) { + cvAvailable.wait(conditionLock); } - cvAvailable.wait(lock, [this] { return !queue.empty() || terminated; }); - if (terminated) { platform::detachThread(); - return; + break; } - auto function = std::move(queue.front()); - queue.pop(); + // Let other threads run + conditionLock.unlock(); - if (function) { - pendingItems++; + didWork = false; + std::vector> pending; + { + // 1. Gather buckets for us to visit this iteration + std::lock_guard lock(taggedQueueLock); + for (const auto& [tag, queue] : taggedQueue) { + pending.push_back(queue); + } } - lock.unlock(); + // 2. Visit a task from each + for (auto& q : pending) { + std::function tasklet; + { + std::lock_guard lock(q->lock); + if (q->queue.size()) { + tasklet = std::move(q->queue.front()); + q->queue.pop(); + } + if (!tasklet) continue; + } + + q->runningCount++; - if (function) { try { - function(); - - // destroy the function and release its captures before unblocking `waitForEmpty` - function = {}; - if (!--pendingItems) { - std::unique_lock inner_lock(mutex); - if (queue.empty()) { - cvEmpty.notify_all(); + // Indicate some processing was done this iteration. We'll try and do more work + // on the following loop until we run out of work, at which point we wait. + didWork = true; + + tasklet(); + tasklet = {}; // destroy the function and release its captures before unblocking `waitForEmpty` + + if (!--q->runningCount) { + std::lock_guard lock(q->lock); + if (q->queue.empty()) { + q->cv.notify_all(); } } } catch (...) { - std::unique_lock inner_lock(mutex); + std::lock_guard lock(q->lock); if (handler) { handler(std::current_exception()); } - function = {}; - if (!--pendingItems && queue.empty()) { - cvEmpty.notify_all(); + tasklet = {}; + + if (!--q->runningCount && q->queue.empty()) { + q->cv.notify_all(); } if (handler) { @@ -92,47 +109,62 @@ std::thread ThreadedSchedulerBase::makeSchedulerThread(size_t index) { } void ThreadedSchedulerBase::schedule(std::function&& fn) { + schedule(static_cast(this), std::move(fn)); +} + +void ThreadedSchedulerBase::schedule(const void* tag, std::function&& fn) { assert(fn); - if (fn) { - { - // We need to block if adding adding a new task from a thread not controlled by this - // pool. Tasks are added by other tasks, so we must not block a thread we do control - // or `waitForEmpty` will deadlock. - std::unique_lock addLock(addMutex, std::defer_lock); - if (!thisThreadIsOwned()) { - addLock.lock(); - } - std::lock_guard lock(mutex); - queue.push(std::move(fn)); + if (!fn) return; + + std::shared_ptr q; + { + std::lock_guard lock(taggedQueueLock); + auto it = taggedQueue.find(tag); + if (it == taggedQueue.end()) { + q = std::make_shared(); + taggedQueue.insert({tag, q}); + } else { + q = it->second; } - cvAvailable.notify_one(); } + + { + std::lock_guard lock(q->lock); + q->queue.push(std::move(fn)); + } + + cvAvailable.notify_one(); } -std::size_t ThreadedSchedulerBase::waitForEmpty(Milliseconds timeout) { +void ThreadedSchedulerBase::waitForEmpty(const void* tag) { // Must not be called from a thread in our pool, or we would deadlock assert(!thisThreadIsOwned()); if (!thisThreadIsOwned()) { - const auto startTime = util::MonotonicTimer::now(); - const auto isDone = [&] { - return queue.empty() && pendingItems == 0; - }; - // Block any other threads from adding new items - std::scoped_lock addLock(addMutex); - std::unique_lock lock(mutex); - while (!isDone()) { - if (timeout > Milliseconds::zero()) { - const auto elapsed = util::MonotonicTimer::now() - startTime; - if (timeout <= elapsed || !cvEmpty.wait_for(lock, timeout - elapsed, isDone)) { - break; - } - } else { - cvEmpty.wait(lock, isDone); + if (!tag) { + tag = static_cast(this); + } + + std::shared_ptr q; + { + std::lock_guard lock(taggedQueueLock); + auto it = taggedQueue.find(tag); + if (it == taggedQueue.end()) { + return; } + q = it->second; + } + + std::unique_lock queueLock(q->lock); + while (q->queue.size() + q->runningCount) { + q->cv.wait(queueLock); + } + + // After waiting for the queue to empty, go ahead and erase it from the map. + { + std::lock_guard lock(taggedQueueLock); + taggedQueue.erase(tag); } - return queue.size() + pendingItems; } - return 0; } } // namespace mbgl diff --git a/src/mbgl/util/thread_pool.hpp b/src/mbgl/util/thread_pool.hpp index ee0735082c9..aa0efd823ad 100644 --- a/src/mbgl/util/thread_pool.hpp +++ b/src/mbgl/util/thread_pool.hpp @@ -3,9 +3,11 @@ #include #include #include +#include #include #include +#include #include #include #include @@ -15,7 +17,15 @@ namespace mbgl { class ThreadedSchedulerBase : public Scheduler { public: - void schedule(std::function&&) override; + /// @brief Schedule a generic task not assigned to any particular owner. + /// The scheduler itself will own the task. + /// @param fn Task to run + void schedule(std::function&& fn) override; + + /// @brief Schedule a task assigned to the given owner `tag`. + /// @param tag Address of any object used to identify ownership of `fn` + /// @param fn Task to run + void schedule(const void* tag, std::function&& fn) override; protected: ThreadedSchedulerBase() = default; @@ -24,28 +34,30 @@ class ThreadedSchedulerBase : public Scheduler { void terminate(); std::thread makeSchedulerThread(size_t index); - /// Wait until there's nothing pending or in process + /// @brief Wait until there's nothing pending or in process /// Must not be called from a task provided to this scheduler. - /// @param timeout Time to wait, or zero to wait forever. - std::size_t waitForEmpty(Milliseconds timeout) override; + /// @param tag Address of the owner identifying the collection of tasks to + // wait for. Waiting on nullptr waits on tasks owned by the scheduler. + void waitForEmpty(const void* tag = nullptr) override; /// Returns true if called from a thread managed by the scheduler bool thisThreadIsOwned() const { return owningThreadPool.get() == this; } - std::queue> queue; - // protects `queue` - std::mutex mutex; - // Used to block addition of new items while waiting - std::mutex addMutex; // Signal when an item is added to the queue std::condition_variable cvAvailable; - // Signal when the queue becomes empty - std::condition_variable cvEmpty; - // Count of functions removed from the queue but still executing - std::atomic pendingItems{0}; - // Points to the owning pool in owned threads + std::mutex workerMutex; + std::mutex taggedQueueLock; util::ThreadLocal owningThreadPool; bool terminated{false}; + + // Task queues bucketed by tag address + struct Queue { + std::atomic runningCount; /* running tasks */ + std::condition_variable cv; /* queue empty condition */ + std::mutex lock; /* lock */ + std::queue> queue; /* pending task queue */ + }; + mbgl::unordered_map> taggedQueue; }; /** @@ -74,30 +86,70 @@ class ThreadedScheduler : public ThreadedSchedulerBase { } } - void runOnRenderThread(std::function&& fn) override { - std::lock_guard lock(renderMutex); - renderThreadQueue.push(std::move(fn)); + void runOnRenderThread(const void* tag, std::function&& fn) override { + std::shared_ptr queue; + { + std::lock_guard lock(taggedRenderQueueLock); + auto it = taggedRenderQueue.find(tag); + if (it != taggedRenderQueue.end()) { + queue = it->second; + } else { + queue = std::make_shared(); + taggedRenderQueue.insert({tag, queue}); + } + } + + std::lock_guard lock(queue->mutex); + queue->queue.push(std::move(fn)); } - void runRenderJobs() override { - std::lock_guard lock(renderMutex); - while (renderThreadQueue.size()) { - auto fn = std::move(renderThreadQueue.front()); - renderThreadQueue.pop(); + void runRenderJobs(const void* tag, bool closeQueue = false) override { + std::shared_ptr queue; + std::unique_lock lock(taggedRenderQueueLock); + + { + auto it = taggedRenderQueue.find(tag); + if (it != taggedRenderQueue.end()) { + queue = it->second; + } + + if (!closeQueue) { + lock.unlock(); + } + } + + if (!queue) { + return; + } + + std::lock_guard taskLock(queue->mutex); + while (queue->queue.size()) { + auto fn = std::move(queue->queue.front()); + queue->queue.pop(); if (fn) { fn(); } } + + if (closeQueue) { + // We hold both locks and can safely remove the queue entry + taggedRenderQueue.erase(tag); + } } mapbox::base::WeakPtr makeWeakPtr() override { return weakFactory.makeWeakPtr(); } private: std::vector threads; - mapbox::base::WeakPtrFactory weakFactory{this}; - std::queue> renderThreadQueue; - std::mutex renderMutex; + struct RenderQueue { + std::queue> queue; + std::mutex mutex; + }; + mbgl::unordered_map> taggedRenderQueue; + std::mutex taggedRenderQueueLock; + + mapbox::base::WeakPtrFactory weakFactory{this}; }; class SequencedScheduler : public ThreadedScheduler { diff --git a/test/actor/actor.test.cpp b/test/actor/actor.test.cpp index e0558f280a0..319a61eb1fc 100644 --- a/test/actor/actor.test.cpp +++ b/test/actor/actor.test.cpp @@ -91,10 +91,7 @@ TEST(Actor, DestructionBlocksOnSend) { ~TestScheduler() override { EXPECT_TRUE(waited.load()); } - std::size_t waitForEmpty(Milliseconds) override { - assert(false); - return 0; - } + void waitForEmpty(const void*) override { assert(false); } void schedule(std::function&&) final { promise.set_value(); @@ -102,6 +99,9 @@ TEST(Actor, DestructionBlocksOnSend) { std::this_thread::sleep_for(1ms); waited = true; } + + void schedule(const void*, std::function&& fn) final { schedule(std::move(fn)); } + mapbox::base::WeakPtr makeWeakPtr() override { return weakFactory.makeWeakPtr(); } }; diff --git a/test/api/custom_drawable_layer.test.cpp b/test/api/custom_drawable_layer.test.cpp index 8b69b3bd46e..33d36cd38f4 100644 --- a/test/api/custom_drawable_layer.test.cpp +++ b/test/api/custom_drawable_layer.test.cpp @@ -109,7 +109,7 @@ class LineTestDrawableLayer : public mbgl::style::CustomDrawableLayerHost { GeometryCoordinates polyline; for (auto ipoint{0}; ipoint < numPoints; ++ipoint) { polyline.emplace_back(ipoint * util::EXTENT / numPoints, - std::sin(ipoint * 2 * M_PI / numPoints) * util::EXTENT / numLines / 2.f); + static_cast(std::sin(ipoint * 2 * M_PI / numPoints) * util::EXTENT / numLines / 2.f)); } for (auto index{0}; index < numLines; ++index) { diff --git a/test/include/mbgl/test/vector_tile_test.hpp b/test/include/mbgl/test/vector_tile_test.hpp index 63b45aba639..7911573e506 100644 --- a/test/include/mbgl/test/vector_tile_test.hpp +++ b/test/include/mbgl/test/vector_tile_test.hpp @@ -27,7 +27,7 @@ class VectorTileTest { Tileset tileset{{"https://example.com"}, {0, 22}, "none"}; - const std::shared_ptr threadPool = Scheduler::GetBackground(); + TaggedScheduler threadPool; TileParameters tileParameters{1.0, MapDebugOptions(), @@ -39,10 +39,13 @@ class VectorTileTest { glyphManager, 0}; + VectorTileTest() + : threadPool(Scheduler::GetBackground(), this) {} + ~VectorTileTest() { // Ensure that deferred releases are complete before cleaning up - EXPECT_EQ(0, loop.waitForEmpty(Milliseconds::zero())); - EXPECT_EQ(0, threadPool->waitForEmpty()); + loop.waitForEmpty(); + threadPool.waitForEmpty(); } }; diff --git a/test/style/source.test.cpp b/test/style/source.test.cpp index 55655d0fda3..894302cef1c 100644 --- a/test/style/source.test.cpp +++ b/test/style/source.test.cpp @@ -64,7 +64,7 @@ class SourceTest { AnnotationManager annotationManager{style}; std::shared_ptr imageManager = std::make_shared(); std::shared_ptr glyphManager = std::make_shared(); - std::shared_ptr threadPool = Scheduler::GetBackground(); + TaggedScheduler threadPool; TileParameters tileParameters(MapMode mapMode = MapMode::Continuous) { return {1.0, @@ -78,7 +78,8 @@ class SourceTest { 0}; }; - SourceTest() { + SourceTest() + : threadPool(Scheduler::GetBackground(), this) { // Squelch logging. Log::setObserver(std::make_unique()); @@ -88,7 +89,7 @@ class SourceTest { transformState = transform.getState(); } - ~SourceTest() { threadPool->waitForEmpty(); } + ~SourceTest() { threadPool.waitForEmpty(); } void run() { loop.run(); } @@ -781,8 +782,8 @@ class FakeTileSource : public RenderTileSetSource { MOCK_METHOD1(tileSetNecessity, void(TileNecessity)); MOCK_METHOD1(tileSetMinimumUpdateInterval, void(Duration)); - explicit FakeTileSource(Immutable impl_, std::shared_ptr threadPool_) - : RenderTileSetSource(std::move(impl_), std::move(threadPool_)) {} + explicit FakeTileSource(Immutable impl_, const TaggedScheduler& threadPool_) + : RenderTileSetSource(std::move(impl_), threadPool_) {} void updateInternal(const Tileset& tileset, const std::vector>& layers, const bool needsRendering, @@ -885,8 +886,8 @@ TEST(Source, RenderTileSetSourceUpdate) { class FakeRenderTileSetSource : public RenderTileSetSource { public: - explicit FakeRenderTileSetSource(Immutable impl_, std::shared_ptr threadPool_) - : RenderTileSetSource(std::move(impl_), std::move(threadPool_)) {} + explicit FakeRenderTileSetSource(Immutable impl_, TaggedScheduler threadPool_) + : RenderTileSetSource(std::move(impl_), threadPool_) {} MOCK_METHOD0(mockedUpdateInternal, void()); diff --git a/test/tile/tile_cache.test.cpp b/test/tile/tile_cache.test.cpp index 51c0c92ef8f..825ce087ae0 100644 --- a/test/tile/tile_cache.test.cpp +++ b/test/tile/tile_cache.test.cpp @@ -45,7 +45,8 @@ class VectorTileMock : public VectorTile { TEST(TileCache, Smoke) { VectorTileTest test; - TileCache cache(Scheduler::GetBackground(), 1); + TaggedScheduler scheduler(Scheduler::GetBackground(), &test); + TileCache cache(scheduler, 1); const OverscaledTileID id(0, 0, 0); auto tile = std::make_unique(id, "source", test.tileParameters, test.tileset); diff --git a/test/util/async_task.test.cpp b/test/util/async_task.test.cpp index e264a4ceb83..e8b2f4c39e5 100644 --- a/test/util/async_task.test.cpp +++ b/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 retainer = Scheduler::GetBackground(); - auto mailbox = std::make_shared(*retainer); + TaggedScheduler retainer = {Scheduler::GetBackground(), &loop}; + auto mailbox = std::make_shared(retainer); TestWorker worker(&async); ActorRef workerRef(worker, mailbox); @@ -133,8 +133,8 @@ TEST(AsyncTask, ThreadSafety) { AsyncTask async([&count] { ++count; }); - std::shared_ptr retainer = Scheduler::GetBackground(); - auto mailbox = std::make_shared(*retainer); + TaggedScheduler retainer = {Scheduler::GetBackground(), &loop}; + auto mailbox = std::make_shared(retainer); TestWorker worker(&async); ActorRef workerRef(worker, mailbox); diff --git a/test/util/thread.test.cpp b/test/util/thread.test.cpp index 01bfce2304b..c647dfbf1bf 100644 --- a/test/util/thread.test.cpp +++ b/test/util/thread.test.cpp @@ -343,7 +343,7 @@ TEST(Thread, PoolWait) { pool->schedule([&] { std::this_thread::sleep_for(Milliseconds(100)); }); } - EXPECT_EQ(0, pool->waitForEmpty()); + pool->waitForEmpty(); } TEST(Thread, PoolWaitRecursiveAdd) { @@ -358,7 +358,7 @@ TEST(Thread, PoolWaitRecursiveAdd) { std::this_thread::sleep_for(Milliseconds(10)); }); - EXPECT_EQ(0, pool->waitForEmpty()); + pool->waitForEmpty(); } TEST(Thread, PoolWaitAdd) { @@ -385,25 +385,10 @@ TEST(Thread, PoolWaitAdd) { // more items would be added by the sequential task if not blocked pool->schedule([&] { std::this_thread::sleep_for(Milliseconds(100)); }); - EXPECT_EQ(0, pool->waitForEmpty()); + pool->waitForEmpty(); addActive = false; - EXPECT_EQ(0, pool->waitForEmpty()); -} - -TEST(Thread, PoolWaitTimeout) { - auto pool = Scheduler::GetBackground(); - - std::mutex mutex; - { - std::lock_guard outerLock(mutex); - pool->schedule([&] { std::lock_guard innerLock(mutex); }); - - // should always time out - EXPECT_EQ(1, pool->waitForEmpty(Milliseconds(100))); - } - - EXPECT_EQ(0, pool->waitForEmpty()); + pool->waitForEmpty(); } TEST(Thread, PoolWaitException) { @@ -425,7 +410,7 @@ TEST(Thread, PoolWaitException) { } // Exceptions shouldn't cause deadlocks by, e.g., abandoning locks. - EXPECT_EQ(0, pool->waitForEmpty()); + pool->waitForEmpty(); EXPECT_EQ(threadCount, caught); } @@ -434,8 +419,8 @@ TEST(Thread, WrongThread) { auto pool = Scheduler::GetBackground(); // Asserts in debug builds, silently ignored in release. - pool->schedule([&] { EXPECT_EQ(0, pool->waitForEmpty()); }); + pool->schedule([&] { pool->waitForEmpty(); }); - EXPECT_EQ(0, pool->waitForEmpty()); + pool->waitForEmpty(); } #endif