From cd39a02d0c61158526c39188707b71f41824485d Mon Sep 17 00:00:00 2001 From: Dane Springmeyer Date: Tue, 28 Jan 2014 12:03:48 -0800 Subject: [PATCH 1/9] unhide the scons command run by make --- Makefile | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Makefile b/Makefile index c4e07e68c..de9564ebb 100755 --- a/Makefile +++ b/Makefile @@ -14,10 +14,10 @@ endif all: mapnik install: - @python scons/scons.py -j$(JOBS) --config=cache --implicit-cache --max-drift=1 install + python scons/scons.py -j$(JOBS) --config=cache --implicit-cache --max-drift=1 install mapnik: - @python scons/scons.py -j$(JOBS) --config=cache --implicit-cache --max-drift=1 + python scons/scons.py -j$(JOBS) --config=cache --implicit-cache --max-drift=1 clean: @python scons/scons.py -j$(JOBS) -c --config=cache --implicit-cache --max-drift=1 From f286363ad0edf4f364559b5413e4e9cbe620c02f Mon Sep 17 00:00:00 2001 From: Dane Springmeyer Date: Tue, 28 Jan 2014 15:05:10 -0800 Subject: [PATCH 2/9] more consistent memset/memcpy usage --- include/mapnik/hextree.hpp | 3 ++- include/mapnik/octree.hpp | 3 ++- plugins/input/rasterlite/rasterlite_featureset.cpp | 5 ++--- plugins/input/shape/dbfile.cpp | 3 ++- src/png_reader.cpp | 3 ++- tests/cpp_tests/agg_blend_src_over_test.cpp | 7 ++++--- utils/shapeindex/quadtree.hpp | 14 +++++++------- 7 files changed, 21 insertions(+), 17 deletions(-) diff --git a/include/mapnik/hextree.hpp b/include/mapnik/hextree.hpp index a77545278..8f2327ec5 100644 --- a/include/mapnik/hextree.hpp +++ b/include/mapnik/hextree.hpp @@ -33,6 +33,7 @@ // stl #include +#include #include #include #include @@ -68,7 +69,7 @@ class hextree : private mapnik::noncopyable pixel_count(0), children_count(0) { - memset(&children_[0],0,sizeof(children_)); + std::memset(&children_[0],0,sizeof(children_)); } ~node () diff --git a/include/mapnik/octree.hpp b/include/mapnik/octree.hpp index e8c66f4af..027cbcf90 100644 --- a/include/mapnik/octree.hpp +++ b/include/mapnik/octree.hpp @@ -30,6 +30,7 @@ // stl #include +#include #include #include @@ -62,7 +63,7 @@ class octree : private mapnik::noncopyable children_count(0), index(0) { - memset(&children_[0],0,sizeof(children_)); + std::memset(&children_[0],0,sizeof(children_)); } ~node() diff --git a/plugins/input/rasterlite/rasterlite_featureset.cpp b/plugins/input/rasterlite/rasterlite_featureset.cpp index adf68faec..554f239a3 100644 --- a/plugins/input/rasterlite/rasterlite_featureset.cpp +++ b/plugins/input/rasterlite/rasterlite_featureset.cpp @@ -31,8 +31,7 @@ #include #include -// boost - +#include using mapnik::coord2d; using mapnik::box2d; @@ -122,7 +121,7 @@ feature_ptr rasterlite_featureset::get_feature(mapnik::query const& q) unsigned char* raster_data = static_cast(raster); unsigned char* image_data = image.getBytes(); - memcpy (image_data, raster_data, size); + std::memcpy(image_data, raster_data, size); feature->set_raster(rasterp); diff --git a/plugins/input/shape/dbfile.cpp b/plugins/input/shape/dbfile.cpp index dd5b3478e..dca016938 100644 --- a/plugins/input/shape/dbfile.cpp +++ b/plugins/input/shape/dbfile.cpp @@ -38,6 +38,7 @@ // stl #include +#include #include dbf_file::dbf_file() @@ -223,7 +224,7 @@ void dbf_file::read_header() skip(22); std::streampos offset=0; char name[11]; - memset(&name,0,11); + std::memset(&name,0,11); fields_.reserve(num_fields_); for (int i=0;i // stl +#include #include namespace mapnik @@ -158,7 +159,7 @@ template void png_reader::init() { png_byte header[8]; - memset(header,0,8); + std::memset(header,0,8); stream_.read(reinterpret_cast(header),8); if ( stream_.gcount() != 8) { diff --git a/tests/cpp_tests/agg_blend_src_over_test.cpp b/tests/cpp_tests/agg_blend_src_over_test.cpp index 763324d81..9d8aec720 100644 --- a/tests/cpp_tests/agg_blend_src_over_test.cpp +++ b/tests/cpp_tests/agg_blend_src_over_test.cpp @@ -1,6 +1,7 @@ #include #include #include +#include #include #include #include @@ -32,7 +33,7 @@ color blend(color const& source, color const& dest, unsigned cover=255) dest_pre.premultiply(); unsigned char* buffer = new unsigned char[size*size*stride]; - memset(buffer, 0, size*size*stride); + std::memset(buffer, 0, size*size*stride); buffer[0] = dest_pre.r; buffer[1] = dest_pre.g; buffer[2] = dest_pre.b; @@ -62,7 +63,7 @@ color normal_blend(color const& source, color const& dest, unsigned cover=255) dest_pre.premultiply(); // source buffer unsigned char* source_buffer = new unsigned char[size*size*stride]; - memset(source_buffer, 0, size*size*stride); + std::memset(source_buffer, 0, size*size*stride); source_buffer[0] = source_pre.r; source_buffer[1] = source_pre.g; source_buffer[2] = source_pre.b; @@ -72,7 +73,7 @@ color normal_blend(color const& source, color const& dest, unsigned cover=255) // destination buffer unsigned char* dest_buffer = new unsigned char[size*size*stride]; - memset(dest_buffer, 0, size*size*stride); + std::memset(dest_buffer, 0, size*size*stride); dest_buffer[0] = dest_pre.r; dest_buffer[1] = dest_pre.g; dest_buffer[2] = dest_pre.b; diff --git a/utils/shapeindex/quadtree.hpp b/utils/shapeindex/quadtree.hpp index 57aee0f16..db6478755 100644 --- a/utils/shapeindex/quadtree.hpp +++ b/utils/shapeindex/quadtree.hpp @@ -42,7 +42,7 @@ struct quadtree_node quadtree_node(const box2d& ext) : ext_(ext),data_() { - memset(children_,0,sizeof(quadtree_node*)*4); + std::memset(children_,0,sizeof(quadtree_node*)*4); } ~quadtree_node() @@ -118,7 +118,7 @@ public: void write(std::ostream& out) { char header[16]; - memset(header,0,16); + std::memset(header,0,16); header[0]='m'; header[1]='a'; header[2]='p'; @@ -205,10 +205,10 @@ private: int shape_count=node->data_.size(); int recsize=sizeof(box2d) + 3 * sizeof(int) + shape_count * sizeof(T); char* node_record=new char[recsize]; - memset(node_record,0,recsize); - memcpy(node_record,&offset,4); - memcpy(node_record+4,&node->ext_,sizeof(box2d)); - memcpy(node_record+36,&shape_count,4); + std::memset(node_record,0,recsize); + std::memcpy(node_record,&offset,4); + std::memcpy(node_record+4,&node->ext_,sizeof(box2d)); + std::memcpy(node_record+36,&shape_count,4); for (int i=0;idata_[i]),sizeof(T)); @@ -221,7 +221,7 @@ private: ++num_subnodes; } } - memcpy(node_record + 40 + shape_count * sizeof(T),&num_subnodes,4); + std::memcpy(node_record + 40 + shape_count * sizeof(T),&num_subnodes,4); out.write(node_record,recsize); delete [] node_record; From fb0dca7b5419b5a8876f9440eb2dd6b3b8aac270 Mon Sep 17 00:00:00 2001 From: Dane Springmeyer Date: Tue, 28 Jan 2014 19:47:23 -0800 Subject: [PATCH 3/9] array allocation test --- benchmark/run | 1 + benchmark/test_array_allocation.cpp | 226 ++++++++++++++++++++++++++++ 2 files changed, 227 insertions(+) create mode 100644 benchmark/test_array_allocation.cpp diff --git a/benchmark/run b/benchmark/run index a909710ca..c4b0deb40 100755 --- a/benchmark/run +++ b/benchmark/run @@ -10,6 +10,7 @@ function run { ${BASE}/$1 --threads $2 --iterations $(expr $3 / $2); } +run test_array_allocation 20 100000 run test_png_encoding1 10 1000 run test_png_encoding2 10 50 run test_to_string1 10 100000 diff --git a/benchmark/test_array_allocation.cpp b/benchmark/test_array_allocation.cpp new file mode 100644 index 000000000..f21dd8648 --- /dev/null +++ b/benchmark/test_array_allocation.cpp @@ -0,0 +1,226 @@ +#include "bench_framework.hpp" +#include +#include +#include +#include + +// http://stackoverflow.com/questions/17347254/why-is-allocation-and-deallocation-of-stdvector-slower-than-dynamic-array-on-m + +#define FULL_ZERO_CHECK + +inline void ensure_zero(uint8_t * data, uint32_t size) { +#ifdef FULL_ZERO_CHECK + for (std::size_t i=0;i array_; + test1(mapnik::parameters const& params) + : test_case(params), + size_(*params.get("size",256*256)), + array_(size_,0) { } + bool validate() const + { + return true; + } + void operator()() const + { + for (std::size_t i=0;i array_; + test1b(mapnik::parameters const& params) + : test_case(params), + size_(*params.get("size",256*256)), + array_(size_,0) { } + bool validate() const + { + return true; + } + void operator()() const + { + for (std::size_t i=0;i array_; + test2(mapnik::parameters const& params) + : test_case(params), + size_(*params.get("size",256*256)), + array_(size_,0) { } + bool validate() const + { + return true; + } + void operator()() const + { + for (std::size_t i=0;i(::operator new(sizeof(uint8_t)*size_)); + memcpy(data, &array_[0], size_); + ensure_zero(data,size_); + ::operator delete(data),data=0; + } + } +}; + +class test3 : public benchmark::test_case +{ +public: + uint32_t size_; + std::vector array_; + test3(mapnik::parameters const& params) + : test_case(params), + size_(*params.get("size",256*256)), + array_(size_,0) { } + bool validate() const + { + return true; + } + void operator()() const + { + for (std::size_t i=0;i data(size_); + ensure_zero(&data[0],data.size()); + } + } +}; + + +class test3b : public benchmark::test_case +{ +public: + uint32_t size_; + std::vector array_; + test3b(mapnik::parameters const& params) + : test_case(params), + size_(*params.get("size",256*256)), + array_(size_,0) { } + bool validate() const + { + return true; + } + void operator()() const + { + for (std::size_t i=0;i data(0); + data.resize(size_,0); + ensure_zero(&data[0],data.size()); + } + } +}; + + +class test3c : public benchmark::test_case +{ +public: + uint32_t size_; + std::vector array_; + test3c(mapnik::parameters const& params) + : test_case(params), + size_(*params.get("size",256*256)), + array_(size_,0) { } + bool validate() const + { + return true; + } + void operator()() const + { + for (std::size_t i=0;i data(0); + data.assign(size_,0); + ensure_zero(&data[0],data.size()); + } + } +}; + +class test4 : public benchmark::test_case +{ +public: + uint32_t size_; + std::vector array_; + test4(mapnik::parameters const& params) + : test_case(params), + size_(*params.get("size",256*256)), + array_(size_,0) { } + bool validate() const + { + return true; + } + void operator()() const + { + for (std::size_t i=0;i Date: Wed, 29 Jan 2014 13:03:35 -0800 Subject: [PATCH 4/9] add std::string and std::valarray to array allocation benchmark --- benchmark/test_array_allocation.cpp | 84 +++++++++++++++++++++++++++++ 1 file changed, 84 insertions(+) diff --git a/benchmark/test_array_allocation.cpp b/benchmark/test_array_allocation.cpp index f21dd8648..c87630466 100644 --- a/benchmark/test_array_allocation.cpp +++ b/benchmark/test_array_allocation.cpp @@ -3,6 +3,7 @@ #include #include #include +#include // http://stackoverflow.com/questions/17347254/why-is-allocation-and-deallocation-of-stdvector-slower-than-dynamic-array-on-m @@ -189,6 +190,77 @@ public: } }; +class test5 : public benchmark::test_case +{ +public: + uint32_t size_; + std::vector array_; + test5(mapnik::parameters const& params) + : test_case(params), + size_(*params.get("size",256*256)), + array_(size_,0) { } + bool validate() const + { + return true; + } + void operator()() const + { + for (std::size_t i=0;i array_; + test5b(mapnik::parameters const& params) + : test_case(params), + size_(*params.get("size",256*256)), + array_(size_,0) { } + bool validate() const + { + return true; + } + void operator()() const + { + for (std::size_t i=0;i +// http://isocpp.org/blog/2013/04/trip-report-iso-c-spring-2013-meeting +// http://lists.cs.uiuc.edu/pipermail/cfe-commits/Week-of-Mon-20130909/088700.html +// http://stackoverflow.com/questions/17303902/any-alternative-to-stddynarray-presently-available + +class test6 : public benchmark::test_case +{ +public: + uint32_t size_; + std::vector array_; + test6(mapnik::parameters const& params) + : test_case(params), + size_(*params.get("size",256*256)), + array_(size_,0) { } + bool validate() const + { + return true; + } + void operator()() const + { + for (std::size_t i=0;i data(size_,0); + ensure_zero(&data[0],size_); + } + } +}; + int main(int argc, char** argv) { mapnik::parameters params; @@ -222,5 +294,17 @@ int main(int argc, char** argv) test3c test_runner(params); run(test_runner,"vector/assign"); } + { + test5 test_runner(params); + run(test_runner,"std::string range"); + } + { + test5 test_runner(params); + run(test_runner,"std::string &[0]"); + } + { + test5 test_runner(params); + run(test_runner,"valarray"); + } return 0; } From 538af2515d390fc668a3faf6dd1877831ea8fc4c Mon Sep 17 00:00:00 2001 From: Dane Springmeyer Date: Fri, 31 Jan 2014 19:12:31 -0800 Subject: [PATCH 5/9] fix rendering of multiple styles with OGR plugin - refs #2048 and mapbox/tilemill#2202 --- plugins/input/ogr/ogr_featureset.cpp | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/plugins/input/ogr/ogr_featureset.cpp b/plugins/input/ogr/ogr_featureset.cpp index 220e8e05c..b2a3d55f7 100644 --- a/plugins/input/ogr/ogr_featureset.cpp +++ b/plugins/input/ogr/ogr_featureset.cpp @@ -52,7 +52,7 @@ ogr_featureset::ogr_featureset(mapnik::context_ptr const& ctx, layer_(layer), layerdef_(layer.GetLayerDefn()), tr_(new transcoder(encoding)), - fidcolumn_(layer_.GetFIDColumn()), + fidcolumn_(layer_.GetFIDColumn()), // TODO - unused count_(0) { layer_.SetSpatialFilterRect (extent.minx(), @@ -67,6 +67,13 @@ ogr_featureset::~ogr_featureset() feature_ptr ogr_featureset::next() { + if (count_ == 0) + { + // Reset the layer reading on the first feature read + // this is a hack, but needed due to https://github.com/mapnik/mapnik/issues/2048 + // Proper solution is to avoid storing layer state in featureset + layer_.ResetReading(); + } OGRFeature *poFeature; while ((poFeature = layer_.GetNextFeature()) != nullptr) { From f6b74ff819e0001ce9a5a4333e62320f05e64292 Mon Sep 17 00:00:00 2001 From: Dane Springmeyer Date: Wed, 5 Feb 2014 12:31:55 -0800 Subject: [PATCH 6/9] fix #2141 --- SConstruct | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/SConstruct b/SConstruct index 31807d71b..8efe726ac 100644 --- a/SConstruct +++ b/SConstruct @@ -767,16 +767,20 @@ def FindBoost(context, prefixes, thread_flag): if BOOST_LIB_DIR: msg += '\nFound boost libs: %s' % BOOST_LIB_DIR env['BOOST_LIBS'] = BOOST_LIB_DIR - else: + elif not env['BOOST_LIBS']: env['BOOST_LIBS'] = '/usr/' + env['LIBDIR_SCHEMA'] msg += '\nUsing default boost lib dir: %s' % env['BOOST_LIBS'] + else: + msg += '\nUsing boost lib dir: %s' % env['BOOST_LIBS'] if BOOST_INCLUDE_DIR: msg += '\nFound boost headers: %s' % BOOST_INCLUDE_DIR env['BOOST_INCLUDES'] = BOOST_INCLUDE_DIR - else: + elif not env['BOOST_INCLUDES']: env['BOOST_INCLUDES'] = '/usr/include' msg += '\nUsing default boost include dir: %s' % env['BOOST_INCLUDES'] + else: + msg += '\nUsing boost include dir: %s' % env['BOOST_INCLUDES'] if not env['BOOST_TOOLKIT'] and not env['BOOST_ABI'] and not env['BOOST_VERSION']: if BOOST_APPEND: From 88613fc2ec0e7cfc34b7616eab31a552e9610091 Mon Sep 17 00:00:00 2001 From: Dane Springmeyer Date: Thu, 6 Feb 2014 17:05:46 -0800 Subject: [PATCH 7/9] check filesystem before trying to open plugin + only report unique directories searched - closes #2131 Conflicts: src/datasource_cache.cpp --- include/mapnik/datasource_cache.hpp | 3 ++- src/datasource_cache.cpp | 15 ++++++++++----- 2 files changed, 12 insertions(+), 6 deletions(-) diff --git a/include/mapnik/datasource_cache.hpp b/include/mapnik/datasource_cache.hpp index a5f16236f..a56c989af 100644 --- a/include/mapnik/datasource_cache.hpp +++ b/include/mapnik/datasource_cache.hpp @@ -35,6 +35,7 @@ // stl #include +#include namespace mapnik { @@ -56,7 +57,7 @@ private: ~datasource_cache(); std::map > plugins_; bool registered_; - std::vector plugin_directories_; + std::set plugin_directories_; }; } diff --git a/src/datasource_cache.cpp b/src/datasource_cache.cpp index 507e1f093..335ee2bb2 100644 --- a/src/datasource_cache.cpp +++ b/src/datasource_cache.cpp @@ -165,8 +165,7 @@ void datasource_cache::register_datasources(std::string const& str) #ifdef MAPNIK_THREADSAFE mapnik::scoped_lock lock(mutex_); #endif - // TODO - only push unique paths - plugin_directories_.push_back(str); + plugin_directories_.insert(str); if (mapnik::util::exists(str) && mapnik::util::is_directory(str)) { boost::filesystem::directory_iterator end_itr; @@ -194,9 +193,15 @@ void datasource_cache::register_datasources(std::string const& str) bool datasource_cache::register_datasource(std::string const& filename) { - bool success = false; try { + if (!mapnik::util::exists(filename)) + { + MAPNIK_LOG_ERROR(datasource_cache) + << "Cannot load '" + << filename << "' (plugin does not exist)"; + return false; + } std::shared_ptr plugin = std::make_shared(filename,"datasource_name"); if (plugin->valid()) { @@ -212,7 +217,7 @@ bool datasource_cache::register_datasource(std::string const& filename) MAPNIK_LOG_DEBUG(datasource_cache) << "datasource_cache: Registered=" << plugin->name(); - success = true; + return true; } } else @@ -228,7 +233,7 @@ bool datasource_cache::register_datasource(std::string const& filename) << "Exception caught while loading plugin library: " << filename << " (" << ex.what() << ")"; } - return success; + return false; } } From 3d9071588353ae2f2cd07fcd06ff24d57068ab5a Mon Sep 17 00:00:00 2001 From: Dane Springmeyer Date: Thu, 6 Feb 2014 18:24:35 -0800 Subject: [PATCH 8/9] only return true if plugins are actually newly registered --- src/datasource_cache.cpp | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/src/datasource_cache.cpp b/src/datasource_cache.cpp index 335ee2bb2..800cdd56d 100644 --- a/src/datasource_cache.cpp +++ b/src/datasource_cache.cpp @@ -213,11 +213,13 @@ bool datasource_cache::register_datasource(std::string const& filename) } else { - plugins_.insert(std::make_pair(plugin->name(),plugin)); - MAPNIK_LOG_DEBUG(datasource_cache) - << "datasource_cache: Registered=" - << plugin->name(); - return true; + if (plugins_.insert(std::make_pair(plugin->name(),plugin)).second) + { + MAPNIK_LOG_ERROR(datasource_cache) + << "datasource_cache: Registered=" + << plugin->name(); + return true; + } } } else From d99daff007e9d658e102451eb4b52e1dd8ff214a Mon Sep 17 00:00:00 2001 From: Dane Springmeyer Date: Sun, 9 Feb 2014 13:25:00 -0800 Subject: [PATCH 9/9] iwyu --- include/mapnik/webp_io.hpp | 1 + 1 file changed, 1 insertion(+) diff --git a/include/mapnik/webp_io.hpp b/include/mapnik/webp_io.hpp index 6f8ed2e18..1e1ccc2e8 100644 --- a/include/mapnik/webp_io.hpp +++ b/include/mapnik/webp_io.hpp @@ -24,6 +24,7 @@ #define MAPNIK_WEBP_IO_HPP // mapnik +#include #include // webp