From 0a5495e4423be247323ef435e8545602b4e6338e Mon Sep 17 00:00:00 2001 From: Mickey Rose Date: Tue, 3 Jan 2017 20:53:55 +0100 Subject: [PATCH 1/2] change render_thunk_list to std::list Wrapping render_thunk in std::unique_ptr is one extra allocation per element, with no purpose. The somewhat costly xyz_render_thunk move constructor is only called once upon insertion, regardless of whether we're emplacing render_thunk or unique_ptr. --- include/mapnik/renderer_common/render_group_symbolizer.hpp | 4 ++-- include/mapnik/renderer_common/render_thunk.hpp | 3 +-- src/renderer_common/render_thunk_extractor.cpp | 6 +++--- 3 files changed, 6 insertions(+), 7 deletions(-) diff --git a/include/mapnik/renderer_common/render_group_symbolizer.hpp b/include/mapnik/renderer_common/render_group_symbolizer.hpp index e7b1b2248..65cc0f94f 100644 --- a/include/mapnik/renderer_common/render_group_symbolizer.hpp +++ b/include/mapnik/renderer_common/render_group_symbolizer.hpp @@ -42,9 +42,9 @@ struct render_thunk_list_dispatch { offset_ = offset; - for (render_thunk_ptr const& thunk : thunks) + for (render_thunk const& thunk : thunks) { - util::apply_visitor(std::ref(*this), *thunk); + util::apply_visitor(std::ref(*this), thunk); } } diff --git a/include/mapnik/renderer_common/render_thunk.hpp b/include/mapnik/renderer_common/render_thunk.hpp index 2a957d260..66b108bb9 100644 --- a/include/mapnik/renderer_common/render_thunk.hpp +++ b/include/mapnik/renderer_common/render_thunk.hpp @@ -107,8 +107,7 @@ struct text_render_thunk : util::movable using render_thunk = util::variant; -using render_thunk_ptr = std::unique_ptr; -using render_thunk_list = std::list; +using render_thunk_list = std::list; } // namespace mapnik diff --git a/src/renderer_common/render_thunk_extractor.cpp b/src/renderer_common/render_thunk_extractor.cpp index c437262da..1a5d1b854 100644 --- a/src/renderer_common/render_thunk_extractor.cpp +++ b/src/renderer_common/render_thunk_extractor.cpp @@ -55,7 +55,7 @@ struct thunk_markers_renderer_context : markers_renderer_context { vector_marker_render_thunk thunk(src, attrs, marker_tr, params.opacity, comp_op_, params.snap_to_pixels); - thunks_.push_back(std::make_unique(std::move(thunk))); + thunks_.emplace_back(std::move(thunk)); } virtual void render_marker(image_rgba8 const& src, @@ -64,7 +64,7 @@ struct thunk_markers_renderer_context : markers_renderer_context { raster_marker_render_thunk thunk(src, marker_tr, params.opacity, comp_op_, params.snap_to_pixels); - thunks_.push_back(std::make_unique(std::move(thunk))); + thunks_.emplace_back(std::move(thunk)); } private: @@ -128,7 +128,7 @@ void render_thunk_extractor::extract_text_thunk(text_render_thunk::helper_ptr && halo_rasterizer_enum halo_rasterizer = get(sym, keys::halo_rasterizer, feature_, common_.vars_, HALO_RASTERIZER_FULL); text_render_thunk thunk(std::move(helper), opacity, comp_op, halo_rasterizer); - thunks_.push_back(std::make_unique(std::move(thunk))); + thunks_.emplace_back(std::move(thunk)); update_box(); } From 63128fdba116c1fa144f9e8b292bcbd69cb04da0 Mon Sep 17 00:00:00 2001 From: Mickey Rose Date: Tue, 3 Jan 2017 21:43:14 +0100 Subject: [PATCH 2/2] can't store noncopyable list in std::vector std::list can have a throwing move constructor. std::vector of such lists makes copies when growing its storage array, it doesn't move them. render_thunk_list is noncopyable (because render_thunk is noncopyable), and so can't be stored in std::vector in some STL implementations. --- src/renderer_common/render_group_symbolizer.cpp | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/renderer_common/render_group_symbolizer.cpp b/src/renderer_common/render_group_symbolizer.cpp index 9892dfc8d..8e3aba9bd 100644 --- a/src/renderer_common/render_group_symbolizer.cpp +++ b/src/renderer_common/render_group_symbolizer.cpp @@ -66,7 +66,7 @@ void render_group_symbolizer(group_symbolizer const& sym, // keep track of which lists of render thunks correspond to // entries in the group_layout_manager. - std::vector layout_thunks; + std::list layout_thunks; // layout manager to store and arrange bboxes of matched features group_layout_manager layout_manager(props->get_layout()); @@ -182,11 +182,13 @@ void render_group_symbolizer(group_symbolizer const& sym, pixel_position_list const& positions = helper.get(); for (pixel_position const& pos : positions) { - for (size_t layout_i = 0; layout_i < layout_thunks.size(); ++layout_i) + size_t layout_i = 0; + for (auto const& thunks : layout_thunks) { pixel_position const& offset = layout_manager.offset_at(layout_i); pixel_position render_offset = pos + offset; - render_thunks.render_list(layout_thunks[layout_i], render_offset); + render_thunks.render_list(thunks, render_offset); + ++layout_i; } } }