From c4c75452b66af07e3cec5474de753dd591afca49 Mon Sep 17 00:00:00 2001 From: mwilsnd <53413200+mwilsnd@users.noreply.github.com> Date: Fri, 28 Jun 2024 17:20:03 -0400 Subject: [PATCH] Add TaggedScheduler, couple lifetime of tasks and orchestrator (#2398) --- include/mbgl/actor/actor.hpp | 7 + include/mbgl/actor/established_actor.hpp | 16 ++ include/mbgl/actor/mailbox.hpp | 6 +- include/mbgl/actor/scheduler.hpp | 66 ++++++-- include/mbgl/gfx/context.hpp | 5 +- include/mbgl/gfx/renderer_backend.hpp | 8 + include/mbgl/gl/renderer_backend.hpp | 1 + include/mbgl/renderer/renderer_frontend.hpp | 3 + include/mbgl/style/sources/geojson_source.hpp | 3 +- include/mbgl/style/style.hpp | 3 +- include/mbgl/util/identity.hpp | 9 +- include/mbgl/util/run_loop.hpp | 3 +- .../src/cpp/android_renderer_backend.cpp | 2 +- .../src/cpp/android_renderer_backend.hpp | 2 +- .../src/cpp/android_renderer_frontend.cpp | 4 + .../src/cpp/android_renderer_frontend.hpp | 2 + .../MapLibreAndroid/src/cpp/map_renderer.cpp | 12 +- .../MapLibreAndroid/src/cpp/map_renderer.hpp | 10 +- .../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/MLNRendererFrontend.h | 5 + platform/darwin/src/run_loop.cpp | 9 +- .../include/mbgl/gfx/headless_frontend.hpp | 1 + .../src/mbgl/gfx/headless_frontend.cpp | 4 + .../default/src/mbgl/map/map_snapshotter.cpp | 6 + platform/default/src/mbgl/util/run_loop.cpp | 9 +- platform/glfw/glfw_renderer_frontend.cpp | 5 + platform/glfw/glfw_renderer_frontend.hpp | 1 + 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 | 12 +- .../annotation/render_annotation_source.cpp | 4 +- .../annotation/render_annotation_source.hpp | 2 +- src/mbgl/gfx/renderer_backend.cpp | 8 +- src/mbgl/gl/context.cpp | 4 +- src/mbgl/gl/renderer_backend.cpp | 6 +- src/mbgl/map/map_impl.cpp | 2 +- src/mbgl/mtl/context.cpp | 4 +- src/mbgl/renderer/render_orchestrator.cpp | 18 +-- src/mbgl/renderer/render_orchestrator.hpp | 7 +- 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/sprite/sprite_loader.cpp | 40 ++--- src/mbgl/sprite/sprite_loader.hpp | 6 +- src/mbgl/style/sources/geojson_source.cpp | 57 +++---- .../style/sources/geojson_source_impl.cpp | 16 +- src/mbgl/style/style.cpp | 4 +- src/mbgl/style/style_impl.cpp | 4 +- src/mbgl/style/style_impl.hpp | 3 +- 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/raster_dem_tile.cpp | 6 +- src/mbgl/tile/raster_dem_tile.hpp | 2 +- src/mbgl/tile/raster_tile.cpp | 6 +- src/mbgl/tile/raster_tile.hpp | 2 +- src/mbgl/tile/tile_cache.cpp | 10 +- src/mbgl/tile/tile_cache.hpp | 24 ++- src/mbgl/util/thread_pool.cpp | 147 +++++++++++------- src/mbgl/util/thread_pool.hpp | 102 +++++++++--- test/actor/actor.test.cpp | 10 +- test/api/custom_drawable_layer.test.cpp | 5 +- test/include/mbgl/test/vector_tile_test.hpp | 34 ++-- test/sprite/sprite_loader.test.cpp | 13 +- test/style/source.test.cpp | 22 +-- test/style/style.test.cpp | 12 +- test/style/style_layer.test.cpp | 4 +- test/tile/custom_geometry_tile.test.cpp | 25 +-- test/tile/geojson_tile.test.cpp | 25 +-- test/tile/raster_dem_tile.test.cpp | 25 +-- test/tile/raster_tile.test.cpp | 25 +-- test/tile/tile_cache.test.cpp | 74 ++++----- test/util/async_task.test.cpp | 12 +- test/util/thread.test.cpp | 79 +++++++--- 99 files changed, 759 insertions(+), 489 deletions(-) diff --git a/include/mbgl/actor/actor.hpp b/include/mbgl/actor/actor.hpp index af42bec7b..071217e04 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 @@ -61,6 +63,11 @@ public: 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) : retainer(std::move(scheduler)), diff --git a/include/mbgl/actor/established_actor.hpp b/include/mbgl/actor/established_actor.hpp index 3a6003367..6cfa755f3 100644 --- a/include/mbgl/actor/established_actor.hpp +++ b/include/mbgl/actor/established_actor.hpp @@ -39,6 +39,17 @@ public: 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); + } + // Construct the `Object` from a tuple containing the constructor arguments // (i.e. `Object(std::get<0>(args), std::get<1>(args), ...)`) template >::value> @@ -48,6 +59,11 @@ public: 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 b558062ae..14df53b5f 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 @@ public: 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 @@ private: Abandoned }; + util::SimpleIdentity schedulerTag = util::SimpleIdentity::Empty; mapbox::base::WeakPtr weakScheduler; std::recursive_mutex receivingMutex; diff --git a/include/mbgl/actor/scheduler.hpp b/include/mbgl/actor/scheduler.hpp index 4ebb017cb..b604f60de 100644 --- a/include/mbgl/actor/scheduler.hpp +++ b/include/mbgl/actor/scheduler.hpp @@ -1,6 +1,7 @@ #pragma once #include +#include #include @@ -39,12 +40,17 @@ public: /// Enqueues a function for execution. virtual void schedule(std::function&&) = 0; + virtual void schedule(const util::SimpleIdentity, 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 util::SimpleIdentity, std::function&&) {} + /// Run render thread jobs for the given tag + /// @param tag Tag of owner + /// @param closeQueue Runs all render jobs and then removes the internal queue. + virtual void runRenderJobs([[maybe_unused]] const util::SimpleIdentity tag, + [[maybe_unused]] bool closeQueue = false) {} /// Returns a closure wrapping the given one. /// /// When the returned closure is invoked for the first time, it schedules @@ -62,15 +68,14 @@ public: /// The |TaskFn| return type must be compatible with the |ReplyFn| argument /// type. Note: the task result is copied and passed by value. template - void scheduleAndReplyValue(const TaskFn& task, const ReplyFn& reply) { + void scheduleAndReplyValue(const util::SimpleIdentity tag, TaskFn&& task, ReplyFn&& reply) { assert(GetCurrent()); - scheduleAndReplyValue(task, reply, GetCurrent()->makeWeakPtr()); + scheduleAndReplyValue(tag, task, reply, GetCurrent()->makeWeakPtr()); } /// 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 util::SimpleIdentity = util::SimpleIdentity::Empty) = 0; /// Set/Get the current Scheduler for this thread static Scheduler* GetCurrent(); @@ -99,21 +104,50 @@ public: protected: template - void scheduleAndReplyValue(const TaskFn& task, + void scheduleAndReplyValue(const util::SimpleIdentity tag, + const TaskFn& task, const ReplyFn& reply, mapbox::base::WeakPtr replyScheduler) { - auto scheduled = [replyScheduler = std::move(replyScheduler), task, reply] { + schedule(tag, [replyScheduler = std::move(replyScheduler), tag, task, reply] { auto lock = replyScheduler.lock(); if (!replyScheduler) return; - auto scheduledReply = [reply, result = task()] { - reply(result); - }; - replyScheduler->schedule(std::move(scheduledReply)); - }; - schedule(std::move(scheduled)); + replyScheduler->schedule(tag, [reply, result = task()] { reply(result); }); + }); } std::function handler; }; +/// @brief A TaggedScheduler pairs a scheduler with an identifier. Tasklets submitted via a TaggedScheduler +/// are bucketed with the tag 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 util::SimpleIdentity tag_) + : tag(tag_), + scheduler(std::move(scheduler_)) {} + TaggedScheduler(const TaggedScheduler&) = default; + + /// @brief Get the wrapped scheduler + /// @return + const std::shared_ptr& get() const noexcept { return scheduler; } + + void schedule(std::function&& fn) { scheduler->schedule(tag, std::move(fn)); } + void runOnRenderThread(std::function&& fn) { scheduler->runOnRenderThread(tag, std::move(fn)); } + void runRenderJobs(bool closeQueue = false) { scheduler->runRenderJobs(tag, closeQueue); } + void waitForEmpty() const noexcept { scheduler->waitForEmpty(tag); } + + /// type. Note: the task result is copied and passed by value. + template + void scheduleAndReplyValue(TaskFn&& task, ReplyFn&& reply) { + scheduler->scheduleAndReplyValue(tag, task, reply); + } + + const mbgl::util::SimpleIdentity tag; + +private: + std::shared_ptr scheduler; +}; + } // namespace mbgl diff --git a/include/mbgl/gfx/context.hpp b/include/mbgl/gfx/context.hpp index d2300a4d3..652deb3cb 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 @@ protected: 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 5d8b74f00..5c6e76057 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 { @@ -25,12 +27,16 @@ enum class ContextMode : bool { class RendererBackend { protected: explicit RendererBackend(ContextMode); + RendererBackend(ContextMode, const TaggedScheduler&); public: virtual ~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(); @@ -48,6 +54,7 @@ public: /// One-time shader initialization virtual void initShaders(gfx::ShaderRegistry&, const ProgramParameters&) = 0; #endif + const mbgl::util::SimpleIdentity uniqueID; protected: virtual std::unique_ptr createContext() = 0; @@ -70,6 +77,7 @@ protected: std::unique_ptr context; const ContextMode contextMode; std::once_flag initialized; + TaggedScheduler threadPool; friend class BackendScope; }; diff --git a/include/mbgl/gl/renderer_backend.hpp b/include/mbgl/gl/renderer_backend.hpp index 0d64a585b..145c1f78f 100644 --- a/include/mbgl/gl/renderer_backend.hpp +++ b/include/mbgl/gl/renderer_backend.hpp @@ -17,6 +17,7 @@ using FramebufferID = uint32_t; class RendererBackend : public gfx::RendererBackend { public: RendererBackend(gfx::ContextMode); + RendererBackend(gfx::ContextMode, const TaggedScheduler&); ~RendererBackend() override; /// Called prior to rendering to update the internally assumed OpenGL state. diff --git a/include/mbgl/renderer/renderer_frontend.hpp b/include/mbgl/renderer/renderer_frontend.hpp index 3ccf5e0f9..108bf0a16 100644 --- a/include/mbgl/renderer/renderer_frontend.hpp +++ b/include/mbgl/renderer/renderer_frontend.hpp @@ -4,6 +4,7 @@ #include #endif +#include #include namespace mbgl { @@ -30,6 +31,8 @@ public: /// Coalescing updates is up to the implementer virtual void update(std::shared_ptr) = 0; + virtual const TaggedScheduler& getThreadPool() const = 0; + protected: }; diff --git a/include/mbgl/style/sources/geojson_source.hpp b/include/mbgl/style/sources/geojson_source.hpp index 273ec7d38..fe3524988 100644 --- a/include/mbgl/style/sources/geojson_source.hpp +++ b/include/mbgl/style/sources/geojson_source.hpp @@ -41,7 +41,7 @@ public: using TileFeatures = mapbox::feature::feature_collection; using Features = mapbox::feature::feature_collection; static std::shared_ptr create(const GeoJSON&, - std::shared_ptr scheduler, + std::shared_ptr sequencedScheduler, const Immutable& = GeoJSONOptions::defaultOptions()); virtual ~GeoJSONData() = default; @@ -80,7 +80,6 @@ protected: private: std::optional url; std::unique_ptr req; - std::shared_ptr threadPool; std::shared_ptr sequencedScheduler; mapbox::base::WeakPtrFactory weakFactory{this}; // Do not add members here, see `WeakPtrFactory` diff --git a/include/mbgl/style/style.hpp b/include/mbgl/style/style.hpp index 8d7faab5d..0d2d45719 100644 --- a/include/mbgl/style/style.hpp +++ b/include/mbgl/style/style.hpp @@ -1,5 +1,6 @@ #pragma once +#include #include #include #include @@ -22,7 +23,7 @@ class Layer; class Style { public: - Style(std::shared_ptr, float pixelRatio); + Style(std::shared_ptr, float pixelRatio, const TaggedScheduler& threadPool_); ~Style(); void loadJSON(const std::string&); diff --git a/include/mbgl/util/identity.hpp b/include/mbgl/util/identity.hpp index cd667d2d1..f6a64f0c4 100644 --- a/include/mbgl/util/identity.hpp +++ b/include/mbgl/util/identity.hpp @@ -27,7 +27,7 @@ public: bool operator!=(const SimpleIdentity& other) const noexcept { return uniqueID != other.uniqueID; } std::int64_t id() const noexcept { return uniqueID; } - bool isEmpty() const noexcept { return uniqueID != emptyID; } + bool isEmpty() const noexcept { return uniqueID == emptyID; } operator bool() const noexcept { return isEmpty(); } bool operator!() const noexcept { return !isEmpty(); } @@ -58,3 +58,10 @@ inline std::string toString(const util::SimpleIdentity& ident) { } // namespace util } // namespace mbgl + +template <> +struct std::hash { + std::size_t operator()(const mbgl::util::SimpleIdentity& s) const noexcept { + return std::hash{}(s.id()); + } +}; diff --git a/include/mbgl/util/run_loop.hpp b/include/mbgl/util/run_loop.hpp index 841814638..16d6d6240 100644 --- a/include/mbgl/util/run_loop.hpp +++ b/include/mbgl/util/run_loop.hpp @@ -79,9 +79,10 @@ public: } void schedule(std::function&& fn) override { invoke(std::move(fn)); } + void schedule(const util::SimpleIdentity, 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 util::SimpleIdentity = util::SimpleIdentity::Empty) override; class Impl; diff --git a/platform/android/MapLibreAndroid/src/cpp/android_renderer_backend.cpp b/platform/android/MapLibreAndroid/src/cpp/android_renderer_backend.cpp index c20b21ced..2eb099de0 100644 --- a/platform/android/MapLibreAndroid/src/cpp/android_renderer_backend.cpp +++ b/platform/android/MapLibreAndroid/src/cpp/android_renderer_backend.cpp @@ -28,7 +28,7 @@ private: AndroidRendererBackend& backend; }; -AndroidRendererBackend::AndroidRendererBackend() +AndroidRendererBackend::AndroidRendererBackend(const TaggedScheduler& threadPool) : gl::RendererBackend(gfx::ContextMode::Unique), mbgl::gfx::Renderable({64, 64}, std::make_unique(*this)) {} diff --git a/platform/android/MapLibreAndroid/src/cpp/android_renderer_backend.hpp b/platform/android/MapLibreAndroid/src/cpp/android_renderer_backend.hpp index 9b06a6cca..2e93bc6ab 100644 --- a/platform/android/MapLibreAndroid/src/cpp/android_renderer_backend.hpp +++ b/platform/android/MapLibreAndroid/src/cpp/android_renderer_backend.hpp @@ -8,7 +8,7 @@ namespace android { class AndroidRendererBackend : public gl::RendererBackend, public mbgl::gfx::Renderable { public: - AndroidRendererBackend(); + AndroidRendererBackend(const TaggedScheduler& threadPool); ~AndroidRendererBackend() override; void updateViewPort(); diff --git a/platform/android/MapLibreAndroid/src/cpp/android_renderer_frontend.cpp b/platform/android/MapLibreAndroid/src/cpp/android_renderer_frontend.cpp index 481e81221..db4a5ae69 100644 --- a/platform/android/MapLibreAndroid/src/cpp/android_renderer_frontend.cpp +++ b/platform/android/MapLibreAndroid/src/cpp/android_renderer_frontend.cpp @@ -82,6 +82,10 @@ void AndroidRendererFrontend::update(std::shared_ptr params) { updateAsyncTask->send(); } +const TaggedScheduler& AndroidRendererFrontend::getThreadPool() const { + return mapRenderer.getThreadPool(); +} + void AndroidRendererFrontend::reduceMemoryUse() { mapRenderer.actor().invoke(&Renderer::reduceMemoryUse); } diff --git a/platform/android/MapLibreAndroid/src/cpp/android_renderer_frontend.hpp b/platform/android/MapLibreAndroid/src/cpp/android_renderer_frontend.hpp index 4dca8ac3b..383580fb4 100644 --- a/platform/android/MapLibreAndroid/src/cpp/android_renderer_frontend.hpp +++ b/platform/android/MapLibreAndroid/src/cpp/android_renderer_frontend.hpp @@ -37,6 +37,8 @@ public: void update(std::shared_ptr) override; + const TaggedScheduler& getThreadPool() const override; + // Feature querying std::vector queryRenderedFeatures(const ScreenCoordinate&, const RenderedQueryOptions&) const; std::vector queryRenderedFeatures(const ScreenBox&, const RenderedQueryOptions&) const; diff --git a/platform/android/MapLibreAndroid/src/cpp/map_renderer.cpp b/platform/android/MapLibreAndroid/src/cpp/map_renderer.cpp index 472b96d14..174f10572 100644 --- a/platform/android/MapLibreAndroid/src/cpp/map_renderer.cpp +++ b/platform/android/MapLibreAndroid/src/cpp/map_renderer.cpp @@ -22,6 +22,7 @@ MapRenderer::MapRenderer(jni::JNIEnv& _env, pixelRatio(pixelRatio_), localIdeographFontFamily(localIdeographFontFamily_ ? jni::Make(_env, localIdeographFontFamily_) : std::optional{}), + threadPool(Scheduler::GetBackground(), {}), mailboxData(this) {} MapRenderer::MailboxData::MailboxData(Scheduler* scheduler_) @@ -84,20 +85,17 @@ void MapRenderer::schedule(std::function&& scheduled) { } } -std::size_t MapRenderer::waitForEmpty(Milliseconds timeout) { +void MapRenderer::waitForEmpty([[maybe_unused]] const util::SimpleIdentity 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; } } @@ -221,7 +219,7 @@ void MapRenderer::onSurfaceCreated(JNIEnv&) { backend.reset(); // Create the new backend and renderer - backend = std::make_unique(); + backend = std::make_unique(threadPool); renderer = std::make_unique(*backend, pixelRatio, localIdeographFontFamily); rendererRef = std::make_unique>(*renderer, mailboxData.getMailbox()); diff --git a/platform/android/MapLibreAndroid/src/cpp/map_renderer.hpp b/platform/android/MapLibreAndroid/src/cpp/map_renderer.hpp index 4ecb8f5d9..d7ad24589 100644 --- a/platform/android/MapLibreAndroid/src/cpp/map_renderer.hpp +++ b/platform/android/MapLibreAndroid/src/cpp/map_renderer.hpp @@ -67,11 +67,12 @@ public: // 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 util::SimpleIdentity, 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 util::SimpleIdentity tag) override; void requestRender(); @@ -79,6 +80,9 @@ public: using SnapshotCallback = std::function; void requestSnapshot(SnapshotCallback); + AndroidRendererBackend& getRendererBackend() const { return *backend; } + const TaggedScheduler& getThreadPool() const { return threadPool; } + protected: // Called from the GL Thread // @@ -119,7 +123,7 @@ private: float pixelRatio; std::optional localIdeographFontFamily; - std::shared_ptr threadPool; + TaggedScheduler threadPool; const MailboxData mailboxData; std::mutex initialisationMutex; 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 72c539040..7be402324 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 @@ public abstract class MapRenderer implements MapRendererScheduler { 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 f1e17e658..15184736f 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 a8a3c3fb4..b47fa38e3 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 class GLSurfaceViewMapRenderer extends MapRenderer implements GLSurfaceVi * {@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 a78a8a907..a58ba0113 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 class MapLibreGLSurfaceView extends SurfaceView implements SurfaceHolder. * @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 class MapLibreGLSurfaceView extends SurfaceView implements SurfaceHolder. * @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 ee20aae60..22a9415a6 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 class TextureViewMapRenderer extends MapRenderer { * {@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 51598b967..11d8a8d91 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 @@ class TextureViewRenderThread extends Thread implements TextureView.SurfaceTextu * @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 72d0c1262..974d4093e 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 @@ -444,9 +444,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 a26c91ead..c7efe9e0d 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 SimpleIdentity 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 ff2420cd0..4f0eff410 100644 --- a/platform/android/src/run_loop_impl.hpp +++ b/platform/android/src/run_loop_impl.hpp @@ -42,7 +42,7 @@ public: Milliseconds processRunnables(); - std::size_t waitForEmpty(Milliseconds timeout); + void waitForEmpty(); ALooper* loop = nullptr; RunLoop* runLoop = nullptr; diff --git a/platform/darwin/src/MLNRendererFrontend.h b/platform/darwin/src/MLNRendererFrontend.h index 401fdd312..c631dda7c 100644 --- a/platform/darwin/src/MLNRendererFrontend.h +++ b/platform/darwin/src/MLNRendererFrontend.h @@ -1,3 +1,4 @@ +#include #include #include #include @@ -33,6 +34,10 @@ class MLNRenderFrontend : public mbgl::RendererFrontend { } } + const mbgl::TaggedScheduler& getThreadPool() const override { + return mbglBackend.getThreadPool(); + } + void setObserver(mbgl::RendererObserver& observer) override { if (!renderer) return; renderer->setObserver(&observer); diff --git a/platform/darwin/src/run_loop.cpp b/platform/darwin/src/run_loop.cpp index 7779b2c98..a8619116a 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 SimpleIdentity 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/include/mbgl/gfx/headless_frontend.hpp b/platform/default/include/mbgl/gfx/headless_frontend.hpp index ff961ffc1..6c2d30a3f 100644 --- a/platform/default/include/mbgl/gfx/headless_frontend.hpp +++ b/platform/default/include/mbgl/gfx/headless_frontend.hpp @@ -37,6 +37,7 @@ public: void reset() override; void update(std::shared_ptr) override; + const TaggedScheduler& getThreadPool() const override; void setObserver(RendererObserver&) override; double getFrameTime() const; diff --git a/platform/default/src/mbgl/gfx/headless_frontend.cpp b/platform/default/src/mbgl/gfx/headless_frontend.cpp index 8efe4cb5b..3760706bc 100644 --- a/platform/default/src/mbgl/gfx/headless_frontend.cpp +++ b/platform/default/src/mbgl/gfx/headless_frontend.cpp @@ -48,6 +48,10 @@ void HeadlessFrontend::update(std::shared_ptr updateParameters } } +const TaggedScheduler& HeadlessFrontend::getThreadPool() const { + return backend->getRendererBackend()->getThreadPool(); +} + void HeadlessFrontend::setObserver(RendererObserver& observer_) { assert(renderer); renderer->setObserver(&observer_); diff --git a/platform/default/src/mbgl/map/map_snapshotter.cpp b/platform/default/src/mbgl/map/map_snapshotter.cpp index 56da5d787..e42fe0719 100644 --- a/platform/default/src/mbgl/map/map_snapshotter.cpp +++ b/platform/default/src/mbgl/map/map_snapshotter.cpp @@ -118,6 +118,8 @@ public: return std::move(stillImage); } + const TaggedScheduler& getThreadPool() const { return frontend.getThreadPool(); } + private: PremultipliedImage stillImage; bool hasPendingStillImageRequest = false; @@ -156,6 +158,10 @@ public: PremultipliedImage takeImage() { return renderer->actor().ask(&SnapshotterRenderer::takeImage).get(); } + const mbgl::TaggedScheduler& getThreadPool() const override { + return renderer->actor().ask(&SnapshotterRenderer::getThreadPool).get(); + } + private: std::shared_ptr updateParameters; const std::unique_ptr> renderer; diff --git a/platform/default/src/mbgl/util/run_loop.cpp b/platform/default/src/mbgl/util/run_loop.cpp index 39d31d87b..54705e481 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 mbgl::util::SimpleIdentity 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/glfw/glfw_renderer_frontend.cpp b/platform/glfw/glfw_renderer_frontend.cpp index 0e06a1663..a79814f01 100644 --- a/platform/glfw/glfw_renderer_frontend.cpp +++ b/platform/glfw/glfw_renderer_frontend.cpp @@ -2,6 +2,7 @@ #include #include +#include #include GLFWRendererFrontend::GLFWRendererFrontend(std::unique_ptr renderer_, GLFWView& glfwView_) @@ -27,6 +28,10 @@ void GLFWRendererFrontend::update(std::shared_ptr params glfwView.invalidate(); } +const mbgl::TaggedScheduler& GLFWRendererFrontend::getThreadPool() const { + return glfwView.getRendererBackend().getThreadPool(); +} + void GLFWRendererFrontend::render() { MLN_TRACE_FUNC(); diff --git a/platform/glfw/glfw_renderer_frontend.hpp b/platform/glfw/glfw_renderer_frontend.hpp index 08e649f6e..252e26676 100644 --- a/platform/glfw/glfw_renderer_frontend.hpp +++ b/platform/glfw/glfw_renderer_frontend.hpp @@ -18,6 +18,7 @@ public: void setObserver(mbgl::RendererObserver&) override; void update(std::shared_ptr) override; + const mbgl::TaggedScheduler& getThreadPool() const override; void render(); mbgl::Renderer* getRenderer(); diff --git a/platform/qt/src/mbgl/run_loop.cpp b/platform/qt/src/mbgl/run_loop.cpp index c978bd81e..ea1828dea 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 mbgl::util::SimpleIdentity 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 45e839261..7ab5364c5 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 mbgl::util::SimpleIdentity 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 56f9fd954..089acdbbf 100644 --- a/platform/qt/src/utils/scheduler.hpp +++ b/platform/qt/src/utils/scheduler.hpp @@ -22,7 +22,7 @@ public: // mbgl::Scheduler implementation. void schedule(std::function&& function) final; - std::size_t waitForEmpty(std::chrono::milliseconds timeout) override; + void waitForEmpty(const mbgl::util::SimpleIdentity tag = mbgl::util::SimpleIdentity::Empty) override; mapbox::base::WeakPtr makeWeakPtr() override { return weakFactory.makeWeakPtr(); } diff --git a/src/mbgl/actor/mailbox.cpp b/src/mbgl/actor/mailbox.cpp index 55ba8899e..ef54c6bb7 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); @@ -90,7 +100,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())); } } diff --git a/src/mbgl/annotation/render_annotation_source.cpp b/src/mbgl/annotation/render_annotation_source.cpp index 622f83862..e84a89c41 100644 --- a/src/mbgl/annotation/render_annotation_source.cpp +++ b/src/mbgl/annotation/render_annotation_source.cpp @@ -11,8 +11,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 d195ea9cb..f8065b922 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 b24a66a27..20d2b85d3 100644 --- a/src/mbgl/gfx/renderer_backend.cpp +++ b/src/mbgl/gfx/renderer_backend.cpp @@ -6,7 +6,13 @@ namespace mbgl { namespace gfx { RendererBackend::RendererBackend(const ContextMode contextMode_) - : contextMode(contextMode_) {} + : contextMode(contextMode_), + threadPool(Scheduler::GetBackground(), uniqueID) {} + +RendererBackend::RendererBackend(const ContextMode contextMode_, const TaggedScheduler& threadPool_) + : contextMode(contextMode_), + threadPool(threadPool_) {} + RendererBackend::~RendererBackend() = default; gfx::Context& RendererBackend::getContext() { diff --git a/src/mbgl/gl/context.cpp b/src/mbgl/gl/context.cpp index b53903101..2e0797dc1 100644 --- a/src/mbgl/gl/context.cpp +++ b/src/mbgl/gl/context.cpp @@ -102,7 +102,7 @@ Context::Context(RendererBackend& backend_) Context::~Context() noexcept { if (cleanupOnDestruction) { - Scheduler::GetBackground()->runRenderJobs(); + backend.getThreadPool().runRenderJobs(true /* closeQueue */); reset(); #if !defined(NDEBUG) @@ -115,7 +115,7 @@ Context::~Context() noexcept { void Context::beginFrame() { MLN_TRACE_FUNC(); - 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 f3aefb132..fa8badc2d 100644 --- a/src/mbgl/gl/renderer_backend.cpp +++ b/src/mbgl/gl/renderer_backend.cpp @@ -19,10 +19,14 @@ namespace gl { RendererBackend::RendererBackend(const gfx::ContextMode contextMode_) : gfx::RendererBackend(contextMode_) {} +RendererBackend::RendererBackend(const gfx::ContextMode contextMode_, const TaggedScheduler& threadPool_) + : gfx::RendererBackend(contextMode_, threadPool_) {} + std::unique_ptr RendererBackend::createContext() { MLN_TRACE_FUNC(); - 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/map/map_impl.cpp b/src/mbgl/map/map_impl.cpp index 8fa8f6743..2fdc3a9b8 100644 --- a/src/mbgl/map/map_impl.cpp +++ b/src/mbgl/map/map_impl.cpp @@ -52,7 +52,7 @@ Map::Impl::Impl(RendererFrontend& frontend_, pixelRatio(mapOptions.pixelRatio()), crossSourceCollisions(mapOptions.crossSourceCollisions()), fileSource(std::move(fileSource_)), - style(std::make_unique(fileSource, pixelRatio)), + style(std::make_unique(fileSource, pixelRatio, frontend_.getThreadPool())), annotationManager(*style) { transform.setNorthOrientation(mapOptions.northOrientation()); style->impl->setObserver(this); diff --git a/src/mbgl/mtl/context.cpp b/src/mbgl/mtl/context.cpp index 3e8d5389d..e4c8ac617 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/renderer/render_orchestrator.cpp b/src/mbgl/renderer/render_orchestrator.cpp index 431e7301d..ab2bf4d2e 100644 --- a/src/mbgl/renderer/render_orchestrator.cpp +++ b/src/mbgl/renderer/render_orchestrator.cpp @@ -114,7 +114,9 @@ public: } // 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()), @@ -125,7 +127,7 @@ RenderOrchestrator::RenderOrchestrator(bool backgroundLayerAsColor_, const std:: layerImpls(makeMutable>>()), renderLight(makeMutable()), backgroundLayerAsColor(backgroundLayerAsColor_), - threadPool(Scheduler::GetBackground()) { + threadPool(threadPool_) { glyphManager->setObserver(this); imageManager->setObserver(this); } @@ -147,14 +149,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{10000}; - [[maybe_unused]] const auto remaining = threadPool->waitForEmpty(deferredCleanupTimeout); -// this assert is causing Android Instrumentation tests to fail -// since they need to run in on a debug build -// ignore it for now, see issue https://github.com/maplibre/maplibre-native/issues/2187 -#ifndef __ANDROID__ - assert(remaining == 0); -#endif + threadPool.waitForEmpty(); } void RenderOrchestrator::setObserver(RendererObserver* observer_) { @@ -201,7 +196,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 69ff4667e..3ec1fe6dd 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,9 @@ 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 +213,7 @@ private: 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 2eb5ce900..8c0481e38 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 4b46e0775..d113f2274 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 7673b5268..ba8ab07f0 100644 --- a/src/mbgl/renderer/renderer_impl.cpp +++ b/src/mbgl/renderer/renderer_impl.cpp @@ -59,7 +59,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 7f054bfa2..13377ef42 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 4c77c6a19..9ed8dc5f6 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 96933e08a..988931840 100644 --- a/src/mbgl/renderer/sources/render_geojson_source.cpp +++ b/src/mbgl/renderer/sources/render_geojson_source.cpp @@ -65,8 +65,8 @@ constexpr const auto extensionGetters = mapbox::eternal::hash_map 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 f0c41c2ab..d7c9c4b8c 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 6f4142614..8cb322ea2 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 1599b76ef..8531c6be8 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 db99831e0..30bf34151 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 c827c8a7e..d650e974e 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 ae44ff3f7..efc9787e2 100644 --- a/src/mbgl/renderer/sources/render_tile_source.cpp +++ b/src/mbgl/renderer/sources/render_tile_source.cpp @@ -383,9 +383,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); } @@ -504,8 +504,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 160ed9541..a755bdd41 100644 --- a/src/mbgl/renderer/sources/render_tile_source.hpp +++ b/src/mbgl/renderer/sources/render_tile_source.hpp @@ -51,7 +51,7 @@ public: 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 @@ private: */ 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 212338db6..c53447eb8 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 326b392b6..fff332c45 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 b7ada67d6..75c1c152f 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 @@ public: 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 55466aa3a..82753ccc4 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 bfae55f09..5068093ff 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/sprite/sprite_loader.cpp b/src/mbgl/sprite/sprite_loader.cpp index ec2483f50..7feb71a42 100644 --- a/src/mbgl/sprite/sprite_loader.cpp +++ b/src/mbgl/sprite/sprite_loader.cpp @@ -26,10 +26,10 @@ struct SpriteLoader::Data { std::unique_ptr spriteRequest; }; -SpriteLoader::SpriteLoader(float pixelRatio_) +SpriteLoader::SpriteLoader(float pixelRatio_, const TaggedScheduler& threadPool_) : pixelRatio(pixelRatio_), observer(&nullObserver), - threadPool(Scheduler::GetBackground()) {} + threadPool(threadPool_) {} SpriteLoader::~SpriteLoader() = default; @@ -93,25 +93,25 @@ void SpriteLoader::emitSpriteLoadedIfComplete(style::Sprite sprite) { std::exception_ptr error; }; - auto parseClosure = [sprite = sprite, image = data->image, json = data->json]() -> ParseResult { - try { - return {parseSprite(sprite.id, *image, *json), nullptr}; - } catch (...) { - return {{}, std::current_exception()}; - } - }; + threadPool.scheduleAndReplyValue( + /* parseClosure */ + [sprite = sprite, image = data->image, json = data->json]() -> ParseResult { + try { + return {parseSprite(sprite.id, *image, *json), nullptr}; + } catch (...) { + return {{}, std::current_exception()}; + } + }, + /* resultClosure */ + [this, sprite = sprite, weak = weakFactory.makeWeakPtr()](ParseResult result) { + if (!weak) return; // This instance has been deleted. - auto resultClosure = [this, sprite = sprite, weak = weakFactory.makeWeakPtr()](ParseResult result) { - if (!weak) return; // This instance has been deleted. - - if (result.error) { - observer->onSpriteError(std::optional(sprite), result.error); - return; - } - observer->onSpriteLoaded(std::optional(sprite), std::move(result.images)); - }; - - threadPool->scheduleAndReplyValue(parseClosure, resultClosure); + if (result.error) { + observer->onSpriteError(std::optional(sprite), result.error); + return; + } + observer->onSpriteLoaded(std::optional(sprite), std::move(result.images)); + }); } void SpriteLoader::setObserver(SpriteLoaderObserver* observer_) { diff --git a/src/mbgl/sprite/sprite_loader.hpp b/src/mbgl/sprite/sprite_loader.hpp index e34a71a7c..539f6a1e0 100644 --- a/src/mbgl/sprite/sprite_loader.hpp +++ b/src/mbgl/sprite/sprite_loader.hpp @@ -1,5 +1,6 @@ #pragma once +#include #include #include #include @@ -15,11 +16,10 @@ namespace mbgl { class FileSource; class SpriteLoaderObserver; -class Scheduler; class SpriteLoader { public: - SpriteLoader(float pixelRatio); + SpriteLoader(float pixelRatio, const TaggedScheduler& threadPool_); ~SpriteLoader(); void load(const std::optional sprite, FileSource&); @@ -39,7 +39,7 @@ private: std::mutex dataMapMutex; SpriteLoaderObserver* observer = nullptr; - std::shared_ptr threadPool; + TaggedScheduler threadPool; mapbox::base::WeakPtrFactory weakFactory{this}; // Do not add members here, see `WeakPtrFactory` }; diff --git a/src/mbgl/style/sources/geojson_source.cpp b/src/mbgl/style/sources/geojson_source.cpp index a00477d3a..ebdda4222 100644 --- a/src/mbgl/style/sources/geojson_source.cpp +++ b/src/mbgl/style/sources/geojson_source.cpp @@ -9,6 +9,7 @@ #include #include #include +#include namespace mbgl { namespace style { @@ -21,7 +22,6 @@ Immutable GeoJSONOptions::defaultOptions() { GeoJSONSource::GeoJSONSource(std::string id, Immutable options) : Source(makeMutable(std::move(id), std::move(options))), - threadPool(Scheduler::GetBackground()), sequencedScheduler(Scheduler::GetSequenced()) {} GeoJSONSource::~GeoJSONSource() = default; @@ -77,32 +77,37 @@ void GeoJSONSource::loadDescription(FileSource& fileSource) { } else if (res.noContent) { observer->onSourceError(*this, std::make_exception_ptr(std::runtime_error("unexpectedly empty GeoJSON"))); } else { - auto makeImplInBackground = [currentImpl = baseImpl, - data = res.data, - seqScheduler{sequencedScheduler}]() -> Immutable { - assert(data); - auto& current = static_cast(*currentImpl); - conversion::Error error; - std::shared_ptr geoJSONData; - if (std::optional geoJSON = conversion::convertJSON(*data, error)) { - geoJSONData = GeoJSONData::create(*geoJSON, std::move(seqScheduler), current.getOptions()); - } else { - // Create an empty GeoJSON VT object to make sure we're not - // infinitely waiting for tiles to load. - Log::Error(Event::ParseStyle, "Failed to parse GeoJSON data: " + error.message); - } - return makeMutable(current, std::move(geoJSONData)); - }; - auto onImplReady = [this, self = makeWeakPtr(), capturedReq = req.get()](Immutable newImpl) { - assert(capturedReq); - if (!self) return; // This source has been deleted. - if (capturedReq != req.get()) return; // A new request is being processed, ignore this impl. + // Note: This task appears to be safe enough to schedule on the generic background queue. + // This task does not reference other objects who's lifetimes are coupled with a map. + Scheduler::GetBackground()->scheduleAndReplyValue( + util::SimpleIdentity::Empty, + /* makeImplInBackground */ + [currentImpl = baseImpl, + data = res.data, + seqScheduler{sequencedScheduler}]() -> Immutable { + assert(data); + auto& current = static_cast(*currentImpl); + conversion::Error error; + std::shared_ptr geoJSONData; + if (std::optional geoJSON = conversion::convertJSON(*data, error)) { + geoJSONData = GeoJSONData::create(*geoJSON, std::move(seqScheduler), current.getOptions()); + } else { + // Create an empty GeoJSON VT object to make sure we're not + // infinitely waiting for tiles to load. + Log::Error(Event::ParseStyle, "Failed to parse GeoJSON data: " + error.message); + } + return makeMutable(current, std::move(geoJSONData)); + }, + /* onImplReady */ + [this, self = makeWeakPtr(), capturedReq = req.get()](Immutable newImpl) { + assert(capturedReq); + if (!self) return; // This source has been deleted. + if (capturedReq != req.get()) return; // A new request is being processed, ignore this impl. - baseImpl = std::move(newImpl); - loaded = true; - observer->onSourceLoaded(*this); - }; - threadPool->scheduleAndReplyValue(makeImplInBackground, onImplReady); + baseImpl = std::move(newImpl); + loaded = true; + observer->onSourceLoaded(*this); + }); } }); } diff --git a/src/mbgl/style/sources/geojson_source_impl.cpp b/src/mbgl/style/sources/geojson_source_impl.cpp index 84872cbfe..413e0bd6e 100644 --- a/src/mbgl/style/sources/geojson_source_impl.cpp +++ b/src/mbgl/style/sources/geojson_source_impl.cpp @@ -4,6 +4,7 @@ #include #include #include +#include #ifdef _MSC_VER #pragma warning(push) @@ -25,7 +26,8 @@ namespace style { class GeoJSONVTData final : public GeoJSONData { void getTile(const CanonicalTileID& id, const std::function& fn) final { assert(fn); - scheduler->scheduleAndReplyValue( + sequencedScheduler->scheduleAndReplyValue( + util::SimpleIdentity::Empty, [id, geoJSONVT_impl = this->impl]() -> TileFeatures { return geoJSONVT_impl->getTile(id.z, id.x, id.y).features; }, @@ -41,14 +43,14 @@ class GeoJSONVTData final : public GeoJSONData { friend GeoJSONData; GeoJSONVTData(const GeoJSON& geoJSON, const mapbox::geojsonvt::Options& options, - std::shared_ptr scheduler_) + std::shared_ptr sequencedScheduler_) : impl(std::make_shared(geoJSON, options)), - scheduler(std::move(scheduler_)) { - assert(scheduler); + sequencedScheduler(std::move(sequencedScheduler_)) { + assert(sequencedScheduler); } std::shared_ptr impl; // Accessed on worker thread. - std::shared_ptr scheduler; + std::shared_ptr sequencedScheduler; }; class SuperclusterData final : public GeoJSONData { @@ -89,7 +91,7 @@ T evaluateFeature(const mapbox::feature::feature& f, // static std::shared_ptr GeoJSONData::create(const GeoJSON& geoJSON, - std::shared_ptr scheduler, + std::shared_ptr sequencedScheduler, const Immutable& options) { constexpr double scale = util::EXTENT / util::tileSize_D; if (options->cluster && geoJSON.is() && !geoJSON.get().empty()) { @@ -126,7 +128,7 @@ std::shared_ptr GeoJSONData::create(const GeoJSON& geoJSON, vtOptions.buffer = static_cast(::round(scale * options->buffer)); vtOptions.tolerance = scale * options->tolerance; vtOptions.lineMetrics = options->lineMetrics; - return std::shared_ptr(new GeoJSONVTData(geoJSON, vtOptions, std::move(scheduler))); + return std::shared_ptr(new GeoJSONVTData(geoJSON, vtOptions, std::move(sequencedScheduler))); } GeoJSONSource::Impl::Impl(std::string id_, Immutable options_) diff --git a/src/mbgl/style/style.cpp b/src/mbgl/style/style.cpp index afbc95812..4f8723ead 100644 --- a/src/mbgl/style/style.cpp +++ b/src/mbgl/style/style.cpp @@ -10,8 +10,8 @@ namespace mbgl { namespace style { -Style::Style(std::shared_ptr fileSource, float pixelRatio) - : impl(std::make_unique(std::move(fileSource), pixelRatio)) {} +Style::Style(std::shared_ptr fileSource, float pixelRatio, const TaggedScheduler& threadPool_) + : impl(std::make_unique(std::move(fileSource), pixelRatio, threadPool_)) {} Style::~Style() = default; diff --git a/src/mbgl/style/style_impl.cpp b/src/mbgl/style/style_impl.cpp index 4b2119ba0..bc32c0e22 100644 --- a/src/mbgl/style/style_impl.cpp +++ b/src/mbgl/style/style_impl.cpp @@ -30,9 +30,9 @@ namespace style { static Observer nullObserver; -Style::Impl::Impl(std::shared_ptr fileSource_, float pixelRatio) +Style::Impl::Impl(std::shared_ptr fileSource_, float pixelRatio, const TaggedScheduler& threadPool_) : fileSource(std::move(fileSource_)), - spriteLoader(std::make_unique(pixelRatio)), + spriteLoader(std::make_unique(pixelRatio, threadPool_)), light(std::make_unique()), observer(&nullObserver) { spriteLoader->setObserver(this); diff --git a/src/mbgl/style/style_impl.hpp b/src/mbgl/style/style_impl.hpp index 63d6b54c1..04591b16b 100644 --- a/src/mbgl/style/style_impl.hpp +++ b/src/mbgl/style/style_impl.hpp @@ -1,5 +1,6 @@ #pragma once +#include #include #include #include @@ -36,7 +37,7 @@ class Style::Impl : public SpriteLoaderObserver, public LightObserver, public util::noncopyable { public: - Impl(std::shared_ptr, float pixelRatio); + Impl(std::shared_ptr, float pixelRatio, const TaggedScheduler& threadPool_); ~Impl() override; void loadJSON(const std::string&); diff --git a/src/mbgl/tile/geometry_tile.cpp b/src/mbgl/tile/geometry_tile.cpp index 086cd862f..a052168c7 100644 --- a/src/mbgl/tile/geometry_tile.cpp +++ b/src/mbgl/tile/geometry_tile.cpp @@ -169,10 +169,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, @@ -194,7 +195,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 1948983d6..aa9aa8319 100644 --- a/src/mbgl/tile/geometry_tile.hpp +++ b/src/mbgl/tile/geometry_tile.hpp @@ -106,7 +106,7 @@ protected: private: void markObsolete(); - 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 732498426..24a81c4ca 100644 --- a/src/mbgl/tile/geometry_tile_worker.cpp +++ b/src/mbgl/tile/geometry_tile_worker.cpp @@ -31,6 +31,7 @@ using namespace style; GeometryTileWorker::GeometryTileWorker(ActorRef self_, ActorRef parent_, + const TaggedScheduler& scheduler_, OverscaledTileID id_, std::string sourceID_, const std::atomic& obsolete_, @@ -39,6 +40,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_), @@ -49,7 +51,7 @@ GeometryTileWorker::GeometryTileWorker(ActorRef self_, GeometryTileWorker::~GeometryTileWorker() { MLN_TRACE_FUNC(); - 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 bb169a0e8..ccd546520 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 @@ private: ActorRef self; ActorRef parent; + TaggedScheduler scheduler; const OverscaledTileID id; const std::string sourceID; diff --git a/src/mbgl/tile/raster_dem_tile.cpp b/src/mbgl/tile/raster_dem_tile.cpp index 5628c8aea..a624917f7 100644 --- a/src/mbgl/tile/raster_dem_tile.cpp +++ b/src/mbgl/tile/raster_dem_tile.cpp @@ -17,9 +17,9 @@ namespace mbgl { RasterDEMTile::RasterDEMTile(const OverscaledTileID& id_, const TileParameters& parameters, const Tileset& tileset) : Tile(Kind::RasterDEM, id_), loader(*this, id_, parameters, tileset), - threadPool(Scheduler::GetBackground()), + threadPool(parameters.threadPool), mailbox(std::make_shared(*Scheduler::GetCurrent())), - worker(Scheduler::GetBackground(), ActorRef(*this, mailbox)) { + worker(parameters.threadPool, ActorRef(*this, mailbox)) { encoding = tileset.encoding; if (id.canonical.y == 0) { // this tile doesn't have upper neighboring tiles so marked those as backfilled @@ -37,7 +37,7 @@ RasterDEMTile::~RasterDEMTile() { // The bucket has resources that need to be released on the render thread. if (bucket) { - threadPool->runOnRenderThread([bucket_{std::move(bucket)}]() {}); + threadPool.runOnRenderThread([bucket_{std::move(bucket)}]() {}); } } diff --git a/src/mbgl/tile/raster_dem_tile.hpp b/src/mbgl/tile/raster_dem_tile.hpp index 644360d11..ad0e5f191 100644 --- a/src/mbgl/tile/raster_dem_tile.hpp +++ b/src/mbgl/tile/raster_dem_tile.hpp @@ -93,7 +93,7 @@ private: TileLoader loader; - const std::shared_ptr threadPool; + TaggedScheduler threadPool; std::shared_ptr mailbox; Actor worker; diff --git a/src/mbgl/tile/raster_tile.cpp b/src/mbgl/tile/raster_tile.cpp index 702a06d59..b9f97e698 100644 --- a/src/mbgl/tile/raster_tile.cpp +++ b/src/mbgl/tile/raster_tile.cpp @@ -17,16 +17,16 @@ namespace mbgl { RasterTile::RasterTile(const OverscaledTileID& id_, const TileParameters& parameters, const Tileset& tileset) : Tile(Kind::Raster, id_), loader(*this, id_, parameters, tileset), - threadPool(Scheduler::GetBackground()), + threadPool(parameters.threadPool), mailbox(std::make_shared(*Scheduler::GetCurrent())), - worker(Scheduler::GetBackground(), ActorRef(*this, mailbox)) {} + worker(parameters.threadPool, ActorRef(*this, mailbox)) {} RasterTile::~RasterTile() { markObsolete(); // The bucket has resources that need to be released on the render thread. if (bucket) { - threadPool->runOnRenderThread([bucket_{std::move(bucket)}]() {}); + threadPool.runOnRenderThread([bucket_{std::move(bucket)}]() {}); } } diff --git a/src/mbgl/tile/raster_tile.hpp b/src/mbgl/tile/raster_tile.hpp index 18c3dd878..544540a12 100644 --- a/src/mbgl/tile/raster_tile.hpp +++ b/src/mbgl/tile/raster_tile.hpp @@ -42,7 +42,7 @@ private: TileLoader loader; - const std::shared_ptr threadPool; + TaggedScheduler threadPool; std::shared_ptr mailbox; Actor worker; diff --git a/src/mbgl/tile/tile_cache.cpp b/src/mbgl/tile/tile_cache.cpp index 2d7c98206..8dc0bac35 100644 --- a/src/mbgl/tile/tile_cache.cpp +++ b/src/mbgl/tile/tile_cache.cpp @@ -51,10 +51,16 @@ void TileCache::deferredRelease(std::unique_ptr&& tile) { // If this temporary outlives the `schedule` call, and the function is executed immediately // by a waiting thread and is already complete, that temporary reference ends up being the // last one and the destruction actually occurs here on this thread. - std::function func{[tile_{CaptureWrapper{std::move(tile)}}]() { + std::function func{[tile_{CaptureWrapper{std::move(tile)}}, this]() mutable { + tile_.item = {}; + deferredDeletionsPending--; + deferredSignal.notify_all(); }}; - threadPool->schedule(std::move(func)); + std::unique_lock counterLock(deferredSignalLock); + deferredDeletionsPending++; + + 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 fe7f41f7b..72d430db3 100644 --- a/src/mbgl/tile/tile_cache.hpp +++ b/src/mbgl/tile/tile_cache.hpp @@ -1,22 +1,33 @@ #pragma once +#include #include #include #include #include #include +#include +#include +#include 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_) {} + ~TileCache() { + clear(); + + std::unique_lock counterLock(deferredSignalLock); + while (deferredDeletionsPending != 0) { + deferredSignal.wait(counterLock); + } + } + /// Change the maximum size of the cache. void setSize(size_t); @@ -38,7 +49,10 @@ public: private: std::map> tiles; std::list orderedKeys; - std::shared_ptr threadPool; + TaggedScheduler threadPool; + std::atomic deferredDeletionsPending{0}; + std::mutex deferredSignalLock; + std::condition_variable deferredSignal; size_t size; }; diff --git a/src/mbgl/util/thread_pool.cpp b/src/mbgl/util/thread_pool.cpp index d02963bf8..af3f2e6a3 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; } @@ -37,48 +34,64 @@ std::thread ThreadedSchedulerBase::makeSchedulerThread(size_t index) { owningThreadPool.set(this); while (true) { - std::unique_lock lock(mutex); - if (queue.empty() && !pendingItems) { - cvEmpty.notify_all(); + std::unique_lock conditionLock(workerMutex); + if (!terminated && taskCount == 0) { + 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++; + 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()) { + q->runningCount++; + tasklet = std::move(q->queue.front()); + q->queue.pop(); + } + if (!tasklet) continue; + } + + assert(taskCount > 0); + taskCount--; - if (function) { try { - function(); + tasklet(); + tasklet = {}; // destroy the function and release its captures before unblocking `waitForEmpty` - // 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(); + 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 +105,61 @@ std::thread ThreadedSchedulerBase::makeSchedulerThread(size_t index) { } void ThreadedSchedulerBase::schedule(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)); - } - cvAvailable.notify_one(); - } + schedule(uniqueID, std::move(fn)); } -std::size_t ThreadedSchedulerBase::waitForEmpty(Milliseconds timeout) { +void ThreadedSchedulerBase::schedule(const util::SimpleIdentity tag, std::function&& fn) { + assert(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; + } + } + + { + std::lock_guard lock(q->lock); + q->queue.push(std::move(fn)); + taskCount++; + } + + cvAvailable.notify_one(); +} + +void ThreadedSchedulerBase::waitForEmpty(const util::SimpleIdentity 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); + const auto tagToFind = tag.isEmpty() ? uniqueID : tag; + + std::shared_ptr q; + { + std::lock_guard lock(taggedQueueLock); + auto it = taggedQueue.find(tagToFind); + 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(tagToFind); } - 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 a287e9821..be4cf95d4 100644 --- a/src/mbgl/util/thread_pool.hpp +++ b/src/mbgl/util/thread_pool.hpp @@ -3,9 +3,12 @@ #include #include #include +#include +#include #include #include +#include #include #include #include @@ -15,7 +18,16 @@ 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 Identifier object to indicate ownership of `fn` + /// @param fn Task to run + void schedule(const util::SimpleIdentity tag, std::function&& fn) override; + const util::SimpleIdentity uniqueID; protected: ThreadedSchedulerBase() = default; @@ -24,28 +36,31 @@ protected: 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 Tag of the owner to identify the collection of tasks to + // wait for. Not providing a tag waits on tasks owned by the scheduler. + void waitForEmpty(const util::SimpleIdentity = util::SimpleIdentity::Empty) 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; + std::atomic taskCount{0}; 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,20 +89,55 @@ public: } } - void runOnRenderThread(std::function&& fn) override { - std::lock_guard lock(renderMutex); - renderThreadQueue.push(std::move(fn)); + void runOnRenderThread(const util::SimpleIdentity 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 util::SimpleIdentity 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(); } @@ -95,8 +145,12 @@ public: private: std::vector threads; - 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}; // Do not add members here, see `WeakPtrFactory` diff --git a/test/actor/actor.test.cpp b/test/actor/actor.test.cpp index abc5655f6..fe39803af 100644 --- a/test/actor/actor.test.cpp +++ b/test/actor/actor.test.cpp @@ -90,10 +90,7 @@ TEST(Actor, DestructionBlocksOnSend) { ~TestScheduler() override { EXPECT_TRUE(waited.load()); } - std::size_t waitForEmpty(Milliseconds) override { - assert(false); - return 0; - } + void waitForEmpty(const util::SimpleIdentity) override { assert(false); } void schedule(std::function&&) final { promise.set_value(); @@ -101,6 +98,11 @@ TEST(Actor, DestructionBlocksOnSend) { std::this_thread::sleep_for(1ms); waited = true; } + + void schedule(const util::SimpleIdentity, std::function&& fn) override final { + schedule(std::move(fn)); + } + mapbox::base::WeakPtr makeWeakPtr() override { return weakFactory.makeWeakPtr(); } mapbox::base::WeakPtrFactory weakFactory{this}; diff --git a/test/api/custom_drawable_layer.test.cpp b/test/api/custom_drawable_layer.test.cpp index 8b69b3bd4..18575d22e 100644 --- a/test/api/custom_drawable_layer.test.cpp +++ b/test/api/custom_drawable_layer.test.cpp @@ -108,8 +108,9 @@ public: constexpr auto numPoints = 100; 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); + polyline.emplace_back( + ipoint * util::EXTENT / numPoints, + 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 63b45aba6..f17a626db 100644 --- a/test/include/mbgl/test/vector_tile_test.hpp +++ b/test/include/mbgl/test/vector_tile_test.hpp @@ -16,10 +16,10 @@ namespace mbgl { class VectorTileTest { public: + util::SimpleIdentity uniqueID; std::shared_ptr fileSource = std::make_shared(); TransformState transformState; util::RunLoop loop; - style::Style style{fileSource, 1}; AnnotationManager annotationManager{style}; const std::shared_ptr imageManager = std::make_shared(); @@ -27,22 +27,30 @@ public: Tileset tileset{{"https://example.com"}, {0, 22}, "none"}; - const std::shared_ptr threadPool = Scheduler::GetBackground(); + TaggedScheduler threadPool; - TileParameters tileParameters{1.0, - MapDebugOptions(), - transformState, - fileSource, - MapMode::Continuous, - annotationManager.makeWeakPtr(), - imageManager, - glyphManager, - 0}; + TileParameters tileParameters; + style::Style style; + + VectorTileTest() + : threadPool(Scheduler::GetBackground(), uniqueID), + tileParameters{1.0, + MapDebugOptions(), + transformState, + fileSource, + MapMode::Continuous, + annotationManager.makeWeakPtr(), + imageManager, + glyphManager, + 0, + threadPool}, + style{fileSource, 1, threadPool} {} ~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(); + threadPool.runRenderJobs(true); } }; diff --git a/test/sprite/sprite_loader.test.cpp b/test/sprite/sprite_loader.test.cpp index 75eaa6c98..0da618bb7 100644 --- a/test/sprite/sprite_loader.test.cpp +++ b/test/sprite/sprite_loader.test.cpp @@ -31,12 +31,21 @@ public: class SpriteLoaderTest { public: - SpriteLoaderTest() = default; + SpriteLoaderTest() + : threadPool(Scheduler::GetBackground(), uniqueID), + spriteLoader(1, threadPool) {} + ~SpriteLoaderTest() { + threadPool.waitForEmpty(); + threadPool.runRenderJobs(true); + } + + util::SimpleIdentity uniqueID; util::RunLoop loop; StubFileSource fileSource; StubSpriteLoaderObserver observer; - SpriteLoader spriteLoader{1}; + TaggedScheduler threadPool; + SpriteLoader spriteLoader; void run() { // Squelch logging. diff --git a/test/style/source.test.cpp b/test/style/source.test.cpp index 8be5ed674..0d33eb57a 100644 --- a/test/style/source.test.cpp +++ b/test/style/source.test.cpp @@ -54,17 +54,18 @@ using SourceType = mbgl::style::SourceType; class SourceTest { public: + util::SimpleIdentity uniqueID; util::RunLoop loop; std::shared_ptr fileSource = std::make_shared(); StubStyleObserver styleObserver; StubRenderSourceObserver renderSourceObserver; Transform transform; TransformState transformState; - Style style{fileSource, 1}; 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; + Style style; TileParameters tileParameters(MapMode mapMode = MapMode::Continuous) { return {1.0, @@ -75,10 +76,13 @@ public: annotationManager.makeWeakPtr(), imageManager, glyphManager, - 0}; + 0, + threadPool}; }; - SourceTest() { + SourceTest() + : threadPool(Scheduler::GetBackground(), uniqueID), + style{fileSource, 1, threadPool} { // Squelch logging. Log::setObserver(std::make_unique()); @@ -88,7 +92,7 @@ public: transformState = transform.getState(); } - ~SourceTest() { threadPool->waitForEmpty(); } + ~SourceTest() { threadPool.waitForEmpty(); } void run() { loop.run(); } @@ -783,8 +787,8 @@ public: 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, @@ -887,8 +891,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/style/style.test.cpp b/test/style/style.test.cpp index cd453691a..b08989cd9 100644 --- a/test/style/style.test.cpp +++ b/test/style/style.test.cpp @@ -23,7 +23,7 @@ TEST(Style, Properties) { util::RunLoop loop; auto fileSource = std::make_shared(); - Style::Impl style{fileSource, 1.0}; + Style::Impl style{fileSource, 1.0, {Scheduler::GetBackground(), {}}}; style.loadJSON(R"STYLE({"name": "Test"})STYLE"); ASSERT_EQ("Test", style.getName()); @@ -65,7 +65,7 @@ TEST(Style, DuplicateSource) { util::RunLoop loop; auto fileSource = std::make_shared(); - Style::Impl style{fileSource, 1.0}; + Style::Impl style{fileSource, 1.0, {Scheduler::GetBackground(), {}}}; style.loadJSON(util::read_file("test/fixtures/resources/style-unused-sources.json")); @@ -85,7 +85,7 @@ TEST(Style, RemoveSourceInUse) { FixtureLog log; auto fileSource = std::make_shared(); - Style::Impl style{fileSource, 1.0}; + Style::Impl style{fileSource, 1.0, {Scheduler::GetBackground(), {}}}; style.loadJSON(util::read_file("test/fixtures/resources/style-unused-sources.json")); style.addSource(std::make_unique("sourceId", "mptiler://tiles/contours")); @@ -113,7 +113,7 @@ TEST(Style, RemoveSourceInUse) { TEST(Style, SourceImplsOrder) { util::RunLoop loop; auto fileSource = std::make_shared(); - Style::Impl style{fileSource, 1.0}; + Style::Impl style{fileSource, 1.0, {Scheduler::GetBackground(), {}}}; style.addSource(std::make_unique("c", "mptiler://tiles/contours")); style.addSource(std::make_unique("b", "mptiler://tiles/contours")); @@ -135,7 +135,7 @@ TEST(Style, SourceImplsOrder) { TEST(Style, AddRemoveImage) { util::RunLoop loop; auto fileSource = std::make_shared(); - Style::Impl style{fileSource, 1.0}; + Style::Impl style{fileSource, 1.0, {Scheduler::GetBackground(), {}}}; style.addImage(std::make_unique("one", PremultipliedImage({16, 16}), 2.0f)); style.addImage(std::make_unique("two", PremultipliedImage({16, 16}), 2.0f)); style.addImage(std::make_unique("three", PremultipliedImage({16, 16}), 2.0f)); @@ -152,7 +152,7 @@ TEST(Style, AddRemoveRemoveImage) { // regression test for https://github.com/mapbox/mapbox-gl-native/pull/16391 util::RunLoop loop; auto fileSource = std::make_shared(); - Style::Impl style{fileSource, 1.0}; + Style::Impl style{fileSource, 1.0, {Scheduler::GetBackground(), {}}}; style.addImage(std::make_unique("one", PremultipliedImage({16, 16}), 2.0f)); style.addImage(std::make_unique("two", PremultipliedImage({16, 16}), 2.0f)); style.addImage(std::make_unique("three", PremultipliedImage({16, 16}), 2.0f)); diff --git a/test/style/style_layer.test.cpp b/test/style/style_layer.test.cpp index 0a4cae49a..0a72411ec 100644 --- a/test/style/style_layer.test.cpp +++ b/test/style/style_layer.test.cpp @@ -292,7 +292,7 @@ TEST(Layer, DuplicateLayer) { // Setup style auto fileSource = std::make_shared(); - Style::Impl style{fileSource, 1.0}; + Style::Impl style{fileSource, 1.0, {Scheduler::GetBackground(), {}}}; style.loadJSON(util::read_file("test/fixtures/resources/style-unused-sources.json")); // Add initial layer @@ -313,7 +313,7 @@ TEST(Layer, IncompatibleLayer) { // Setup style auto fileSource = std::make_shared(); - Style::Impl style{fileSource, 1.0}; + Style::Impl style{fileSource, 1.0, {Scheduler::GetBackground(), {}}}; style.loadJSON(util::read_file("test/fixtures/resources/style-unused-sources.json")); // Try to add duplicate diff --git a/test/tile/custom_geometry_tile.test.cpp b/test/tile/custom_geometry_tile.test.cpp index d80e4cd87..57fae1b8b 100644 --- a/test/tile/custom_geometry_tile.test.cpp +++ b/test/tile/custom_geometry_tile.test.cpp @@ -22,23 +22,28 @@ using namespace mbgl::style; class CustomTileTest { public: + util::SimpleIdentity uniqueID; std::shared_ptr fileSource = std::make_shared(); TransformState transformState; util::RunLoop loop; - style::Style style{fileSource, 1}; AnnotationManager annotationManager{style}; std::shared_ptr imageManager = std::make_shared(); std::shared_ptr glyphManager = std::make_shared(); + TileParameters tileParameters; + style::Style style; - TileParameters tileParameters{1.0, - MapDebugOptions(), - transformState, - fileSource, - MapMode::Continuous, - annotationManager.makeWeakPtr(), - imageManager, - glyphManager, - 0}; + CustomTileTest() + : tileParameters{1.0, + MapDebugOptions(), + transformState, + fileSource, + MapMode::Continuous, + annotationManager.makeWeakPtr(), + imageManager, + glyphManager, + 0, + {Scheduler::GetBackground(), uniqueID}}, + style{fileSource, 1, tileParameters.threadPool} {} }; TEST(CustomGeometryTile, InvokeFetchTile) { diff --git a/test/tile/geojson_tile.test.cpp b/test/tile/geojson_tile.test.cpp index 2db82f01e..4889192fe 100644 --- a/test/tile/geojson_tile.test.cpp +++ b/test/tile/geojson_tile.test.cpp @@ -22,24 +22,29 @@ using namespace mbgl::style; class GeoJSONTileTest { public: + util::SimpleIdentity uniqueID; std::shared_ptr fileSource = std::make_shared(); TransformState transformState; util::RunLoop loop; - style::Style style{fileSource, 1}; AnnotationManager annotationManager{style}; std::shared_ptr imageManager = std::make_shared(); std::shared_ptr glyphManager = std::make_shared(); Tileset tileset{{"https://example.com"}, {0, 22}, "none"}; + TileParameters tileParameters; + style::Style style; - TileParameters tileParameters{1.0, - MapDebugOptions(), - transformState, - fileSource, - MapMode::Continuous, - annotationManager.makeWeakPtr(), - imageManager, - glyphManager, - 0}; + GeoJSONTileTest() + : tileParameters{1.0, + MapDebugOptions(), + transformState, + fileSource, + MapMode::Continuous, + annotationManager.makeWeakPtr(), + imageManager, + glyphManager, + 0, + {Scheduler::GetBackground(), uniqueID}}, + style{fileSource, 1, tileParameters.threadPool} {} }; namespace { diff --git a/test/tile/raster_dem_tile.test.cpp b/test/tile/raster_dem_tile.test.cpp index d727d3c77..98f4a9b7b 100644 --- a/test/tile/raster_dem_tile.test.cpp +++ b/test/tile/raster_dem_tile.test.cpp @@ -16,24 +16,29 @@ using namespace mbgl; class RasterDEMTileTest { public: + util::SimpleIdentity uniqueID; std::shared_ptr fileSource = std::make_shared(); TransformState transformState; util::RunLoop loop; - style::Style style{fileSource, 1}; AnnotationManager annotationManager{style}; std::shared_ptr imageManager = std::make_shared(); std::shared_ptr glyphManager = std::make_shared(); Tileset tileset{{"https://example.com"}, {0, 22}, "none"}; + TileParameters tileParameters; + style::Style style; - TileParameters tileParameters{1.0, - MapDebugOptions(), - transformState, - fileSource, - MapMode::Continuous, - annotationManager.makeWeakPtr(), - imageManager, - glyphManager, - 0}; + RasterDEMTileTest() + : tileParameters{1.0, + MapDebugOptions(), + transformState, + fileSource, + MapMode::Continuous, + annotationManager.makeWeakPtr(), + imageManager, + glyphManager, + 0, + {Scheduler::GetBackground(), uniqueID}}, + style{fileSource, 1, tileParameters.threadPool} {} }; TEST(RasterDEMTile, setError) { diff --git a/test/tile/raster_tile.test.cpp b/test/tile/raster_tile.test.cpp index 213801d20..26a239608 100644 --- a/test/tile/raster_tile.test.cpp +++ b/test/tile/raster_tile.test.cpp @@ -16,24 +16,29 @@ using namespace mbgl; class RasterTileTest { public: + util::SimpleIdentity uniqueID; std::shared_ptr fileSource = std::make_shared(); TransformState transformState; util::RunLoop loop; - style::Style style{fileSource, 1}; AnnotationManager annotationManager{style}; std::shared_ptr imageManager = std::make_shared(); std::shared_ptr glyphManager = std::make_shared(); Tileset tileset{{"https://example.com"}, {0, 22}, "none"}; + TileParameters tileParameters; + style::Style style; - TileParameters tileParameters{1.0, - MapDebugOptions(), - transformState, - fileSource, - MapMode::Continuous, - annotationManager.makeWeakPtr(), - imageManager, - glyphManager, - 0}; + RasterTileTest() + : tileParameters{1.0, + MapDebugOptions(), + transformState, + fileSource, + MapMode::Continuous, + annotationManager.makeWeakPtr(), + imageManager, + glyphManager, + 0, + {Scheduler::GetBackground(), uniqueID}}, + style{fileSource, 1, tileParameters.threadPool} {} }; TEST(RasterTile, setError) { diff --git a/test/tile/tile_cache.test.cpp b/test/tile/tile_cache.test.cpp index 51c0c92ef..076186a0b 100644 --- a/test/tile/tile_cache.test.cpp +++ b/test/tile/tile_cache.test.cpp @@ -45,48 +45,52 @@ public: TEST(TileCache, Smoke) { VectorTileTest test; - TileCache cache(Scheduler::GetBackground(), 1); - const OverscaledTileID id(0, 0, 0); - auto tile = std::make_unique(id, "source", test.tileParameters, test.tileset); + { + TileCache cache(test.threadPool, 1); + const OverscaledTileID id(0, 0, 0); + auto tile = std::make_unique(id, "source", test.tileParameters, test.tileset); - cache.add(id, std::move(tile)); - EXPECT_TRUE(cache.has(id)); - cache.clear(); - EXPECT_FALSE(cache.has(id)); + cache.add(id, std::move(tile)); + EXPECT_TRUE(cache.has(id)); + cache.clear(); + EXPECT_FALSE(cache.has(id)); + } } TEST(TileCache, Issue15926) { VectorTileTest test; - TileCache cache(test.threadPool, 2); - const OverscaledTileID id0(0, 0, 0); - const OverscaledTileID id1(1, 0, 0); - auto tile1 = std::make_unique(id0, "source", test.tileParameters, test.tileset); - auto tile2 = std::make_unique(id0, "source", test.tileParameters, test.tileset); - auto tile3 = std::make_unique(id1, "source", test.tileParameters, test.tileset); - auto tile4 = std::make_unique(id0, "source", test.tileParameters, test.tileset); - const auto tile1Id = tile1->uniqueId; + { + TileCache cache(test.threadPool, 2); + const OverscaledTileID id0(0, 0, 0); + const OverscaledTileID id1(1, 0, 0); + auto tile1 = std::make_unique(id0, "source", test.tileParameters, test.tileset); + auto tile2 = std::make_unique(id0, "source", test.tileParameters, test.tileset); + auto tile3 = std::make_unique(id1, "source", test.tileParameters, test.tileset); + auto tile4 = std::make_unique(id0, "source", test.tileParameters, test.tileset); + const auto tile1Id = tile1->uniqueId; - // add - cache.add(id0, std::move(tile1)); - EXPECT_TRUE(cache.has(id0)); + // add + cache.add(id0, std::move(tile1)); + EXPECT_TRUE(cache.has(id0)); - // adding a key already present doesn't replace the existing item - cache.add(id0, std::move(tile2)); - EXPECT_EQ(tile1Id, static_cast(cache.get(id0))->uniqueId); + // adding a key already present doesn't replace the existing item + cache.add(id0, std::move(tile2)); + EXPECT_EQ(tile1Id, static_cast(cache.get(id0))->uniqueId); - // Evict on add - cache.setSize(1); - cache.add(id1, std::move(tile3)); - EXPECT_FALSE(cache.has(id0)); - EXPECT_TRUE(cache.has(id1)); + // Evict on add + cache.setSize(1); + cache.add(id1, std::move(tile3)); + EXPECT_FALSE(cache.has(id0)); + EXPECT_TRUE(cache.has(id1)); - // Evict due to size limit change - cache.setSize(2); - cache.add(id0, std::move(tile4)); - EXPECT_TRUE(cache.has(id0)); - EXPECT_TRUE(cache.has(id1)); - cache.setSize(1); - // older item should be evicted - EXPECT_TRUE(cache.has(id0)); - EXPECT_FALSE(cache.has(id1)); + // Evict due to size limit change + cache.setSize(2); + cache.add(id0, std::move(tile4)); + EXPECT_TRUE(cache.has(id0)); + EXPECT_TRUE(cache.has(id1)); + cache.setSize(1); + // older item should be evicted + EXPECT_TRUE(cache.has(id0)); + EXPECT_FALSE(cache.has(id1)); + } } diff --git a/test/util/async_task.test.cpp b/test/util/async_task.test.cpp index e264a4ceb..fd58cfd5a 100644 --- a/test/util/async_task.test.cpp +++ b/test/util/async_task.test.cpp @@ -100,12 +100,13 @@ TEST(AsyncTask, DestroyAfterSignaling) { TEST(AsyncTask, RequestCoalescingMultithreaded) { RunLoop loop; + util::SimpleIdentity id; 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(), id}; + auto mailbox = std::make_shared(retainer); TestWorker worker(&async); ActorRef workerRef(worker, mailbox); @@ -127,14 +128,15 @@ TEST(AsyncTask, RequestCoalescingMultithreaded) { TEST(AsyncTask, ThreadSafety) { RunLoop loop; + mbgl::util::SimpleIdentity id; unsigned count = 0, numThreads = 25; std::atomic_uint completed(numThreads); AsyncTask async([&count] { ++count; }); - std::shared_ptr retainer = Scheduler::GetBackground(); - auto mailbox = std::make_shared(*retainer); + TaggedScheduler retainer = {Scheduler::GetBackground(), id}; + auto mailbox = std::make_shared(retainer); TestWorker worker(&async); ActorRef workerRef(worker, mailbox); @@ -168,7 +170,7 @@ TEST(AsyncTask, scheduleAndReplyValue) { }; std::shared_ptr sheduler = Scheduler::GetBackground(); - sheduler->scheduleAndReplyValue(runInBackground, onResult); + sheduler->scheduleAndReplyValue(util::SimpleIdentity::Empty, runInBackground, onResult); loop.run(); } diff --git a/test/util/thread.test.cpp b/test/util/thread.test.cpp index 01bfce230..795ca8d92 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,28 +385,14 @@ 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) { + const auto id = util::SimpleIdentity{}; auto pool = Scheduler::GetBackground(); std::atomic caught{0}; @@ -414,7 +400,7 @@ TEST(Thread, PoolWaitException) { constexpr int threadCount = 3; for (int i = 0; i < threadCount; ++i) { - pool->schedule([=] { + pool->schedule(id, [=] { std::this_thread::sleep_for(Milliseconds(i)); if (i & 1) { throw std::runtime_error("test"); @@ -425,7 +411,7 @@ TEST(Thread, PoolWaitException) { } // Exceptions shouldn't cause deadlocks by, e.g., abandoning locks. - EXPECT_EQ(0, pool->waitForEmpty()); + pool->waitForEmpty(id); EXPECT_EQ(threadCount, caught); } @@ -434,8 +420,55 @@ 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 + +std::function makeCounterThread(TaggedScheduler& pool, + std::atomic* stopFlag, + std::atomic* counter) { + return [=]() mutable { + std::this_thread::sleep_for(std::chrono::microseconds(10)); + (*counter)++; + if (!*stopFlag) { + pool.schedule(makeCounterThread(pool, stopFlag, counter)); + } + }; +} + +TEST(Thread, TaggedPools) { + TaggedScheduler poolTag1{Scheduler::GetBackground(), {}}; + TaggedScheduler poolTag2{Scheduler::GetBackground(), {}}; + + std::atomic stopTasks1{false}; + std::atomic stopTasks2{false}; + std::atomic runCount1{0}; + std::atomic runCount2{0}; + + // Run a bunch of tasks on two different pool tags (that share the same thread pool) + for (auto i = 0; i < 50; i++) { + poolTag1.schedule(makeCounterThread(poolTag1, &stopTasks1, &runCount1)); + poolTag2.schedule(makeCounterThread(poolTag2, &stopTasks2, &runCount2)); + } + std::this_thread::sleep_for(std::chrono::milliseconds(150)); + + // Stop tasks on queue 1 and wait for it to empty + stopTasks1 = true; + poolTag1.waitForEmpty(); + const auto totalRuns1 = runCount1.load(); + + // Do the same for queue 2 + stopTasks2 = true; + poolTag2.waitForEmpty(); + const auto totalRuns2 = runCount2.load(); + + // No other tasks in queue 1 ran while waiting on queue 2. + ASSERT_TRUE(totalRuns1 == runCount1); + + std::this_thread::sleep_for(std::chrono::milliseconds(150)); + + // Same for queue 2 + ASSERT_TRUE(totalRuns2 == runCount2); +}