From da678406b1a07e105a5d720ae56245e0a3e72396 Mon Sep 17 00:00:00 2001 From: Jiri Drbalek Date: Thu, 1 Oct 2015 15:14:47 +0000 Subject: [PATCH 1/7] use move semantics instead of shared_ptr --- .../mapnik/feature_style_processor_impl.hpp | 21 +++++++++---------- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/include/mapnik/feature_style_processor_impl.hpp b/include/mapnik/feature_style_processor_impl.hpp index 0c7782198..42bcaa9bc 100644 --- a/include/mapnik/feature_style_processor_impl.hpp +++ b/include/mapnik/feature_style_processor_impl.hpp @@ -69,11 +69,10 @@ struct layer_rendering_material lay_(lay), proj0_(dest), proj1_(lay.srs(),true) {} + + layer_rendering_material(layer_rendering_material && rhs) = default; }; -using layer_rendering_material_ptr = std::shared_ptr; - - template feature_style_processor::feature_style_processor(Map const& m, double scale_factor) : m_(m) @@ -102,7 +101,7 @@ void feature_style_processor::apply(double scale_denom) // in a second time, we fetch the results and // do the actual rendering - std::vector mat_list; + std::vector mat_list; // Define processing context map used by datasources // implementing asynchronous queries @@ -113,9 +112,9 @@ void feature_style_processor::apply(double scale_denom) if (lyr.visible(scale_denom)) { std::set names; - layer_rendering_material_ptr mat = std::make_shared(lyr, proj); + layer_rendering_material mat(lyr, proj); - prepare_layer(*mat, + prepare_layer(mat, ctx_map, p, m_.scale(), @@ -127,18 +126,18 @@ void feature_style_processor::apply(double scale_denom) names); // Store active material - if (!mat->active_styles_.empty()) + if (!mat.active_styles_.empty()) { - mat_list.push_back(mat); + mat_list.emplace_back(std::move(mat)); } } } - for ( layer_rendering_material_ptr mat : mat_list ) + for ( layer_rendering_material & mat : mat_list ) { - if (!mat->active_styles_.empty()) + if (!mat.active_styles_.empty()) { - render_material(*mat,p); + render_material(mat, p); } } From 5c0aa52d22f5e159970edca4d5b16912fc801e4b Mon Sep 17 00:00:00 2001 From: Jiri Drbalek Date: Thu, 1 Oct 2015 19:47:58 +0000 Subject: [PATCH 2/7] apply constness --- include/mapnik/feature_style_processor.hpp | 2 +- include/mapnik/feature_style_processor_impl.hpp | 12 ++++++------ 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/include/mapnik/feature_style_processor.hpp b/include/mapnik/feature_style_processor.hpp index 062c9d666..c0ba4f383 100644 --- a/include/mapnik/feature_style_processor.hpp +++ b/include/mapnik/feature_style_processor.hpp @@ -110,7 +110,7 @@ private: /*! * \brief render features list queued when they are available. */ - void render_material(layer_rendering_material & mat, Processor & p ); + void render_material(layer_rendering_material const & mat, Processor & p ); Map const& m_; }; diff --git a/include/mapnik/feature_style_processor_impl.hpp b/include/mapnik/feature_style_processor_impl.hpp index 42bcaa9bc..89e0c3a6d 100644 --- a/include/mapnik/feature_style_processor_impl.hpp +++ b/include/mapnik/feature_style_processor_impl.hpp @@ -133,7 +133,7 @@ void feature_style_processor::apply(double scale_denom) } } - for ( layer_rendering_material & mat : mat_list ) + for ( layer_rendering_material const & mat : mat_list ) { if (!mat.active_styles_.empty()) { @@ -442,11 +442,11 @@ void feature_style_processor::prepare_layer(layer_rendering_material template -void feature_style_processor::render_material(layer_rendering_material & mat, +void feature_style_processor::render_material(layer_rendering_material const & mat, Processor & p ) { - std::vector & active_styles = mat.active_styles_; - std::vector & featureset_ptr_list = mat.featureset_ptr_list_; + std::vector const & active_styles = mat.active_styles_; + std::vector const & featureset_ptr_list = mat.featureset_ptr_list_; if (featureset_ptr_list.empty()) { // The datasource wasn't queried because of early return @@ -463,7 +463,7 @@ void feature_style_processor::render_material(layer_rendering_materia layer const& lay = mat.lay_; - std::vector & rule_caches = mat.rule_caches_; + std::vector const & rule_caches = mat.rule_caches_; proj_transform prj_trans(mat.proj0_,mat.proj1_); @@ -543,7 +543,7 @@ void feature_style_processor::render_material(layer_rendering_materia else { std::size_t i = 0; - std::vector::iterator featuresets = featureset_ptr_list.begin(); + std::vector::const_iterator featuresets = featureset_ptr_list.cbegin(); for (feature_type_style const* style : active_styles) { featureset_ptr features = *featuresets++; From 530416f0aa0281f1915227e5ca740819372bae90 Mon Sep 17 00:00:00 2001 From: Dane Springmeyer Date: Thu, 1 Oct 2015 14:47:25 -0700 Subject: [PATCH 3/7] fix suggesed cmd line option syntax [skip ci] --- benchmark/test_rendering.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/benchmark/test_rendering.cpp b/benchmark/test_rendering.cpp index 593307b08..beea28969 100644 --- a/benchmark/test_rendering.cpp +++ b/benchmark/test_rendering.cpp @@ -28,7 +28,7 @@ public: boost::optional map = params.get("map"); if (!map) { - throw std::runtime_error("please provide a --map= arg"); + throw std::runtime_error("please provide a --map arg"); } xml_ = *map; From 3932cc51b37035bee88e118251d7503f6d4f9435 Mon Sep 17 00:00:00 2001 From: Dane Springmeyer Date: Thu, 1 Oct 2015 14:47:55 -0700 Subject: [PATCH 4/7] add getline benchmark - refs #3101 --- benchmark/build.py | 1 + benchmark/test_getline.cpp | 123 ++++++++++++++++++++++++++++++++ plugins/input/csv/csv_utils.hpp | 2 +- 3 files changed, 125 insertions(+), 1 deletion(-) create mode 100644 benchmark/test_getline.cpp diff --git a/benchmark/build.py b/benchmark/build.py index 800a9abb5..37ba86707 100644 --- a/benchmark/build.py +++ b/benchmark/build.py @@ -47,6 +47,7 @@ benchmarks = [ "test_marker_cache.cpp", "test_quad_tree.cpp", "test_noop_rendering.cpp", + "test_getline.cpp", # "test_numeric_cast_vs_static_cast.cpp", ] for cpp_test in benchmarks: diff --git a/benchmark/test_getline.cpp b/benchmark/test_getline.cpp new file mode 100644 index 000000000..c4c609a61 --- /dev/null +++ b/benchmark/test_getline.cpp @@ -0,0 +1,123 @@ +#include "bench_framework.hpp" +#include +#include +#include "../plugins/input/csv/csv_utils.hpp" + +class test : public benchmark::test_case +{ +public: + std::string line_data_; + test(mapnik::parameters const& params) + : test_case(params) + { + boost::optional line_data = params.get("line"); + if (!line_data) + { + throw std::runtime_error("please provide a --line \"one line\ntwo line\""); + } + line_data_ = *line_data; + } + + bool validate() const + { + std::string first = line_data_.substr(line_data_.find_first_not_of('\n')); + char newline = '\n'; + std::string csv_line; + std::stringstream s; + s << line_data_; + std::getline(s,csv_line,s.widen(newline)); + if (csv_line != first) + { + return true; + } + else + { + std::clog << "Error: the parsed line (" << csv_line << ") should be a subset of the original line (" << line_data_ << ") (ensure you pass a line with a \\n)\n"; + } + return true; + } + bool operator()() const + { + char newline = '\n'; + std::string csv_line; + std::stringstream s; + s << line_data_; + for (unsigned i=0;i line_data = params.get("line"); + if (!line_data) + { + throw std::runtime_error("please provide a --line \"one line\ntwo line\""); + } + line_data_ = *line_data; + } + + bool validate() const + { + std::string first = line_data_.substr(line_data_.find_first_not_of('\n')); + char newline = '\n'; + std::string csv_line; + std::stringstream s; + s << line_data_; + csv_utils::getline_csv(s,csv_line,s.widen(newline)); + if (csv_line != first) + { + return true; + } + else + { + std::clog << "Error: the parsed line (" << csv_line << ") should be a subset of the original line (" << line_data_ << ") (ensure you pass a line with a \\n)\n"; + } + return true; + } + bool operator()() const + { + char newline = '\n'; + std::string csv_line; + std::stringstream s; + s << line_data_; + for (unsigned i=0;i extract_geometry(std::vector const& row, geometry_column_locator const& locator) +static inline mapnik::geometry::geometry extract_geometry(std::vector const& row, geometry_column_locator const& locator) { mapnik::geometry::geometry geom; if (locator.type == geometry_column_locator::WKT) From da054c84e6eb986ca20a6e9e52c2dcbb2740f653 Mon Sep 17 00:00:00 2001 From: Dane Springmeyer Date: Thu, 1 Oct 2015 15:08:49 -0700 Subject: [PATCH 5/7] default values for getline bench + hook up in script - refs #3101 --- benchmark/run | 2 +- benchmark/test_getline.cpp | 16 ++++++++-------- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/benchmark/run b/benchmark/run index f91853995..00285b2ba 100755 --- a/benchmark/run +++ b/benchmark/run @@ -9,7 +9,7 @@ function run { ${BASE}/$1 --threads 0 --iterations $3; ${BASE}/$1 --threads $2 --iterations $(expr $3 / $2); } - +run test_getline 30 10000000 #run test_array_allocation 20 100000 #run test_png_encoding1 10 1000 #run test_png_encoding2 10 50 diff --git a/benchmark/test_getline.cpp b/benchmark/test_getline.cpp index c4c609a61..f5709647c 100644 --- a/benchmark/test_getline.cpp +++ b/benchmark/test_getline.cpp @@ -8,14 +8,14 @@ class test : public benchmark::test_case public: std::string line_data_; test(mapnik::parameters const& params) - : test_case(params) + : test_case(params), + line_data_("this is one line\nand this is a second line\nand a third line") { boost::optional line_data = params.get("line"); - if (!line_data) + if (line_data) { - throw std::runtime_error("please provide a --line \"one line\ntwo line\""); + line_data_ = *line_data; } - line_data_ = *line_data; } bool validate() const @@ -56,14 +56,14 @@ class test2 : public benchmark::test_case public: std::string line_data_; test2(mapnik::parameters const& params) - : test_case(params) + : test_case(params), + line_data_("this is one line\nand this is a second line\nand a third line") { boost::optional line_data = params.get("line"); - if (!line_data) + if (line_data) { - throw std::runtime_error("please provide a --line \"one line\ntwo line\""); + line_data_ = *line_data; } - line_data_ = *line_data; } bool validate() const From 336170c13ca7770ad95a9976c43d6291e3483764 Mon Sep 17 00:00:00 2001 From: Dane Springmeyer Date: Thu, 1 Oct 2015 15:34:00 -0700 Subject: [PATCH 6/7] avoid excessive calling of std::ios::widen - refs #3101 --- benchmark/test_getline.cpp | 6 ++++-- plugins/input/csv/csv_datasource.cpp | 7 ++++--- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/benchmark/test_getline.cpp b/benchmark/test_getline.cpp index f5709647c..eac8892eb 100644 --- a/benchmark/test_getline.cpp +++ b/benchmark/test_getline.cpp @@ -42,9 +42,10 @@ public: std::string csv_line; std::stringstream s; s << line_data_; + auto newline_widened = s.widen(newline); for (unsigned i=0;i boxes; - while (is_first_row || csv_utils::getline_csv(stream, csv_line, stream.widen(newline))) + while (is_first_row || csv_utils::getline_csv(stream, csv_line, widened_newline)) { if ((row_limit_ > 0) && (line_number++ > row_limit_)) { From 2f9c9bd4b140e57a8954ddd1e6f1c3a1be020cd8 Mon Sep 17 00:00:00 2001 From: Dane Springmeyer Date: Thu, 1 Oct 2015 15:46:07 -0700 Subject: [PATCH 7/7] update test data - closes #3102 --- test/data | 2 +- test/data-visual | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/test/data b/test/data index 5e7fbab23..76651203a 160000 --- a/test/data +++ b/test/data @@ -1 +1 @@ -Subproject commit 5e7fbab23fcd1874d73b9c7a1558f6b0aacc2cf3 +Subproject commit 76651203a0918a7f85214d0ed561f15741d5e23f diff --git a/test/data-visual b/test/data-visual index 772ce6023..023ffcc01 160000 --- a/test/data-visual +++ b/test/data-visual @@ -1 +1 @@ -Subproject commit 772ce6023ddadbb4ff0d05b43418899980d390cc +Subproject commit 023ffcc01e141eb43f47cee02189b76990792981