From 817c26111a0d6650e7ebae73e46621626106d0a7 Mon Sep 17 00:00:00 2001 From: Mike Morris Date: Wed, 12 Oct 2016 17:28:22 -0400 Subject: [core] [node] pass thread pool impl to Map constructor Updates mbgl::Map constructor usage everywhere Adds NodeThreadPool implementation using AsyncQueue to call Nan::AsyncQueueWorker from main thread --- benchmark/api/query.benchmark.cpp | 4 +- bin/glfw.cpp | 5 ++- bin/render.cpp | 6 ++- cmake/core-files.cmake | 2 - cmake/node.cmake | 2 + cmake/test-files.cmake | 2 +- include/mbgl/map/map.hpp | 5 ++- include/mbgl/platform/default/thread_pool.hpp | 27 +++++++++++++ platform/android/config.cmake | 3 ++ platform/android/src/native_map_view.cpp | 5 ++- platform/android/src/native_map_view.hpp | 2 + platform/default/thread_pool.cpp | 55 +++++++++++++++++++++++++++ platform/ios/config.cmake | 3 ++ platform/ios/src/MGLMapView.mm | 11 +++++- platform/linux/config.cmake | 3 ++ platform/macos/config.cmake | 3 ++ platform/macos/src/MGLMapView.mm | 9 ++++- platform/node/src/node_log.cpp | 3 +- platform/node/src/node_log.hpp | 5 ++- platform/node/src/node_map.cpp | 3 +- platform/node/src/node_map.hpp | 3 ++ platform/node/src/node_thread_pool.cpp | 39 +++++++++++++++++++ platform/node/src/node_thread_pool.hpp | 37 ++++++++++++++++++ platform/node/src/util/async_queue.hpp | 4 +- platform/qt/qt.cmake | 3 ++ platform/qt/src/qmapboxgl.cpp | 3 +- platform/qt/src/qmapboxgl_p.hpp | 2 + src/mbgl/actor/thread_pool.cpp | 55 --------------------------- src/mbgl/actor/thread_pool.hpp | 27 ------------- src/mbgl/map/map.cpp | 15 ++++---- test/actor/actor.test.cpp | 2 +- test/actor/actor_ref.test.cpp | 2 +- test/api/annotations.test.cpp | 4 +- test/api/api_misuse.test.cpp | 7 +++- test/api/custom_layer.test.cpp | 5 ++- test/api/query.test.cpp | 4 +- test/api/render_missing.test.cpp | 7 +++- test/api/repeated_render.test.cpp | 8 +++- test/map/map.test.cpp | 36 +++++++++--------- test/style/source.test.cpp | 2 +- test/tile/raster_tile.test.cpp | 2 +- test/tile/vector_tile.test.cpp | 2 +- test/util/memory.test.cpp | 12 +++--- 43 files changed, 292 insertions(+), 147 deletions(-) create mode 100644 include/mbgl/platform/default/thread_pool.hpp create mode 100644 platform/default/thread_pool.cpp create mode 100644 platform/node/src/node_thread_pool.cpp create mode 100644 platform/node/src/node_thread_pool.hpp delete mode 100644 src/mbgl/actor/thread_pool.cpp delete mode 100644 src/mbgl/actor/thread_pool.hpp diff --git a/benchmark/api/query.benchmark.cpp b/benchmark/api/query.benchmark.cpp index 7fa0781450..a9c4ade2da 100644 --- a/benchmark/api/query.benchmark.cpp +++ b/benchmark/api/query.benchmark.cpp @@ -4,6 +4,7 @@ #include #include #include +#include #include #include #include @@ -37,7 +38,8 @@ public: std::shared_ptr display{ std::make_shared() }; HeadlessView view{ display, 1 }; DefaultFileSource fileSource{ "benchmark/fixtures/api/cache.db", "." }; - Map map{ view, fileSource, MapMode::Still }; + ThreadPool threadPool{ 4 }; + Map map{ view, fileSource, threadPool, MapMode::Still }; ScreenBox box{{ 0, 0 }, { 1000, 1000 }}; }; diff --git a/bin/glfw.cpp b/bin/glfw.cpp index 142095f8a2..c0d61820f8 100644 --- a/bin/glfw.cpp +++ b/bin/glfw.cpp @@ -4,6 +4,7 @@ #include #include #include +#include #include #include @@ -115,7 +116,9 @@ int main(int argc, char *argv[]) { fileSource.setAccessToken(std::string(token)); } - mbgl::Map map(*view, fileSource); + mbgl::ThreadPool threadPool(4); + + mbgl::Map map(*view, fileSource, threadPool); // Load settings mbgl::Settings_JSON settings; diff --git a/bin/render.cpp b/bin/render.cpp index a43bf32ddd..e4d9951c05 100644 --- a/bin/render.cpp +++ b/bin/render.cpp @@ -3,8 +3,9 @@ #include #include -#include #include +#include +#include #include #pragma GCC diagnostic push @@ -84,7 +85,8 @@ int main(int argc, char *argv[]) { } HeadlessView view(pixelRatio, width, height); - Map map(view, fileSource, MapMode::Still); + ThreadPool threadPool(4); + Map map(view, fileSource, threadPool, MapMode::Still); map.setStyleJSON(style); map.setClasses(classes); diff --git a/cmake/core-files.cmake b/cmake/core-files.cmake index 8940899c16..fac5490bb4 100644 --- a/cmake/core-files.cmake +++ b/cmake/core-files.cmake @@ -8,8 +8,6 @@ set(MBGL_CORE_FILES src/mbgl/actor/mailbox.hpp src/mbgl/actor/message.hpp src/mbgl/actor/scheduler.hpp - src/mbgl/actor/thread_pool.cpp - src/mbgl/actor/thread_pool.hpp # algorithm src/mbgl/algorithm/covered_by_children.hpp diff --git a/cmake/node.cmake b/cmake/node.cmake index 75def7c468..8dd95a9419 100644 --- a/cmake/node.cmake +++ b/cmake/node.cmake @@ -11,6 +11,8 @@ target_sources(mbgl-node PRIVATE platform/node/src/node_request.cpp PRIVATE platform/node/src/node_feature.hpp PRIVATE platform/node/src/node_feature.cpp + PRIVATE platform/node/src/node_thread_pool.hpp + PRIVATE platform/node/src/node_thread_pool.cpp PRIVATE platform/node/src/util/async_queue.hpp # We are compiling with the uv loop, but since this target already has the headers for libuv, diff --git a/cmake/test-files.cmake b/cmake/test-files.cmake index 2ade1ba537..598e018224 100644 --- a/cmake/test-files.cmake +++ b/cmake/test-files.cmake @@ -89,9 +89,9 @@ set(MBGL_TEST_FILES # tile test/tile/geometry_tile_data.test.cpp test/tile/raster_tile.test.cpp + test/tile/tile_coordinate.test.cpp test/tile/tile_id.test.cpp test/tile/vector_tile.test.cpp - test/tile/tile_coordinate.test.cpp # util test/util/async_task.test.cpp diff --git a/include/mbgl/map/map.hpp b/include/mbgl/map/map.hpp index 914c2cd0b3..2831206d54 100644 --- a/include/mbgl/map/map.hpp +++ b/include/mbgl/map/map.hpp @@ -19,8 +19,9 @@ namespace mbgl { -class FileSource; class View; +class FileSource; +class Scheduler; class SpriteImage; struct CameraOptions; struct AnimationOptions; @@ -32,7 +33,7 @@ class Layer; class Map : private util::noncopyable { public: - explicit Map(View&, FileSource&, + explicit Map(View&, FileSource&, Scheduler&, MapMode mapMode = MapMode::Continuous, GLContextMode contextMode = GLContextMode::Unique, ConstrainMode constrainMode = ConstrainMode::HeightOnly, diff --git a/include/mbgl/platform/default/thread_pool.hpp b/include/mbgl/platform/default/thread_pool.hpp new file mode 100644 index 0000000000..a14d16d771 --- /dev/null +++ b/include/mbgl/platform/default/thread_pool.hpp @@ -0,0 +1,27 @@ +#pragma once + +#include + +#include +#include +#include +#include + +namespace mbgl { + +class ThreadPool : public Scheduler { +public: + ThreadPool(std::size_t count); + ~ThreadPool() override; + + void schedule(std::weak_ptr) override; + +private: + std::vector threads; + std::queue> queue; + std::mutex mutex; + std::condition_variable cv; + bool terminate { false }; +}; + +} // namespace mbgl diff --git a/platform/android/config.cmake b/platform/android/config.cmake index 1e6132dc30..74412e4ae5 100644 --- a/platform/android/config.cmake +++ b/platform/android/config.cmake @@ -48,6 +48,9 @@ macro(mbgl_platform_core) # Headless view # TODO + + # Thread pool + PRIVATE platform/default/thread_pool.cpp ) target_include_directories(mbgl-core diff --git a/platform/android/src/native_map_view.cpp b/platform/android/src/native_map_view.cpp index 2714b832e3..5bd2694932 100755 --- a/platform/android/src/native_map_view.cpp +++ b/platform/android/src/native_map_view.cpp @@ -60,7 +60,8 @@ NativeMapView::NativeMapView(JNIEnv *env_, jobject obj_, float pixelRatio_, int env(env_), pixelRatio(pixelRatio_), availableProcessors(availableProcessors_), - totalMemory(totalMemory_) { + totalMemory(totalMemory_), + threadPool(4) { mbgl::Log::Debug(mbgl::Event::Android, "NativeMapView::NativeMapView"); assert(env_ != nullptr); @@ -81,7 +82,7 @@ NativeMapView::NativeMapView(JNIEnv *env_, jobject obj_, float pixelRatio_, int mbgl::android::cachePath + "/mbgl-offline.db", mbgl::android::apkPath); - map = std::make_unique(*this, *fileSource, MapMode::Continuous); + map = std::make_unique(*this, *fileSource, threadPool, MapMode::Continuous); float zoomFactor = map->getMaxZoom() - map->getMinZoom() + 1; float cpuFactor = availableProcessors; diff --git a/platform/android/src/native_map_view.hpp b/platform/android/src/native_map_view.hpp index 7974f282d9..d42890dae2 100755 --- a/platform/android/src/native_map_view.hpp +++ b/platform/android/src/native_map_view.hpp @@ -3,6 +3,7 @@ #include #include #include +#include #include #include @@ -95,6 +96,7 @@ private: // Ensure these are initialised last std::unique_ptr fileSource; + mbgl::ThreadPool threadPool; std::unique_ptr map; mbgl::EdgeInsets insets; diff --git a/platform/default/thread_pool.cpp b/platform/default/thread_pool.cpp new file mode 100644 index 0000000000..7108312c58 --- /dev/null +++ b/platform/default/thread_pool.cpp @@ -0,0 +1,55 @@ +#include +#include + +namespace mbgl { + +ThreadPool::ThreadPool(std::size_t count) { + threads.reserve(count); + for (std::size_t i = 0; i < count; ++i) { + threads.emplace_back([this] () { + while (true) { + std::unique_lock lock(mutex); + + cv.wait(lock, [this] { + return !queue.empty() || terminate; + }); + + if (terminate) { + return; + } + + auto mailbox = queue.front(); + queue.pop(); + lock.unlock(); + + if (auto locked = mailbox.lock()) { + locked->receive(); + } + } + }); + } +} + +ThreadPool::~ThreadPool() { + { + std::lock_guard lock(mutex); + terminate = true; + } + + cv.notify_all(); + + for (auto& thread : threads) { + thread.join(); + } +} + +void ThreadPool::schedule(std::weak_ptr mailbox) { + { + std::lock_guard lock(mutex); + queue.push(mailbox); + } + + cv.notify_one(); +} + +} // namespace mbgl diff --git a/platform/ios/config.cmake b/platform/ios/config.cmake index 6c83395a3e..e5480e37df 100644 --- a/platform/ios/config.cmake +++ b/platform/ios/config.cmake @@ -40,6 +40,9 @@ macro(mbgl_platform_core) PRIVATE platform/darwin/src/headless_view_eagl.mm PRIVATE platform/default/headless_display.cpp PRIVATE platform/default/headless_view.cpp + + # Thread pool + PRIVATE platform/default/thread_pool.cpp ) target_add_mason_package(mbgl-core PUBLIC geojson) diff --git a/platform/ios/src/MGLMapView.mm b/platform/ios/src/MGLMapView.mm index 88f4d8a970..194f1a88d4 100644 --- a/platform/ios/src/MGLMapView.mm +++ b/platform/ios/src/MGLMapView.mm @@ -13,6 +13,7 @@ #include #include #include +#include #include #include #include @@ -258,6 +259,7 @@ public: { mbgl::Map *_mbglMap; MBGLView *_mbglView; + mbgl::ThreadPool *_mbglThreadPool; BOOL _opaque; @@ -397,7 +399,8 @@ public: // setup mbgl map mbgl::DefaultFileSource *mbglFileSource = [MGLOfflineStorage sharedOfflineStorage].mbglFileSource; - _mbglMap = new mbgl::Map(*_mbglView, *mbglFileSource, mbgl::MapMode::Continuous, mbgl::GLContextMode::Unique, mbgl::ConstrainMode::None, mbgl::ViewportMode::Default); + _mbglThreadPool = new mbgl::ThreadPool(4); + _mbglMap = new mbgl::Map(*_mbglView, *mbglFileSource, *_mbglThreadPool, mbgl::MapMode::Continuous, mbgl::GLContextMode::Unique, mbgl::ConstrainMode::None, mbgl::ViewportMode::Default); [self validateTileCacheSize]; // start paused if in IB @@ -650,6 +653,12 @@ public: _mbglView = nullptr; } + if (_mbglThreadPool) + { + delete _mbglThreadPool; + _mbglThreadPool = nullptr; + } + if ([[EAGLContext currentContext] isEqual:_context]) { [EAGLContext setCurrentContext:nil]; diff --git a/platform/linux/config.cmake b/platform/linux/config.cmake index 050a03bcb2..86e8697f8c 100644 --- a/platform/linux/config.cmake +++ b/platform/linux/config.cmake @@ -44,6 +44,9 @@ macro(mbgl_platform_core) PRIVATE platform/default/headless_display.cpp PRIVATE platform/default/headless_view.cpp PRIVATE platform/default/headless_view_glx.cpp + + # Thread pool + PRIVATE platform/default/thread_pool.cpp ) target_include_directories(mbgl-core diff --git a/platform/macos/config.cmake b/platform/macos/config.cmake index f8d4ab398f..645feb90c3 100644 --- a/platform/macos/config.cmake +++ b/platform/macos/config.cmake @@ -38,6 +38,9 @@ macro(mbgl_platform_core) PRIVATE platform/darwin/src/headless_view_cgl.cpp PRIVATE platform/default/headless_display.cpp PRIVATE platform/default/headless_view.cpp + + # Thread pool + PRIVATE platform/default/thread_pool.cpp ) target_add_mason_package(mbgl-core PUBLIC geojson) diff --git a/platform/macos/src/MGLMapView.mm b/platform/macos/src/MGLMapView.mm index a8f91a24e7..46766b573b 100644 --- a/platform/macos/src/MGLMapView.mm +++ b/platform/macos/src/MGLMapView.mm @@ -22,6 +22,7 @@ #import #import #import +#import #import #import #import @@ -158,6 +159,7 @@ public: /// Cross-platform map view controller. mbgl::Map *_mbglMap; MGLMapViewImpl *_mbglView; + mbgl::ThreadPool *_mbglThreadPool; NSPanGestureRecognizer *_panGestureRecognizer; NSMagnificationGestureRecognizer *_magnificationGestureRecognizer; @@ -261,7 +263,8 @@ public: [[NSFileManager defaultManager] removeItemAtURL:legacyCacheURL error:NULL]; mbgl::DefaultFileSource *mbglFileSource = [MGLOfflineStorage sharedOfflineStorage].mbglFileSource; - _mbglMap = new mbgl::Map(*_mbglView, *mbglFileSource, mbgl::MapMode::Continuous, mbgl::GLContextMode::Unique, mbgl::ConstrainMode::None, mbgl::ViewportMode::Default); + _mbglThreadPool = new mbgl::ThreadPool(4); + _mbglMap = new mbgl::Map(*_mbglView, *mbglFileSource, *_mbglThreadPool, mbgl::MapMode::Continuous, mbgl::GLContextMode::Unique, mbgl::ConstrainMode::None, mbgl::ViewportMode::Default); [self validateTileCacheSize]; // Install the OpenGL layer. Interface Builder’s synchronous drawing means @@ -466,6 +469,10 @@ public: delete _mbglView; _mbglView = nullptr; } + if (_mbglThreadPool) { + delete _mbglThreadPool; + _mbglThreadPool = nullptr; + } } - (void)observeValueForKeyPath:(NSString *)keyPath ofObject:(id)object change:(__unused NSDictionary *)change context:(void *)context { diff --git a/platform/node/src/node_log.cpp b/platform/node/src/node_log.cpp index 5ea0bcc06a..0a97ebce36 100644 --- a/platform/node/src/node_log.cpp +++ b/platform/node/src/node_log.cpp @@ -1,5 +1,4 @@ #include "node_log.hpp" -#include "util/async_queue.hpp" #include @@ -19,7 +18,7 @@ struct NodeLogObserver::LogMessage { }; NodeLogObserver::NodeLogObserver(v8::Local target) - : queue(new Queue(uv_default_loop(), [this](LogMessage &message) { + : queue(new util::AsyncQueue(uv_default_loop(), [this](LogMessage &message) { Nan::HandleScope scope; auto msg = Nan::New(); diff --git a/platform/node/src/node_log.hpp b/platform/node/src/node_log.hpp index d29e4e28e0..a19c61284b 100644 --- a/platform/node/src/node_log.hpp +++ b/platform/node/src/node_log.hpp @@ -1,5 +1,7 @@ #pragma once +#include "util/async_queue.hpp" + #include #pragma GCC diagnostic push @@ -24,8 +26,7 @@ private: Nan::Persistent module; struct LogMessage; - using Queue = util::AsyncQueue; - Queue *queue = nullptr; + util::AsyncQueue* queue; }; } diff --git a/platform/node/src/node_map.cpp b/platform/node/src/node_map.cpp index 4d4be5be66..a13adbc417 100644 --- a/platform/node/src/node_map.cpp +++ b/platform/node/src/node_map.cpp @@ -777,7 +777,8 @@ NodeMap::NodeMap(v8::Local options) : Nan::HandleScope scope; return Nan::Has(options, Nan::New("ratio").ToLocalChecked()).FromJust() ? Nan::Get(options, Nan::New("ratio").ToLocalChecked()).ToLocalChecked()->NumberValue() : 1.0; }()), - map(std::make_unique(view, *this, mbgl::MapMode::Still)), + threadpool(), + map(std::make_unique(view, *this, threadpool, mbgl::MapMode::Still)), async(new uv_async_t) { view.setMapChangeCallback([&](mbgl::MapChange change) { diff --git a/platform/node/src/node_map.hpp b/platform/node/src/node_map.hpp index c0b025c369..cdca1587ea 100644 --- a/platform/node/src/node_map.hpp +++ b/platform/node/src/node_map.hpp @@ -1,5 +1,7 @@ #pragma once +#include "node_thread_pool.hpp" + #include #include #include @@ -52,6 +54,7 @@ public: std::unique_ptr request(const mbgl::Resource&, mbgl::FileSource::Callback); mbgl::HeadlessView view; + NodeThreadPool threadpool; std::unique_ptr map; std::exception_ptr error; diff --git a/platform/node/src/node_thread_pool.cpp b/platform/node/src/node_thread_pool.cpp new file mode 100644 index 0000000000..a9faef6f09 --- /dev/null +++ b/platform/node/src/node_thread_pool.cpp @@ -0,0 +1,39 @@ +#include "node_thread_pool.hpp" +#include "util/async_queue.hpp" + +#include + +namespace node_mbgl { + +NodeThreadPool::NodeThreadPool() + : queue(new util::AsyncQueue>(uv_default_loop(), [this](std::weak_ptr mailbox) { + Worker* worker = new Worker(mailbox); + Nan::AsyncQueueWorker(worker); + })) { + // Don't keep the event loop alive. + queue->unref(); +} + +NodeThreadPool::~NodeThreadPool() { + queue->stop(); +} + +void NodeThreadPool::schedule(std::weak_ptr mailbox) { + queue->send(std::move(mailbox)); +} + +NodeThreadPool::Worker::Worker(std::weak_ptr mailbox_) + : AsyncWorker(nullptr), + mailbox(std::move(mailbox_)) {}; + +void NodeThreadPool::Worker::Execute() { + if (auto locked = mailbox.lock()) { + locked->receive(); + } +} + +void NodeThreadPool::Worker::WorkComplete() { + // no-op to avoid calling nullptr callback +} + +} // namespace node_mbgl diff --git a/platform/node/src/node_thread_pool.hpp b/platform/node/src/node_thread_pool.hpp new file mode 100644 index 0000000000..d412e53d3d --- /dev/null +++ b/platform/node/src/node_thread_pool.hpp @@ -0,0 +1,37 @@ +#pragma once + +#include + +#pragma GCC diagnostic push +#pragma GCC diagnostic ignored "-Wunused-parameter" +#pragma GCC diagnostic ignored "-Wshadow" +#include +#pragma GCC diagnostic pop + +namespace node_mbgl { + +namespace util { template class AsyncQueue; } + +class NodeThreadPool : public mbgl::Scheduler { +public: + NodeThreadPool(); + ~NodeThreadPool(); + + void schedule(std::weak_ptr) override; + +private: + util::AsyncQueue>* queue; + + class Worker : public Nan::AsyncWorker { + public: + Worker(std::weak_ptr); + + void Execute(); + void WorkComplete(); + + private: + std::weak_ptr mailbox; + }; +}; + +} // namespace node_mbgl diff --git a/platform/node/src/util/async_queue.hpp b/platform/node/src/util/async_queue.hpp index b9081b3aeb..87737437c3 100644 --- a/platform/node/src/util/async_queue.hpp +++ b/platform/node/src/util/async_queue.hpp @@ -28,6 +28,7 @@ public: q->process(); }); } + ~AsyncQueue() {} void send(T &&data) { { @@ -60,9 +61,6 @@ public: } private: - ~AsyncQueue() { - } - void process() { std::unique_ptr item; while (true) { diff --git a/platform/qt/qt.cmake b/platform/qt/qt.cmake index dc60455b0a..398e173cfa 100644 --- a/platform/qt/qt.cmake +++ b/platform/qt/qt.cmake @@ -30,6 +30,9 @@ set(MBGL_QT_FILES # Misc PRIVATE platform/default/log_stderr.cpp + # Thread pool + PRIVATE platform/default/thread_pool.cpp + # Platform integration PRIVATE platform/qt/src/async_task.cpp PRIVATE platform/qt/src/async_task_impl.hpp diff --git a/platform/qt/src/qmapboxgl.cpp b/platform/qt/src/qmapboxgl.cpp index 103ed83256..d0a05301f8 100644 --- a/platform/qt/src/qmapboxgl.cpp +++ b/platform/qt/src/qmapboxgl.cpp @@ -811,8 +811,9 @@ QMapboxGLPrivate::QMapboxGLPrivate(QMapboxGL *q, const QMapboxGLSettings &settin settings.cacheDatabasePath().toStdString(), settings.assetPath().toStdString(), settings.cacheDatabaseMaximumSize())) + , threadPool(4) , mapObj(std::make_unique( - *this, *fileSourceObj, + *this, *fileSourceObj, threadPool, static_cast(settings.mapMode()), static_cast(settings.contextMode()), static_cast(settings.constrainMode()), diff --git a/platform/qt/src/qmapboxgl_p.hpp b/platform/qt/src/qmapboxgl_p.hpp index 11b5f695ca..c35ac4e7ba 100644 --- a/platform/qt/src/qmapboxgl_p.hpp +++ b/platform/qt/src/qmapboxgl_p.hpp @@ -4,6 +4,7 @@ #include #include +#include #include #include @@ -34,6 +35,7 @@ public: QMapboxGL *q_ptr { nullptr }; std::unique_ptr fileSourceObj; + mbgl::ThreadPool threadPool; std::unique_ptr mapObj; bool dirty { false }; diff --git a/src/mbgl/actor/thread_pool.cpp b/src/mbgl/actor/thread_pool.cpp deleted file mode 100644 index 2995ed26ad..0000000000 --- a/src/mbgl/actor/thread_pool.cpp +++ /dev/null @@ -1,55 +0,0 @@ -#include -#include - -namespace mbgl { - -ThreadPool::ThreadPool(std::size_t count) { - threads.reserve(count); - for (std::size_t i = 0; i < count; ++i) { - threads.emplace_back([this] () { - while (true) { - std::unique_lock lock(mutex); - - cv.wait(lock, [this] { - return !queue.empty() || terminate; - }); - - if (terminate) { - return; - } - - auto mailbox = queue.front(); - queue.pop(); - lock.unlock(); - - if (auto locked = mailbox.lock()) { - locked->receive(); - } - } - }); - } -} - -ThreadPool::~ThreadPool() { - { - std::lock_guard lock(mutex); - terminate = true; - } - - cv.notify_all(); - - for (auto& thread : threads) { - thread.join(); - } -} - -void ThreadPool::schedule(std::weak_ptr mailbox) { - { - std::lock_guard lock(mutex); - queue.push(mailbox); - } - - cv.notify_one(); -} - -} // namespace mbgl diff --git a/src/mbgl/actor/thread_pool.hpp b/src/mbgl/actor/thread_pool.hpp deleted file mode 100644 index a14d16d771..0000000000 --- a/src/mbgl/actor/thread_pool.hpp +++ /dev/null @@ -1,27 +0,0 @@ -#pragma once - -#include - -#include -#include -#include -#include - -namespace mbgl { - -class ThreadPool : public Scheduler { -public: - ThreadPool(std::size_t count); - ~ThreadPool() override; - - void schedule(std::weak_ptr) override; - -private: - std::vector threads; - std::queue> queue; - std::mutex mutex; - std::condition_variable cv; - bool terminate { false }; -}; - -} // namespace mbgl diff --git a/src/mbgl/map/map.cpp b/src/mbgl/map/map.cpp index bf2462e2ab..d948225d8a 100644 --- a/src/mbgl/map/map.cpp +++ b/src/mbgl/map/map.cpp @@ -21,7 +21,7 @@ #include #include #include -#include +#include #include namespace mbgl { @@ -36,7 +36,7 @@ enum class RenderState : uint8_t { class Map::Impl : public style::Observer { public: - Impl(View&, FileSource&, MapMode, GLContextMode, ConstrainMode, ViewportMode); + Impl(View&, FileSource&, Scheduler&, MapMode, GLContextMode, ConstrainMode, ViewportMode); void onSourceAttributionChanged(style::Source&, const std::string&) override; void onUpdate(Update) override; @@ -51,6 +51,7 @@ public: View& view; FileSource& fileSource; + Scheduler& scheduler; RenderState renderState = RenderState::Never; Transform transform; @@ -63,7 +64,6 @@ public: Update updateFlags = Update::Nothing; util::AsyncTask asyncUpdate; - ThreadPool workerThreadPool; std::unique_ptr annotationManager; std::unique_ptr painter; @@ -81,20 +81,22 @@ public: bool loading = false; }; -Map::Map(View& view, FileSource& fileSource, MapMode mapMode, GLContextMode contextMode, ConstrainMode constrainMode, ViewportMode viewportMode) - : impl(std::make_unique(view, fileSource, mapMode, contextMode, constrainMode, viewportMode)) { +Map::Map(View& view, FileSource& fileSource, Scheduler& scheduler, MapMode mapMode, GLContextMode contextMode, ConstrainMode constrainMode, ViewportMode viewportMode) + : impl(std::make_unique(view, fileSource, scheduler, mapMode, contextMode, constrainMode, viewportMode)) { view.initialize(this); update(Update::Dimensions); } Map::Impl::Impl(View& view_, FileSource& fileSource_, + Scheduler& scheduler_, MapMode mode_, GLContextMode contextMode_, ConstrainMode constrainMode_, ViewportMode viewportMode_) : view(view_), fileSource(fileSource_), + scheduler(scheduler_), transform([this](MapChange change) { view.notifyMapChange(change); }, constrainMode_, viewportMode_), @@ -102,7 +104,6 @@ Map::Impl::Impl(View& view_, contextMode(contextMode_), pixelRatio(view.getPixelRatio()), asyncUpdate([this] { update(); }), - workerThreadPool(4), annotationManager(std::make_unique(pixelRatio)) { } @@ -230,7 +231,7 @@ void Map::Impl::update() { style::UpdateParameters parameters(pixelRatio, debugOptions, transform.getState(), - workerThreadPool, + scheduler, fileSource, mode, *annotationManager, diff --git a/test/actor/actor.test.cpp b/test/actor/actor.test.cpp index 7bb76784d6..c24d0b6c25 100644 --- a/test/actor/actor.test.cpp +++ b/test/actor/actor.test.cpp @@ -1,5 +1,5 @@ #include -#include +#include #include diff --git a/test/actor/actor_ref.test.cpp b/test/actor/actor_ref.test.cpp index 655529035f..4be714278e 100644 --- a/test/actor/actor_ref.test.cpp +++ b/test/actor/actor_ref.test.cpp @@ -1,5 +1,5 @@ #include -#include +#include #include diff --git a/test/api/annotations.test.cpp b/test/api/annotations.test.cpp index 6d6734a10f..c1716a5228 100644 --- a/test/api/annotations.test.cpp +++ b/test/api/annotations.test.cpp @@ -1,6 +1,7 @@ #include #include +#include #include #include #include @@ -25,7 +26,8 @@ public: std::shared_ptr display { std::make_shared() }; HeadlessView view { display, 1 }; StubFileSource fileSource; - Map map { view, fileSource, MapMode::Still }; + ThreadPool threadPool { 4 }; + Map map { view, fileSource, threadPool, MapMode::Still }; void checkRendering(const char * name) { test::checkImage(std::string("test/fixtures/annotations/") + name, diff --git a/test/api/api_misuse.test.cpp b/test/api/api_misuse.test.cpp index e9fe307718..2016e278da 100644 --- a/test/api/api_misuse.test.cpp +++ b/test/api/api_misuse.test.cpp @@ -5,6 +5,7 @@ #include #include #include +#include #include #include @@ -22,8 +23,9 @@ TEST(API, RenderWithoutCallback) { HeadlessView view(display, 1); view.resize(128, 512); StubFileSource fileSource; + ThreadPool threadPool(4); - std::unique_ptr map = std::make_unique(view, fileSource, MapMode::Still); + std::unique_ptr map = std::make_unique(view, fileSource, threadPool, MapMode::Still); map->renderStill(nullptr); // Force Map thread to join. @@ -46,8 +48,9 @@ TEST(API, RenderWithoutStyle) { HeadlessView view(display, 1); view.resize(128, 512); StubFileSource fileSource; + ThreadPool threadPool(4); - Map map(view, fileSource, MapMode::Still); + Map map(view, fileSource, threadPool, MapMode::Still); std::exception_ptr error; map.renderStill([&](std::exception_ptr error_, PremultipliedImage&&) { diff --git a/test/api/custom_layer.test.cpp b/test/api/custom_layer.test.cpp index 2dded19256..1342dfa50c 100644 --- a/test/api/custom_layer.test.cpp +++ b/test/api/custom_layer.test.cpp @@ -4,6 +4,7 @@ #include #include #include +#include #include #include #include @@ -94,7 +95,9 @@ TEST(CustomLayer, Basic) { DefaultFileSource fileSource(":memory:", "test/fixtures/api/assets"); #endif - Map map(view, fileSource, MapMode::Still); + ThreadPool threadPool(4); + + Map map(view, fileSource, threadPool, MapMode::Still); map.setStyleJSON(util::read_file("test/fixtures/api/water.json")); map.setLatLngZoom({ 37.8, -122.5 }, 10); map.addLayer(std::make_unique( diff --git a/test/api/query.test.cpp b/test/api/query.test.cpp index 7da0f4311c..da38dd0cb2 100644 --- a/test/api/query.test.cpp +++ b/test/api/query.test.cpp @@ -1,6 +1,7 @@ #include #include #include +#include #include #include #include @@ -28,7 +29,8 @@ public: std::shared_ptr display { std::make_shared() }; HeadlessView view { display, 1 }; StubFileSource fileSource; - Map map { view, fileSource, MapMode::Still }; + ThreadPool threadPool { 4 }; + Map map { view, fileSource, threadPool, MapMode::Still }; }; } // end namespace diff --git a/test/api/render_missing.test.cpp b/test/api/render_missing.test.cpp index 024ebd3729..d57a5a98bd 100644 --- a/test/api/render_missing.test.cpp +++ b/test/api/render_missing.test.cpp @@ -2,8 +2,9 @@ #include #include -#include #include +#include +#include #include #include #include @@ -33,9 +34,11 @@ TEST(API, TEST_REQUIRES_SERVER(RenderMissingTile)) { DefaultFileSource fileSource(":memory:", "test/fixtures/api/assets"); #endif + ThreadPool threadPool(4); + Log::setObserver(std::make_unique()); - Map map(view, fileSource, MapMode::Still); + Map map(view, fileSource, threadPool, MapMode::Still); std::string message; diff --git a/test/api/repeated_render.test.cpp b/test/api/repeated_render.test.cpp index cf71cb8416..4229f9d7ad 100644 --- a/test/api/repeated_render.test.cpp +++ b/test/api/repeated_render.test.cpp @@ -2,8 +2,9 @@ #include #include -#include #include +#include +#include #include #include #include @@ -20,6 +21,7 @@ TEST(API, RepeatedRender) { auto display = std::make_shared(); HeadlessView view(display, 1, 256, 512); + #ifdef MBGL_ASSET_ZIP // Regenerate with `cd test/fixtures/api/ && zip -r assets.zip assets/` DefaultFileSource fileSource(":memory:", "test/fixtures/api/assets.zip"); @@ -27,9 +29,11 @@ TEST(API, RepeatedRender) { DefaultFileSource fileSource(":memory:", "test/fixtures/api/assets"); #endif + ThreadPool threadPool(4); + Log::setObserver(std::make_unique()); - Map map(view, fileSource, MapMode::Still); + Map map(view, fileSource, threadPool, MapMode::Still); { map.setStyleJSON(style); diff --git a/test/map/map.test.cpp b/test/map/map.test.cpp index 5b66d9b753..c03a038dc0 100644 --- a/test/map/map.test.cpp +++ b/test/map/map.test.cpp @@ -4,8 +4,9 @@ #include #include -#include #include +#include +#include #include #include #include @@ -24,11 +25,12 @@ struct MapTest { std::shared_ptr display { std::make_shared() }; HeadlessView view { display, 1 }; StubFileSource fileSource; + ThreadPool threadPool { 4 }; }; TEST(Map, LatLngBehavior) { MapTest test; - Map map(test.view, test.fileSource, MapMode::Still); + Map map(test.view, test.fileSource, test.threadPool, MapMode::Still); map.setStyleJSON(util::read_file("test/fixtures/api/empty.json")); @@ -62,7 +64,7 @@ TEST(Map, Offline) { fileSource.put(Resource::glyphs(prefix + "{fontstack}/{range}.pbf", {{"Helvetica"}}, {0, 255}), expiredItem("glyph.pbf")); NetworkStatus::Set(NetworkStatus::Status::Offline); - Map map(test.view, fileSource, MapMode::Still); + Map map(test.view, fileSource, test.threadPool, MapMode::Still); map.setStyleURL(prefix + "style.json"); test::checkImage("test/fixtures/map/offline", @@ -86,7 +88,7 @@ TEST(Map, SetStyleInvalidJSON) { }); { - Map map(test.view, test.fileSource, MapMode::Still); + Map map(test.view, test.fileSource, test.threadPool, MapMode::Still); map.setStyleJSON("invalid"); } @@ -117,7 +119,7 @@ TEST(Map, SetStyleInvalidURL) { } }); - Map map(test.view, test.fileSource, MapMode::Still); + Map map(test.view, test.fileSource, test.threadPool, MapMode::Still); map.setStyleURL("mapbox://bar"); test.runLoop.run(); @@ -126,7 +128,7 @@ TEST(Map, SetStyleInvalidURL) { TEST(Map, DoubleStyleLoad) { MapTest test; - Map map(test.view, test.fileSource, MapMode::Still); + Map map(test.view, test.fileSource, test.threadPool, MapMode::Still); map.setStyleJSON(""); map.setStyleJSON(""); } @@ -137,7 +139,7 @@ TEST(Map, StyleFresh) { MapTest test; FakeFileSource fileSource; - Map map(test.view, fileSource, MapMode::Still); + Map map(test.view, fileSource, test.threadPool, MapMode::Still); map.setStyleURL("mapbox://styles/test"); EXPECT_EQ(1u, fileSource.requests.size()); @@ -157,7 +159,7 @@ TEST(Map, StyleExpired) { MapTest test; FakeFileSource fileSource; - Map map(test.view, fileSource, MapMode::Still); + Map map(test.view, fileSource, test.threadPool, MapMode::Still); map.setStyleURL("mapbox://styles/test"); EXPECT_EQ(1u, fileSource.requests.size()); @@ -184,7 +186,7 @@ TEST(Map, StyleExpiredWithAnnotations) { MapTest test; FakeFileSource fileSource; - Map map(test.view, fileSource, MapMode::Still); + Map map(test.view, fileSource, test.threadPool, MapMode::Still); map.setStyleURL("mapbox://styles/test"); EXPECT_EQ(1u, fileSource.requests.size()); @@ -208,7 +210,7 @@ TEST(Map, StyleEarlyMutation) { MapTest test; FakeFileSource fileSource; - Map map(test.view, fileSource, MapMode::Still); + Map map(test.view, fileSource, test.threadPool, MapMode::Still); map.setStyleURL("mapbox://styles/test"); map.addLayer(std::make_unique("bg")); @@ -222,7 +224,7 @@ TEST(Map, StyleEarlyMutation) { TEST(Map, StyleLoadedSignal) { MapTest test; - Map map(test.view, test.fileSource, MapMode::Still); + Map map(test.view, test.fileSource, test.threadPool, MapMode::Still); // The map should emit a signal on style loaded bool emitted = false; @@ -243,7 +245,7 @@ TEST(Map, StyleLoadedSignal) { TEST(Map, AddLayer) { MapTest test; - Map map(test.view, test.fileSource, MapMode::Still); + Map map(test.view, test.fileSource, test.threadPool, MapMode::Still); map.setStyleJSON(util::read_file("test/fixtures/api/empty.json")); auto layer = std::make_unique("background"); @@ -256,7 +258,7 @@ TEST(Map, AddLayer) { TEST(Map, RemoveLayer) { MapTest test; - Map map(test.view, test.fileSource, MapMode::Still); + Map map(test.view, test.fileSource, test.threadPool, MapMode::Still); map.setStyleJSON(util::read_file("test/fixtures/api/empty.json")); auto layer = std::make_unique("background"); @@ -281,7 +283,7 @@ TEST(Map, DisabledSources) { return {}; }; - Map map(test.view, test.fileSource, MapMode::Still); + Map map(test.view, test.fileSource, test.threadPool, MapMode::Still); map.setZoom(1); // This stylesheet has two raster layers, one that starts at zoom 1, the other at zoom 0. @@ -331,7 +333,7 @@ TEST(Map, DisabledSources) { TEST(Map, Classes) { MapTest test; - Map map(test.view, test.fileSource, MapMode::Still); + Map map(test.view, test.fileSource, test.threadPool, MapMode::Still); map.setStyleJSON(util::read_file("test/fixtures/api/empty.json")); EXPECT_FALSE(map.getTransitionOptions().duration); @@ -365,7 +367,7 @@ TEST(Map, Classes) { TEST(Map, AddImage) { MapTest test; - Map map(test.view, test.fileSource, MapMode::Still); + Map map(test.view, test.fileSource, test.threadPool, MapMode::Still); auto decoded1 = decodeImage(util::read_file("test/fixtures/sprites/default_marker.png")); auto decoded2 = decodeImage(util::read_file("test/fixtures/sprites/default_marker.png")); auto image1 = std::make_unique(std::move(decoded1), 1.0); @@ -382,7 +384,7 @@ TEST(Map, AddImage) { TEST(Map, RemoveImage) { MapTest test; - Map map(test.view, test.fileSource, MapMode::Still); + Map map(test.view, test.fileSource, test.threadPool, MapMode::Still); auto decoded = decodeImage(util::read_file("test/fixtures/sprites/default_marker.png")); auto image = std::make_unique(std::move(decoded), 1.0); diff --git a/test/style/source.test.cpp b/test/style/source.test.cpp index 3270894a59..3aebe10769 100644 --- a/test/style/source.test.cpp +++ b/test/style/source.test.cpp @@ -11,7 +11,7 @@ #include #include #include -#include +#include #include #include diff --git a/test/tile/raster_tile.test.cpp b/test/tile/raster_tile.test.cpp index 4e9e907321..2a15fd1b99 100644 --- a/test/tile/raster_tile.test.cpp +++ b/test/tile/raster_tile.test.cpp @@ -3,7 +3,7 @@ #include #include -#include +#include #include #include #include diff --git a/test/tile/vector_tile.test.cpp b/test/tile/vector_tile.test.cpp index fa75e24480..c07db42c3c 100644 --- a/test/tile/vector_tile.test.cpp +++ b/test/tile/vector_tile.test.cpp @@ -3,7 +3,7 @@ #include #include -#include +#include #include #include #include diff --git a/test/util/memory.test.cpp b/test/util/memory.test.cpp index d1b9f2412a..e2ace99c41 100644 --- a/test/util/memory.test.cpp +++ b/test/util/memory.test.cpp @@ -4,6 +4,7 @@ #include #include #include +#include #include #include @@ -58,6 +59,7 @@ public: std::shared_ptr display { std::make_shared() }; HeadlessView view { display, 2 }; StubFileSource fileSource; + ThreadPool threadPool { 4 }; private: Response response(const std::string& path) { @@ -91,7 +93,7 @@ private: TEST(Memory, Vector) { MemoryTest test; - Map map(test.view, test.fileSource, MapMode::Still); + Map map(test.view, test.fileSource, test.threadPool, MapMode::Still); map.setZoom(16); // more map features map.setStyleURL("mapbox://streets"); @@ -101,7 +103,7 @@ TEST(Memory, Vector) { TEST(Memory, Raster) { MemoryTest test; - Map map(test.view, test.fileSource, MapMode::Still); + Map map(test.view, test.fileSource, test.threadPool, MapMode::Still); map.setStyleURL("mapbox://satellite"); test::render(map); @@ -128,7 +130,7 @@ TEST(Memory, Footprint) { // Warm up buffers and cache. for (unsigned i = 0; i < 10; ++i) { - Map map(test.view, test.fileSource, MapMode::Still); + Map map(test.view, test.fileSource,test.threadPool, MapMode::Still); renderMap(&map, "mapbox://streets"); renderMap(&map, "mapbox://satellite"); }; @@ -142,7 +144,7 @@ TEST(Memory, Footprint) { long vectorInitialRSS = getRSS(); for (unsigned i = 0; i < runs; ++i) { - auto vector = std::make_unique(test.view, test.fileSource, MapMode::Still); + auto vector = std::make_unique(test.view, test.fileSource, test.threadPool, MapMode::Still); renderMap(vector.get(), "mapbox://streets"); maps.push_back(std::move(vector)); }; @@ -151,7 +153,7 @@ TEST(Memory, Footprint) { long rasterInitialRSS = getRSS(); for (unsigned i = 0; i < runs; ++i) { - auto raster = std::make_unique(test.view, test.fileSource, MapMode::Still); + auto raster = std::make_unique(test.view, test.fileSource, test.threadPool, MapMode::Still); renderMap(raster.get(), "mapbox://satellite"); maps.push_back(std::move(raster)); }; -- cgit v1.2.1