From 1499475d04811d52a7fbfbe53b6b042093ea5f17 Mon Sep 17 00:00:00 2001 From: Mickey Rose Date: Mon, 15 Feb 2016 21:07:01 +0100 Subject: [PATCH 1/6] remove to_utf8, instead use UnicodeString::toUTF8String --- include/mapnik/value.hpp | 42 +++++++--------------------------------- src/expression_node.cpp | 21 ++++++++++---------- 2 files changed, 18 insertions(+), 45 deletions(-) diff --git a/include/mapnik/value.hpp b/include/mapnik/value.hpp index 1192442b7..280b2f5b3 100644 --- a/include/mapnik/value.hpp +++ b/include/mapnik/value.hpp @@ -43,30 +43,7 @@ #include #include -namespace mapnik { - -inline void to_utf8(mapnik::value_unicode_string const& input, std::string & target) -{ - if (input.isEmpty()) return; - - const int BUF_SIZE = 256; - char buf [BUF_SIZE]; - int len; - - UErrorCode err = U_ZERO_ERROR; - u_strToUTF8(buf, BUF_SIZE, &len, input.getBuffer(), input.length(), &err); - if (err == U_BUFFER_OVERFLOW_ERROR || err == U_STRING_NOT_TERMINATED_WARNING ) - { - const std::unique_ptr buf_ptr(new char [len+1]); - err = U_ZERO_ERROR; - u_strToUTF8(buf_ptr.get() , len + 1, &len, input.getBuffer(), input.length(), &err); - target.assign(buf_ptr.get() , static_cast(len)); - } - else - { - target.assign(buf, static_cast(len)); - } -} +namespace mapnik { using value_base = util::variant; @@ -582,7 +559,7 @@ struct convert value_double operator() (value_unicode_string const& val) const { std::string utf8; - to_utf8(val,utf8); + val.toUTF8String(utf8); return operator()(utf8); } @@ -621,7 +598,7 @@ struct convert value_integer operator() (value_unicode_string const& val) const { std::string utf8; - to_utf8(val,utf8); + val.toUTF8String(utf8); return operator()(utf8); } @@ -646,7 +623,7 @@ struct convert std::string operator() (value_unicode_string const& val) const { std::string utf8; - to_utf8(val,utf8); + val.toUTF8String(utf8); return utf8; } @@ -664,7 +641,7 @@ struct convert std::string operator() (value_null const&) const { - return ""; + return std::string(); } }; @@ -694,17 +671,12 @@ struct to_unicode_impl value_unicode_string operator() (value_bool val) const { - if (val) { - std::string str("true"); - return value_unicode_string(str.c_str()); - } - std::string str("false"); - return value_unicode_string(str.c_str()); + return value_unicode_string(val ? "true" : "false"); } value_unicode_string operator() (value_null const&) const { - return value_unicode_string(""); + return value_unicode_string(); } }; diff --git a/src/expression_node.cpp b/src/expression_node.cpp index 377d091c9..380c36a4e 100644 --- a/src/expression_node.cpp +++ b/src/expression_node.cpp @@ -37,6 +37,14 @@ namespace mapnik { +#if defined(BOOST_REGEX_HAS_ICU) +static void fromUTF32toUTF8(std::basic_string const& src, std::string & dst) +{ + int32_t len = safe_cast(src.length()); + value_unicode_string::fromUTF32(src.data(), len).toUTF8String(dst); +} +#endif + struct _regex_match_impl : util::noncopyable { #if defined(BOOST_REGEX_HAS_ICU) _regex_match_impl(value_unicode_string const& ustr) : @@ -94,10 +102,7 @@ std::string regex_match_node::to_string() const str_ +=".match('"; auto const& pattern = impl_.get()->pattern_; #if defined(BOOST_REGEX_HAS_ICU) - std::string utf8; - value_unicode_string ustr = value_unicode_string::fromUTF32( &pattern.str()[0], safe_cast(pattern.str().length())); - to_utf8(ustr,utf8); - str_ += utf8; + fromUTF32toUTF8(pattern.str(), str_); #else str_ += pattern.str(); #endif @@ -141,13 +146,9 @@ std::string regex_replace_node::to_string() const auto const& pattern = impl_.get()->pattern_; auto const& format = impl_.get()->format_; #if defined(BOOST_REGEX_HAS_ICU) - std::string utf8; - value_unicode_string ustr = value_unicode_string::fromUTF32( &pattern.str()[0], safe_cast(pattern.str().length())); - to_utf8(ustr,utf8); - str_ += utf8; + fromUTF32toUTF8(pattern.str(), str_); str_ +="','"; - to_utf8(format ,utf8); - str_ += utf8; + format.toUTF8String(str_); #else str_ += pattern.str(); str_ +="','"; From 90fcdc90d3559d8df98f5b7586dfef0ca45abf61 Mon Sep 17 00:00:00 2001 From: Mickey Rose Date: Tue, 16 Feb 2016 16:27:32 +0100 Subject: [PATCH 2/6] benchmarks: fix output formatting snprintf is a better tool for this task, anyway, but here's what was wrong with the stream formatting: s << name << ":" << std::setw(45 - (int)s.tellp()) cannot be used to align columns, tellp() may return any of: 0 name.length() name.length() + strlen(":") because the compiler is allowed to reorder the evaluation of sub-expressions as it likes -- it may evaluate s.tellp() before evaluating (s << name) or in-between (s << name) and (s << ":") try with gcc-4.8 --- benchmark/bench_framework.hpp | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/benchmark/bench_framework.hpp b/benchmark/bench_framework.hpp index 52088e4e1..366f82706 100644 --- a/benchmark/bench_framework.hpp +++ b/benchmark/bench_framework.hpp @@ -9,7 +9,7 @@ // stl #include -#include +#include // snprintf #include #include #include @@ -96,11 +96,7 @@ int run(T const& test_runner, std::string const& name) { std::chrono::high_resolution_clock::time_point start; std::chrono::high_resolution_clock::duration elapsed; - std::stringstream s; - s << name << ":" - << std::setw(45 - (int)s.tellp()) << std::right - << " t:" << test_runner.threads() - << " i:" << test_runner.iterations(); + if (test_runner.threads() > 0) { using thread_group = std::vector >; @@ -120,9 +116,15 @@ int run(T const& test_runner, std::string const& name) test_runner(); elapsed = std::chrono::high_resolution_clock::now() - start; } - s << std::setw(65 - (int)s.tellp()) << std::right - << std::chrono::duration_cast(elapsed).count() << " milliseconds\n"; - std::clog << s.str(); + + char msg[200]; + std::snprintf(msg, sizeof(msg), + "%-43s %3zu threads %9zu iters %6.0f milliseconds", + name.c_str(), + test_runner.threads(), + test_runner.iterations(), + std::chrono::duration(elapsed).count()); + std::clog << msg << "\n"; } return 0; } From ece5b93f088f5a7fd652853b7b15cdde236fea21 Mon Sep 17 00:00:00 2001 From: Mickey Rose Date: Tue, 16 Feb 2016 18:37:24 +0100 Subject: [PATCH 3/6] benchmarks: add option --log-severity this option is applied immediately after parsing args to override MAPNIK_DEFAUL_LOG_SEVERITY --- benchmark/bench_framework.hpp | 74 +++++++++++++++++++++++++++++------ 1 file changed, 62 insertions(+), 12 deletions(-) diff --git a/benchmark/bench_framework.hpp b/benchmark/bench_framework.hpp index 366f82706..8eb01ce85 100644 --- a/benchmark/bench_framework.hpp +++ b/benchmark/bench_framework.hpp @@ -2,6 +2,7 @@ #define __MAPNIK_BENCH_FRAMEWORK_HPP__ // mapnik +#include #include #include #include @@ -43,21 +44,70 @@ public: virtual ~test_case() {} }; -void handle_args(int argc, char** argv, mapnik::parameters & params) +// gathers --long-option values in 'params'; +// returns the index of the first non-option argument, +// or negated index of an ill-formed option argument +inline int parse_args(int argc, char** argv, mapnik::parameters & params) { - if (argc > 0) { - for (int i=1;i("log-severity")) { + if (*severity == "debug") + mapnik::logger::set_severity(mapnik::logger::debug); + else if (*severity == "warn") + mapnik::logger::set_severity(mapnik::logger::warn); + else if (*severity == "error") + mapnik::logger::set_severity(mapnik::logger::error); + else if (*severity == "none") + mapnik::logger::set_severity(mapnik::logger::none); + else + std::clog << "ignoring option --log-severity='" << *severity + << "' (allowed values are: debug, warn, error, none)\n"; + } +} + +inline int handle_args(int argc, char** argv, mapnik::parameters & params) +{ + int res = parse_args(argc, argv, params); + handle_common_args(params); + return res; } #define BENCHMARK(test_class,name) \ From 29c4f730ebb45ac2ba9130c4c1a0f85bba3cf294 Mon Sep 17 00:00:00 2001 From: Mickey Rose Date: Tue, 16 Feb 2016 19:38:09 +0100 Subject: [PATCH 4/6] benchmarks: fix errors in non-compiling sources --- benchmark/test_array_allocation.cpp | 27 --------------------------- benchmark/test_png_encoding1.cpp | 2 +- benchmark/test_png_encoding2.cpp | 2 +- 3 files changed, 2 insertions(+), 29 deletions(-) diff --git a/benchmark/test_array_allocation.cpp b/benchmark/test_array_allocation.cpp index fda9bb6ab..e11a68fd8 100644 --- a/benchmark/test_array_allocation.cpp +++ b/benchmark/test_array_allocation.cpp @@ -231,33 +231,6 @@ public: } }; -class test3d : public benchmark::test_case -{ -public: - uint32_t size_; - std::vector array_; - test3d(mapnik::parameters const& params) - : test_case(params), - size_(*params.get("size",256*256)), - array_(size_,0) { } - bool validate() const - { - return true; - } - bool operator()() const - { - for (std::size_t i=0;i data(size_); - for (std::size_t i=0;i Date: Tue, 16 Feb 2016 19:53:03 +0100 Subject: [PATCH 5/6] benchmarks: helper class for running a sequence of tests --- benchmark/bench_framework.hpp | 28 ++++++++ benchmark/test_array_allocation.cpp | 71 ++++--------------- .../test_numeric_cast_vs_static_cast.cpp | 16 ++--- benchmark/test_polygon_clipping_rendering.cpp | 32 ++------- benchmark/test_proj_transform1.cpp | 38 ++-------- 5 files changed, 59 insertions(+), 126 deletions(-) diff --git a/benchmark/bench_framework.hpp b/benchmark/bench_framework.hpp index 8eb01ce85..c78125149 100644 --- a/benchmark/bench_framework.hpp +++ b/benchmark/bench_framework.hpp @@ -186,6 +186,34 @@ int run(T const& test_runner, std::string const& name) return 0; } +struct sequencer +{ + sequencer(int argc, char** argv) + : exit_code_(0) + { + benchmark::handle_args(argc, argv, params_); + } + + int done() const + { + return exit_code_; + } + + template + sequencer & run(std::string const& name, Args && ...args) + { + // Test instance lifetime is confined to this function + Test test_runner(params_, std::forward(args)...); + // any failing test run will make exit code non-zero + exit_code_ |= benchmark::run(test_runner, name); + return *this; // allow chaining calls + } + +protected: + mapnik::parameters params_; + int exit_code_; +}; + } #endif // __MAPNIK_BENCH_FRAMEWORK_HPP__ diff --git a/benchmark/test_array_allocation.cpp b/benchmark/test_array_allocation.cpp index e11a68fd8..186c1620e 100644 --- a/benchmark/test_array_allocation.cpp +++ b/benchmark/test_array_allocation.cpp @@ -359,62 +359,21 @@ public: int main(int argc, char** argv) { - int return_value = 0; - mapnik::parameters params; - benchmark::handle_args(argc,argv,params); - { - test4 test_runner4(params); - return_value = return_value | run(test_runner4,"calloc"); - } - { - test1 test_runner(params); - return_value = return_value | run(test_runner,"malloc/memcpy"); - } - { - test1b test_runner(params); - return_value = return_value | run(test_runner,"malloc/memset"); - } - { - test1c test_runner(params); - return_value = return_value | run(test_runner,"operator new/std::fill"); - } - { - test2 test_runner(params); - return_value = return_value | run(test_runner,"operator new/memcpy"); - } - { - test3 test_runner(params); - return_value = return_value | run(test_runner,"vector(N)"); - } - { - test3b test_runner(params); - return_value = return_value | run(test_runner,"vector/resize"); - } - { - test3c test_runner(params); - return_value = return_value | run(test_runner,"vector/assign"); - } - { - test3d test_runner(params); - return_value = return_value | run(test_runner,"deque(N)"); - } - { - test5 test_runner(params); - return_value = return_value | run(test_runner,"std::string range"); - } - { - test5b test_runner(params); - return_value = return_value | run(test_runner,"std::string &[0]"); - } - { - test6 test_runner(params); - return_value = return_value | run(test_runner,"valarray"); - } + return benchmark::sequencer(argc, argv) + .run("calloc") + .run("malloc/memcpy") + .run("malloc/memset") + .run("operator new/std::fill") + .run("operator new/memcpy") + .run("vector(N)") + .run("vector/resize") + .run("vector/assign") + .run("deque(N)") + .run("std::string range") + .run("std::string &[0]") + .run("valarray") #if BOOST_VERSION >= 105400 - { - test7 test_runner(params); - return_value = return_value | run(test_runner,"static_vector"); - } + .run("static_vector") #endif - return return_value; + .done(); } diff --git a/benchmark/test_numeric_cast_vs_static_cast.cpp b/benchmark/test_numeric_cast_vs_static_cast.cpp index 8ea6e6be1..9e14bf31e 100644 --- a/benchmark/test_numeric_cast_vs_static_cast.cpp +++ b/benchmark/test_numeric_cast_vs_static_cast.cpp @@ -73,16 +73,8 @@ public: int main(int argc, char** argv) { - mapnik::parameters params; - benchmark::handle_args(argc,argv,params); - int return_value = 0; - { - test_static test_runner(params); - return_value = return_value | run(test_runner,"static_cast"); - } - { - test_numeric test_runner(params); - return_value = return_value | run(test_runner,"numeric_cast"); - } - return return_value; + return benchmark::sequencer(argc, argv) + .run("static_cast") + .run("numeric_cast") + .done(); } diff --git a/benchmark/test_polygon_clipping_rendering.cpp b/benchmark/test_polygon_clipping_rendering.cpp index ab23459c4..758505d9d 100644 --- a/benchmark/test_polygon_clipping_rendering.cpp +++ b/benchmark/test_polygon_clipping_rendering.cpp @@ -51,30 +51,10 @@ int main(int argc, char** argv) mapnik::box2d z1(-20037508.3428,-8317435.0606,20037508.3428,18399242.7298); // bbox for 16/10491/22911.png mapnik::box2d z16(-13622912.929097254,6026906.8062295765,-13621689.93664469,6028129.79868214); - int return_value = 0; - { - test test_runner(params, - "benchmark/data/polygon_rendering_clip.xml", - z1); - return_value = return_value | run(test_runner,"polygon clip render z1"); - } - { - test test_runner(params, - "benchmark/data/polygon_rendering_no_clip.xml", - z1); - return_value = return_value | run(test_runner,"polygon noclip render z1"); - } - { - test test_runner(params, - "benchmark/data/polygon_rendering_clip.xml", - z16); - return_value = return_value | run(test_runner,"polygon clip render z16"); - } - { - test test_runner(params, - "benchmark/data/polygon_rendering_no_clip.xml", - z16); - return_value = return_value | run(test_runner,"polygon noclip render z16"); - } - return return_value; + return benchmark::sequencer(argc, argv) + .run("polygon clip render z1", "benchmark/data/polygon_rendering_clip.xml", z1) + .run("polygon noclip render z1", "benchmark/data/polygon_rendering_no_clip.xml", z1) + .run("polygon clip render z16", "benchmark/data/polygon_rendering_clip.xml", z16) + .run("polygon noclip render z16", "benchmark/data/polygon_rendering_no_clip.xml", z16) + .done(); } diff --git a/benchmark/test_proj_transform1.cpp b/benchmark/test_proj_transform1.cpp index 451c51ae1..37f4b0c4c 100644 --- a/benchmark/test_proj_transform1.cpp +++ b/benchmark/test_proj_transform1.cpp @@ -59,42 +59,16 @@ public: // echo -180 -60 | cs2cs -f "%.10f" +init=epsg:4326 +to +init=epsg:3857 int main(int argc, char** argv) { - mapnik::parameters params; - benchmark::handle_args(argc,argv,params); mapnik::box2d from(-180,-80,180,80); mapnik::box2d to(-20037508.3427892476,-15538711.0963092316,20037508.3427892476,15538711.0963092316); std::string from_str("+init=epsg:4326"); std::string to_str("+init=epsg:3857"); std::string from_str2("+proj=longlat +ellps=WGS84 +datum=WGS84 +no_defs"); std::string to_str2("+proj=merc +a=6378137 +b=6378137 +lat_ts=0.0 +lon_0=0.0 +x_0=0.0 +y_0=0.0 +k=1.0 +units=m +nadgrids=@null +wktext +no_defs +over"); - int return_value = 0; - test test_runner(params, - from_str, - to_str, - from, - to, - true); - return_value = return_value | run(test_runner,"lonlat->merc epsg"); - test test_runner2(params, - from_str2, - to_str2, - from, - to, - true); - return_value = return_value | run(test_runner2,"lonlat->merc literal"); - test test_runner3(params, - to_str, - from_str, - to, - from, - true); - return_value = return_value | run(test_runner3,"merc->lonlat epsg"); - test test_runner4(params, - to_str2, - from_str2, - to, - from, - true); - return_value = return_value | run(test_runner4,"merc->lonlat literal"); - return return_value; + return benchmark::sequencer(argc, argv) + .run("lonlat->merc epsg", from_str, to_str, from, to, true) + .run("lonlat->merc literal", from_str2, to_str2, from, to, true) + .run("merc->lonlat epsg", to_str, from_str, to, from, true) + .run("merc->lonlat literal", to_str2, from_str2, to, from, true) + .done(); } From 8762e1d034fd8a71432a290c74d1bc5b4eb8a066 Mon Sep 17 00:00:00 2001 From: Mickey Rose Date: Wed, 17 Feb 2016 01:30:32 +0100 Subject: [PATCH 6/6] benchmarks: add option --min-duration=SECONDS - with this option test_runner() will be called repeatedly (doing the given number of --iterations each time) until SECONDS have elapsed - added output column showing average time per iteration, should help when comparing different runs - shrinked the # of iterations column using k/M suffixes --- benchmark/bench_framework.hpp | 94 ++++++++++++++++++++++++----------- 1 file changed, 64 insertions(+), 30 deletions(-) diff --git a/benchmark/bench_framework.hpp b/benchmark/bench_framework.hpp index c78125149..34c63a8e7 100644 --- a/benchmark/bench_framework.hpp +++ b/benchmark/bench_framework.hpp @@ -39,6 +39,10 @@ public: { return iterations_; } + mapnik::parameters const& params() const + { + return params_; + } virtual bool validate() const = 0; virtual bool operator()() const = 0; virtual ~test_case() {} @@ -138,52 +142,82 @@ int run(T const& test_runner, std::string const& name) if (!test_runner.validate()) { std::clog << "test did not validate: " << name << "\n"; - return -1; + return 1; } // run test once before timing // if it returns false then we'll abort timing - if (test_runner()) + if (!test_runner()) { - std::chrono::high_resolution_clock::time_point start; - std::chrono::high_resolution_clock::duration elapsed; + return 2; + } - if (test_runner.threads() > 0) + std::chrono::high_resolution_clock::time_point start; + std::chrono::high_resolution_clock::duration elapsed; + auto opt_min_duration = test_runner.params().template get("min-duration", 0.0); + std::chrono::duration min_seconds(*opt_min_duration); + auto min_duration = std::chrono::duration_cast(min_seconds); + std::size_t loops = 0; + + if (test_runner.threads() > 0) + { + using thread_group = std::vector >; + using value_type = thread_group::value_type; + thread_group tg; + for (std::size_t i=0;i >; - using value_type = thread_group::value_type; - thread_group tg; - for (std::size_t i=0;ijoinable()) t->join();}); - elapsed = std::chrono::high_resolution_clock::now() - start; + tg.emplace_back(new std::thread(test_runner)); } - else - { - start = std::chrono::high_resolution_clock::now(); + start = std::chrono::high_resolution_clock::now(); + std::for_each(tg.begin(), tg.end(), [](value_type & t) {if (t->joinable()) t->join();}); + elapsed = std::chrono::high_resolution_clock::now() - start; + loops = 1; + } + else + { + start = std::chrono::high_resolution_clock::now(); + do { test_runner(); elapsed = std::chrono::high_resolution_clock::now() - start; - } - - char msg[200]; - std::snprintf(msg, sizeof(msg), - "%-43s %3zu threads %9zu iters %6.0f milliseconds", - name.c_str(), - test_runner.threads(), - test_runner.iterations(), - std::chrono::duration(elapsed).count()); - std::clog << msg << "\n"; + ++loops; + } while (elapsed < min_duration); } + + double iters = loops * test_runner.iterations(); + double dur_total = std::chrono::duration(elapsed).count(); + double dur_avg = dur_total / iters; + char iters_unit = ' '; + char msg[200]; + + if (iters >= 1e7) iters *= 1e-6, iters_unit = 'M'; + else if (iters >= 1e4) iters *= 1e-3, iters_unit = 'k'; + + std::snprintf(msg, sizeof(msg), + "%-43s %3zu threads %4.0f%c iters %6.0f milliseconds", + name.c_str(), + test_runner.threads(), + iters, iters_unit, + dur_total); + std::clog << msg; + + // log average time per iteration, currently only for non-threaded runs + if (test_runner.threads() == 0) + { + char unit = 'm'; + if (dur_avg < 1e-5) dur_avg *= 1e+9, unit = 'p'; + else if (dur_avg < 1e-2) dur_avg *= 1e+6, unit = 'n'; + else if (dur_avg < 1e+1) dur_avg *= 1e+3, unit = 'u'; + std::snprintf(msg, sizeof(msg), " %4.0f%cs/iter", dur_avg, unit); + std::clog << msg; + } + + std::clog << "\n"; return 0; } catch (std::exception const& ex) { std::clog << "test runner did not complete: " << ex.what() << "\n"; - return -1; + return 4; } - return 0; } struct sequencer