Scope the Tile worker thread to an Environment

tmpsantos · kkaefer · commit aa9adbb8f89e · 2015-03-18T12:51:33.000+01:00
We are also removing the code that is passing the Environment around
and instead, we get now the Environment in the TileData using
Environment::Get().

When processing the job, the work will be temporally registered to
the Environment as "TileWorker_0/0/0" which can be used for logging
in the future. At the end of the job, it gets unregistered
automatically from the Environment.
diff --git a/.clang-format b/.clang-format
@@ -2,15 +2,15 @@ Standard: Cpp11
 IndentWidth: 4
 AccessModifierOffset: -4
 UseTab: Never
-BinPackParameters: true
+BinPackParameters: false
 AllowShortIfStatementsOnASingleLine: false
 AllowShortLoopsOnASingleLine: false
 AllowShortBlocksOnASingleLine: false
 AllowShortFunctionsOnASingleLine: false
 ConstructorInitializerAllOnOneLineOrOnePerLine: true
 AlwaysBreakTemplateDeclarations: true
 NamespaceIndentation: None
-PointerBindsToType: false
+PointerBindsToType: true
 SpacesInParentheses: false
 BreakBeforeBraces: Attach
 ColumnLimit: 100
diff --git a/src/mbgl/map/environment.cpp b/src/mbgl/map/environment.cpp
@@ -35,7 +35,7 @@ class ThreadInfoStore {
         // FIXME: We should never need to overwrite a thread here and we only allow
         // this today because on the Static mode, the Map thread and the Main thread
         // are same. Replace this with emplace() when this gets fixed.
-        threadSet[std::this_thread::get_id()] = ThreadInfo{env, type, name};
+        threadSet[std::this_thread::get_id()] = ThreadInfo{ env, type, name };
     }
 
     void unregisterThread() {
@@ -80,7 +80,7 @@ Environment::Scope::~Scope() {
     threadInfoStore.unregisterThread();
 }
 
-Environment::Environment(FileSource &fs) : fileSource(fs), loop(uv_loop_new()) {
+Environment::Environment(FileSource& fs) : fileSource(fs), loop(uv_loop_new()) {
 }
 
 Environment& Environment::Get() {
@@ -102,16 +102,18 @@ std::string Environment::threadName() {
     return threadInfoStore.getThreadInfo().name;
 }
 
-void Environment::requestAsync(const Resource &resource, std::function<void(const Response &)> callback) {
+void Environment::requestAsync(const Resource& resource,
+                               std::function<void(const Response&)> callback) {
     fileSource.request(resource, *this, std::move(callback));
 }
 
-Request *Environment::request(const Resource &resource, std::function<void(const Response &)> callback) {
+Request* Environment::request(const Resource& resource,
+                              std::function<void(const Response&)> callback) {
     assert(currentlyOn(ThreadType::Map));
     return fileSource.request(resource, loop, *this, std::move(callback));
 }
 
-void Environment::cancelRequest(Request *req) {
+void Environment::cancelRequest(Request* req) {
     assert(currentlyOn(ThreadType::Map));
     fileSource.cancel(req);
 }
diff --git a/src/mbgl/map/environment.hpp b/src/mbgl/map/environment.hpp
@@ -20,6 +20,7 @@ enum class ThreadType : uint8_t {
     Unknown    = 0,
     Main       = 1 << 0,
     Map        = 1 << 1,
+    TileWorker = 1 << 2,
 };
 
 class Environment final : private util::noncopyable {
@@ -33,25 +34,25 @@ class Environment final : private util::noncopyable {
         std::thread::id id;
     };
 
-    Environment(FileSource &);
+    Environment(FileSource&);
 
     static Environment& Get();
     static bool inScope();
     static bool currentlyOn(ThreadType);
     static std::string threadName();
 
-    void requestAsync(const Resource &, std::function<void(const Response &)>);
-    Request *request(const Resource &, std::function<void(const Response &)>);
-    void cancelRequest(Request *);
+    void requestAsync(const Resource&, std::function<void(const Response&)>);
+    Request* request(const Resource&, std::function<void(const Response&)>);
+    void cancelRequest(Request*);
 
     // Request to terminate the environment.
     void terminate();
 
 private:
-    FileSource &fileSource;
+    FileSource& fileSource;
 
 public:
-    uv_loop_t *const loop;
+    uv_loop_t* const loop;
 };
 
 }
diff --git a/src/mbgl/map/live_tile_data.cpp b/src/mbgl/map/live_tile_data.cpp
@@ -15,10 +15,9 @@ LiveTileData::LiveTileData(Tile::ID const& id_,
                            GlyphStore& glyphStore_,
                            SpriteAtlas& spriteAtlas_,
                            util::ptr<Sprite> sprite_,
-                           const SourceInfo& source_,
-                           Environment& env_)
+                           const SourceInfo& source_)
     : VectorTileData::VectorTileData(id_, mapMaxZoom, style_, glyphAtlas_, glyphStore_,
-                                     spriteAtlas_, sprite_, source_, env_),
+                                     spriteAtlas_, sprite_, source_),
       annotationManager(annotationManager_) {
     // live features are always ready
     state = State::loaded;
diff --git a/src/mbgl/map/live_tile_data.hpp b/src/mbgl/map/live_tile_data.hpp
@@ -17,8 +17,7 @@ class LiveTileData : public VectorTileData {
                  GlyphStore&,
                  SpriteAtlas&,
                  util::ptr<Sprite>,
-                 const SourceInfo&,
-                 Environment&);
+                 const SourceInfo&);
     ~LiveTileData();
 
     void parse() override;
diff --git a/src/mbgl/map/map.cpp b/src/mbgl/map/map.cpp
@@ -694,7 +694,7 @@ void Map::updateSources(const util::ptr<StyleLayerGroup> &group) {
 void Map::updateTiles() {
     assert(Environment::currentlyOn(ThreadType::Map));
     for (const auto &source : activeSources) {
-        source->source->update(*this, *env, getWorker(), style, *glyphAtlas, *glyphStore,
+        source->source->update(*this, getWorker(), style, *glyphAtlas, *glyphStore,
                                *spriteAtlas, getSprite(), *texturePool, [this]() {
             assert(Environment::currentlyOn(ThreadType::Map));
             triggerUpdate();
diff --git a/src/mbgl/map/raster_tile_data.cpp b/src/mbgl/map/raster_tile_data.cpp
@@ -5,8 +5,8 @@
 using namespace mbgl;
 
 RasterTileData::RasterTileData(Tile::ID const &id_, TexturePool &texturePool,
-                               const SourceInfo &source_, Environment &env_)
-    : TileData(id_, source_, env_), bucket(texturePool, layout) {
+                               const SourceInfo &source_)
+    : TileData(id_, source_), bucket(texturePool, layout) {
 }
 
 RasterTileData::~RasterTileData() {
diff --git a/src/mbgl/map/raster_tile_data.hpp b/src/mbgl/map/raster_tile_data.hpp
@@ -17,7 +17,7 @@ class RasterTileData : public TileData {
     friend class TileParser;
 
 public:
-    RasterTileData(Tile::ID const &id, TexturePool &, const SourceInfo &, Environment &);
+    RasterTileData(Tile::ID const &id, TexturePool &, const SourceInfo &);
     ~RasterTileData();
 
     void parse() override;
diff --git a/src/mbgl/map/source.cpp b/src/mbgl/map/source.cpp
@@ -155,7 +155,7 @@ TileData::State Source::hasTile(const Tile::ID& id) {
     return TileData::State::invalid;
 }
 
-TileData::State Source::addTile(Map &map, Environment &env, uv::worker &worker,
+TileData::State Source::addTile(Map &map, uv::worker &worker,
                                 util::ptr<Style> style, GlyphAtlas &glyphAtlas,
                                 GlyphStore &glyphStore, SpriteAtlas &spriteAtlas,
                                 util::ptr<Sprite> sprite, TexturePool &texturePool,
@@ -187,22 +187,18 @@ TileData::State Source::addTile(Map &map, Environment &env, uv::worker &worker,
     if (!new_tile.data) {
         // If we don't find working tile data, we're just going to load it.
         if (info.type == SourceType::Vector) {
-            new_tile.data = std::make_shared<VectorTileData>(normalized_id, map.getMaxZoom(), style,
-                                                             glyphAtlas, glyphStore,
-                                                             spriteAtlas, sprite,
-                                                             info, env);
+            new_tile.data =
+                std::make_shared<VectorTileData>(normalized_id, map.getMaxZoom(), style, glyphAtlas,
+                                                 glyphStore, spriteAtlas, sprite, info);
             new_tile.data->request(worker, map.getState().getPixelRatio(), callback);
         } else if (info.type == SourceType::Raster) {
-            new_tile.data = std::make_shared<RasterTileData>(normalized_id, texturePool, info, env);
+            new_tile.data = std::make_shared<RasterTileData>(normalized_id, texturePool, info);
             new_tile.data->request(worker, map.getState().getPixelRatio(), callback);
         } else if (info.type == SourceType::Annotations) {
             AnnotationManager& annotationManager = map.getAnnotationManager();
-            new_tile.data = std::make_shared<LiveTileData>(normalized_id,
-                                                           annotationManager,
-                                                           map.getMaxZoom(), style,
-                                                           glyphAtlas, glyphStore,
-                                                           spriteAtlas, sprite,
-                                                           info, env);
+            new_tile.data = std::make_shared<LiveTileData>(normalized_id, annotationManager,
+                                                           map.getMaxZoom(), style, glyphAtlas,
+                                                           glyphStore, spriteAtlas, sprite, info);
             new_tile.data->reparse(worker, callback);
         } else {
             throw std::runtime_error("source type not implemented");
@@ -293,7 +289,6 @@ bool Source::findLoadedParent(const Tile::ID& id, int32_t minCoveringZoom, std::
 }
 
 void Source::update(Map &map,
-                    Environment &env,
                     uv::worker &worker,
                     util::ptr<Style> style,
                     GlyphAtlas &glyphAtlas,
@@ -322,7 +317,7 @@ void Source::update(Map &map,
 
     // Add existing child/parent tiles if the actual tile is not yet loaded
     for (const Tile::ID& id : required) {
-        const TileData::State state = addTile(map, env, worker, style, glyphAtlas, glyphStore,
+        const TileData::State state = addTile(map, worker, style, glyphAtlas, glyphStore,
                                               spriteAtlas, sprite, texturePool, id, callback);
 
         if (state != TileData::State::parsed) {
diff --git a/src/mbgl/map/source.hpp b/src/mbgl/map/source.hpp
@@ -35,7 +35,7 @@ class Source : public std::enable_shared_from_this<Source>, private util::noncop
     Source(SourceInfo&);
 
     void load(Map &, Environment &);
-    void update(Map &, Environment &, uv::worker &, util::ptr<Style>, GlyphAtlas &, GlyphStore &,
+    void update(Map &, uv::worker &, util::ptr<Style>, GlyphAtlas &, GlyphStore &,
                 SpriteAtlas &, util::ptr<Sprite>, TexturePool &, std::function<void()> callback);
     void invalidateTiles(Map&, std::vector<Tile::ID>&);
 
@@ -56,7 +56,7 @@ class Source : public std::enable_shared_from_this<Source>, private util::noncop
     int32_t coveringZoomLevel(const TransformState&) const;
     std::forward_list<Tile::ID> coveringTiles(const TransformState&) const;
 
-    TileData::State addTile(Map &, Environment &, uv::worker &, util::ptr<Style>, GlyphAtlas &,
+    TileData::State addTile(Map &, uv::worker &, util::ptr<Style>, GlyphAtlas &,
                             GlyphStore &, SpriteAtlas &, util::ptr<Sprite>, TexturePool &,
                             const Tile::ID &, std::function<void()> callback);
 
diff --git a/src/mbgl/map/tile_data.cpp b/src/mbgl/map/tile_data.cpp
@@ -12,12 +12,12 @@
 
 using namespace mbgl;
 
-TileData::TileData(Tile::ID const& id_, const SourceInfo& source_, Environment& env_)
+TileData::TileData(Tile::ID const& id_, const SourceInfo& source_)
     : id(id_),
       name(id),
       state(State::initial),
       source(source_),
-      env(env_),
+      env(Environment::Get()),
       debugBucket(debugFontBuffer) {
     // Initialize tile debug coordinates
     debugFontBuffer.addText(name.c_str(), 50, 200, 5);
@@ -96,7 +96,8 @@ void TileData::reparse(uv::worker& worker, std::function<void()> callback)
     // the after work handler
     new uv::work<util::ptr<TileData>>(
         worker,
-        [](util::ptr<TileData>& tile) {
+        [this](util::ptr<TileData>& tile) {
+            Environment::Scope scope(env, ThreadType::TileWorker, "TileWorker_" + tile->name);
             tile->parse();
         },
         [callback](util::ptr<TileData>&) {
diff --git a/src/mbgl/map/tile_data.hpp b/src/mbgl/map/tile_data.hpp
@@ -48,7 +48,7 @@ class TileData : public std::enable_shared_from_this<TileData>,
     };
 
 public:
-    TileData(Tile::ID const &id, const SourceInfo &, Environment &);
+    TileData(Tile::ID const &id, const SourceInfo &);
     ~TileData();
 
     void request(uv::worker&, float pixelRatio, std::function<void ()> callback);
@@ -72,7 +72,7 @@ class TileData : public std::enable_shared_from_this<TileData>,
 
 public:
     const SourceInfo& source;
-    Environment &env;
+    Environment& env;
 
 protected:
     Request *req = nullptr;
diff --git a/src/mbgl/map/vector_tile_data.cpp b/src/mbgl/map/vector_tile_data.cpp
@@ -17,9 +17,8 @@ VectorTileData::VectorTileData(Tile::ID const& id_,
                                GlyphStore& glyphStore_,
                                SpriteAtlas& spriteAtlas_,
                                util::ptr<Sprite> sprite_,
-                               const SourceInfo& source_,
-                               Environment& env_)
-    : TileData(id_, source_, env_),
+                               const SourceInfo& source_)
+    : TileData(id_, source_),
       glyphAtlas(glyphAtlas_),
       glyphStore(glyphStore_),
       spriteAtlas(spriteAtlas_),
diff --git a/src/mbgl/map/vector_tile_data.hpp b/src/mbgl/map/vector_tile_data.hpp
@@ -37,8 +37,7 @@ class VectorTileData : public TileData {
                    GlyphStore&,
                    SpriteAtlas&,
                    util::ptr<Sprite>,
-                   const SourceInfo&,
-                   Environment&);
+                   const SourceInfo&);
     ~VectorTileData();
 
     void parse() override;

Original file line number	Diff line number	Diff line change
`@@ -35,7 +35,7 @@ class ThreadInfoStore {`
`35`	`35`	`// FIXME: We should never need to overwrite a thread here and we only allow`
`36`	`36`	`// this today because on the Static mode, the Map thread and the Main thread`
`37`	`37`	`// are same. Replace this with emplace() when this gets fixed.`
`38`		`- threadSet[std::this_thread::get_id()] = ThreadInfo{env, type, name};`
	`38`	`+ threadSet[std::this_thread::get_id()] = ThreadInfo{ env, type, name };`
`39`	`39`	`}`
`40`	`40`
`41`	`41`	`void unregisterThread() {`
`@@ -80,7 +80,7 @@ Environment::Scope::~Scope() {`
`80`	`80`	`threadInfoStore.unregisterThread();`
`81`	`81`	`}`
`82`	`82`
`83`		`-Environment::Environment(FileSource &fs) : fileSource(fs), loop(uv_loop_new()) {`
	`83`	`+Environment::Environment(FileSource& fs) : fileSource(fs), loop(uv_loop_new()) {`
`84`	`84`	`}`
`85`	`85`
`86`	`86`	`Environment& Environment::Get() {`
`@@ -102,16 +102,18 @@ std::string Environment::threadName() {`
`102`	`102`	`return threadInfoStore.getThreadInfo().name;`
`103`	`103`	`}`
`104`	`104`
`105`		`-void Environment::requestAsync(const Resource &resource, std::function<void(const Response &)> callback) {`
	`105`	`+void Environment::requestAsync(const Resource& resource,`
	`106`	`+ std::function<void(const Response&)> callback) {`
`106`	`107`	`fileSource.request(resource, *this, std::move(callback));`
`107`	`108`	`}`
`108`	`109`
`109`		`-Request *Environment::request(const Resource &resource, std::function<void(const Response &)> callback) {`
	`110`	`+Request* Environment::request(const Resource& resource,`
	`111`	`+ std::function<void(const Response&)> callback) {`
`110`	`112`	`assert(currentlyOn(ThreadType::Map));`
`111`	`113`	`return fileSource.request(resource, loop, *this, std::move(callback));`
`112`	`114`	`}`
`113`	`115`
`114`		`-void Environment::cancelRequest(Request *req) {`
	`116`	`+void Environment::cancelRequest(Request* req) {`
`115`	`117`	`assert(currentlyOn(ThreadType::Map));`
`116`	`118`	`fileSource.cancel(req);`
`117`	`119`	`}`
Original file line number	Diff line number	Diff line change
`@@ -5,8 +5,8 @@`
`5`	`5`	`using namespace mbgl;`
`6`	`6`
`7`	`7`	`RasterTileData::RasterTileData(Tile::ID const &id_, TexturePool &texturePool,`
`8`		`- const SourceInfo &source_, Environment &env_)`
`9`		`- : TileData(id_, source_, env_), bucket(texturePool, layout) {`
	`8`	`+ const SourceInfo &source_)`
	`9`	`+ : TileData(id_, source_), bucket(texturePool, layout) {`
`10`	`10`	`}`
`11`	`11`
`12`	`12`	`RasterTileData::~RasterTileData() {`