From fe887a2c83abd8698d23749126f4a42966533a44 Mon Sep 17 00:00:00 2001 From: Mathis Logemann Date: Fri, 21 Jan 2022 00:10:17 +0100 Subject: [PATCH 1/2] fix #4268 by adding mapped_memory_file --- include/mapnik/mapped_memory_cache.hpp | 8 +++ include/mapnik/util/mapped_memory_file.hpp | 78 ++++++++++++++++++++++ plugins/input/csv/csv_datasource.cpp | 32 +-------- plugins/input/shape/dbfile.cpp | 38 +---------- plugins/input/shape/dbfile.hpp | 19 +----- plugins/input/shape/shapefile.hpp | 59 ++-------------- src/CMakeLists.txt | 1 + src/mapped_memory_cache.cpp | 8 +++ src/util/mapped_memory_file.cpp | 76 +++++++++++++++++++++ test/unit/datasource/shapeindex.cpp | 21 ++---- test/unit/imaging/image_io_test.cpp | 6 +- utils/mapnik-index/process_csv_file.cpp | 40 ++--------- 12 files changed, 194 insertions(+), 192 deletions(-) create mode 100644 include/mapnik/util/mapped_memory_file.hpp create mode 100644 src/util/mapped_memory_file.cpp diff --git a/include/mapnik/mapped_memory_cache.hpp b/include/mapnik/mapped_memory_cache.hpp index 515f1fdce..132d372c4 100644 --- a/include/mapnik/mapped_memory_cache.hpp +++ b/include/mapnik/mapped_memory_cache.hpp @@ -54,6 +54,14 @@ class MAPNIK_DECL mapped_memory_cache : std::unordered_map cache_; public: bool insert(std::string const& key, mapped_region_ptr); + /** + * @brief removes the resource identified by key from the cache, if exists + * + * @param key unique identifier for the resource + * @return true if the resource was removed + * @return false if the resource was not removed or wasn't in the cache + */ + bool remove(std::string const& key); boost::optional find(std::string const& key, bool update_cache = false); void clear(); }; diff --git a/include/mapnik/util/mapped_memory_file.hpp b/include/mapnik/util/mapped_memory_file.hpp new file mode 100644 index 000000000..dae7260d8 --- /dev/null +++ b/include/mapnik/util/mapped_memory_file.hpp @@ -0,0 +1,78 @@ +/***************************************************************************** + * + * This file is part of Mapnik (c++ mapping toolkit) + * + * Copyright (C) 2021 Artem Pavlenko + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA + * + *****************************************************************************/ + +#ifndef MAPNIK_MEMORY_MAPPED_FILE_HPP +#define MAPNIK_MEMORY_MAPPED_FILE_HPP + +#include +#include + +#if defined(MAPNIK_MEMORY_MAPPED_FILE) +#include +#include +#include +#else +#include +#endif + +#include + +namespace mapnik { namespace util { + +/** + * @brief memory mapped file abstraction. Implementation depends on MAPNIK_MEMORY_MAPPED_FILE. + * Might be a simple file wrapper, if MAPNIK_MEMORY_MAPPED_FILE=0 + * + */ +class MAPNIK_DECL mapped_memory_file : public noncopyable { + public: +#ifdef MAPNIK_MEMORY_MAPPED_FILE + using file_source_type = boost::interprocess::ibufferstream; +#else + using file_source_type = std::ifstream; +#endif + + public: + mapped_memory_file(); + explicit mapped_memory_file(std::string const& file_name); + virtual ~mapped_memory_file(); + + file_source_type& file(); + bool is_open() const; + void skip(std::streampos bytes); + + /** + * @brief deletes the file identified by file_name. Might also remove the file from any caches. + */ + static void deleteFile(std::string const& file_name); + protected: + const std::string file_name_; +#ifdef MAPNIK_MEMORY_MAPPED_FILE + mapnik::mapped_region_ptr mapped_region_; +#endif + file_source_type file_; +}; + + +} } + +#endif diff --git a/plugins/input/csv/csv_datasource.cpp b/plugins/input/csv/csv_datasource.cpp index 9ae7d755a..9d3bc6b6e 100644 --- a/plugins/input/csv/csv_datasource.cpp +++ b/plugins/input/csv/csv_datasource.cpp @@ -54,6 +54,7 @@ MAPNIK_DISABLE_WARNING_PUSH MAPNIK_DISABLE_WARNING_POP #include #endif +#include // stl #include @@ -121,35 +122,8 @@ csv_datasource::csv_datasource(parameters const& params) } else { -#if defined (MAPNIK_MEMORY_MAPPED_FILE) - using file_source_type = boost::interprocess::ibufferstream; - file_source_type in; - mapnik::mapped_region_ptr mapped_region; - boost::optional memory = - mapnik::mapped_memory_cache::instance().find(filename_, true); - if (memory) - { - mapped_region = *memory; - in.buffer(static_cast(mapped_region->get_address()),mapped_region->get_size()); - } - else - { - throw std::runtime_error("could not create file mapping for " + filename_); - } -#elif defined (_WIN32) - std::ifstream in(mapnik::utf8_to_utf16(filename_),std::ios_base::in | std::ios_base::binary); - if (!in.is_open()) - { - throw mapnik::datasource_exception("CSV Plugin: could not open: '" + filename_ + "'"); - } -#else - std::ifstream in(filename_.c_str(),std::ios_base::in | std::ios_base::binary); - if (!in.is_open()) - { - throw mapnik::datasource_exception("CSV Plugin: could not open: '" + filename_ + "'"); - } -#endif - parse_csv(in); + mapnik::util::mapped_memory_file in_file{filename_}; + parse_csv(in_file.file()); if (has_disk_index_ && !extent_initialized_) { diff --git a/plugins/input/shape/dbfile.cpp b/plugins/input/shape/dbfile.cpp index 5bc46c3e2..c57fbfbf4 100644 --- a/plugins/input/shape/dbfile.cpp +++ b/plugins/input/shape/dbfile.cpp @@ -51,31 +51,12 @@ dbf_file::dbf_file() record_(0) {} dbf_file::dbf_file(std::string const& file_name) - :num_records_(0), + :mapped_memory_file{file_name}, + num_records_(0), num_fields_(0), record_length_(0), -#if defined(MAPNIK_MEMORY_MAPPED_FILE) - file_(), -#elif defined(_WIN32) - file_(mapnik::utf8_to_utf16(file_name), std::ios::in | std::ios::binary), -#else - file_(file_name.c_str() ,std::ios::in | std::ios::binary), -#endif record_(0) { - -#if defined(MAPNIK_MEMORY_MAPPED_FILE) - boost::optional memory = mapnik::mapped_memory_cache::instance().find(file_name,true); - if (memory) - { - mapped_region_ = *memory; - file_.buffer(static_cast((*memory)->get_address()),(*memory)->get_size()); - } - else - { - throw std::runtime_error("could not create file mapping for "+file_name); - } -#endif if (file_) { read_header(); @@ -88,16 +69,6 @@ dbf_file::~dbf_file() ::operator delete(record_); } - -bool dbf_file::is_open() -{ -#if defined(MAPNIK_MEMORY_MAPPED_FILE) - return (file_.buffer().second > 0); -#else - return file_.is_open(); -#endif -} - int dbf_file::num_records() const { return num_records_; @@ -274,8 +245,3 @@ int dbf_file::read_int() mapnik::read_int32_ndr(b,val); return val; } - -void dbf_file::skip(int bytes) -{ - file_.seekg(bytes,std::ios::cur); -} diff --git a/plugins/input/shape/dbfile.hpp b/plugins/input/shape/dbfile.hpp index bb16f9f3c..ff2cce4a5 100644 --- a/plugins/input/shape/dbfile.hpp +++ b/plugins/input/shape/dbfile.hpp @@ -28,14 +28,7 @@ #include #include -#if defined(MAPNIK_MEMORY_MAPPED_FILE) -#include -#include -MAPNIK_DISABLE_WARNING_PUSH -#include -#include -MAPNIK_DISABLE_WARNING_POP -#endif +#include // stl #include @@ -54,25 +47,18 @@ struct field_descriptor }; -class dbf_file : private mapnik::util::noncopyable +class dbf_file : public mapnik::util::mapped_memory_file { private: int num_records_; int num_fields_; std::size_t record_length_; std::vector fields_; -#if defined(MAPNIK_MEMORY_MAPPED_FILE) - boost::interprocess::ibufferstream file_; - mapnik::mapped_region_ptr mapped_region_; -#else - std::ifstream file_; -#endif char* record_; public: dbf_file(); dbf_file(std::string const& file_name); ~dbf_file(); - bool is_open(); int num_records() const; int num_fields() const; field_descriptor const& descriptor(int col) const; @@ -83,7 +69,6 @@ private: void read_header(); int read_short(); int read_int(); - void skip(int bytes); }; #endif //DBFFILE_HPP diff --git a/plugins/input/shape/shapefile.hpp b/plugins/input/shape/shapefile.hpp index 72ad4be7f..cf233de1a 100644 --- a/plugins/input/shape/shapefile.hpp +++ b/plugins/input/shape/shapefile.hpp @@ -35,16 +35,12 @@ #include #if defined(MAPNIK_MEMORY_MAPPED_FILE) -#include -MAPNIK_DISABLE_WARNING_PUSH -#include #include -#include -MAPNIK_DISABLE_WARNING_POP -#include #endif #include +#include + using mapnik::box2d; using mapnik::read_int32_ndr; using mapnik::read_int32_xdr; @@ -143,64 +139,24 @@ struct shape_record std::size_t length() {return size;} }; -class shape_file : mapnik::util::noncopyable +class shape_file : public mapnik::util::mapped_memory_file { public: - #if defined(MAPNIK_MEMORY_MAPPED_FILE) - using file_source_type = boost::interprocess::ibufferstream; using record_type = shape_record; - mapnik::mapped_region_ptr mapped_region_; #else - using file_source_type = std::ifstream; using record_type = shape_record; #endif - file_source_type file_; - shape_file() {} - shape_file(std::string const& file_name) : -#if defined(MAPNIK_MEMORY_MAPPED_FILE) - file_() -#elif defined(_WIN32) - file_(mapnik::utf8_to_utf16(file_name), std::ios::in | std::ios::binary) -#else - file_(file_name.c_str(), std::ios::in | std::ios::binary) -#endif + shape_file(std::string const& file_name) + : mapped_memory_file(file_name) { -#if defined(MAPNIK_MEMORY_MAPPED_FILE) - boost::optional memory = - mapnik::mapped_memory_cache::instance().find(file_name,true); - - if (memory) - { - mapped_region_ = *memory; - file_.buffer(static_cast(mapped_region_->get_address()),mapped_region_->get_size()); - } - else - { - throw std::runtime_error("could not create file mapping for "+file_name); - } -#endif } ~shape_file() {} - inline file_source_type& file() - { - return file_; - } - - inline bool is_open() const - { -#if defined(MAPNIK_MEMORY_MAPPED_FILE) - return (file_.buffer().second > 0); -#else - return file_.is_open(); -#endif - } - inline void read_record(record_type& rec) { #if defined(MAPNIK_MEMORY_MAPPED_FILE) @@ -241,11 +197,6 @@ public: file_.read(reinterpret_cast(&envelope), sizeof(envelope)); } - inline void skip(std::streampos bytes) - { - file_.seekg(bytes, std::ios::cur); - } - inline void rewind() { seek(100); diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index 831563072..e21672ae8 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -223,6 +223,7 @@ target_sources(mapnik PRIVATE target_sources(mapnik PRIVATE util/math.cpp util/utf_conv_win.cpp + util/mapped_memory_file.cpp ) if(USE_CAIRO) diff --git a/src/mapped_memory_cache.cpp b/src/mapped_memory_cache.cpp index 241dc8c1e..834d8c032 100644 --- a/src/mapped_memory_cache.cpp +++ b/src/mapped_memory_cache.cpp @@ -56,6 +56,14 @@ bool mapped_memory_cache::insert(std::string const& uri, mapped_region_ptr mem) return cache_.emplace(uri,mem).second; } +bool mapped_memory_cache::remove(std::string const& key) +{ +#ifdef MAPNIK_THREADSAFE + std::lock_guard lock(mutex_); +#endif + return cache_.erase(key) > 0; +} + boost::optional mapped_memory_cache::find(std::string const& uri, bool update_cache) { #ifdef MAPNIK_THREADSAFE diff --git a/src/util/mapped_memory_file.cpp b/src/util/mapped_memory_file.cpp new file mode 100644 index 000000000..3c9f99518 --- /dev/null +++ b/src/util/mapped_memory_file.cpp @@ -0,0 +1,76 @@ +#include +#include +#include +#ifdef _WIN32 +#include +#endif + +namespace mapnik +{ + namespace util + { + mapped_memory_file::mapped_memory_file() {} + + mapped_memory_file::mapped_memory_file(std::string const &file_name) + : file_name_{file_name}, +#if defined(MAPNIK_MEMORY_MAPPED_FILE) + file_() +#elif defined(_WIN32) + file_(mapnik::utf8_to_utf16(file_name), std::ios::in | std::ios::binary) +#else + file_(file_name.c_str(), std::ios::in | std::ios::binary) +#endif + { +#if defined(MAPNIK_MEMORY_MAPPED_FILE) + boost::optional memory = + mapnik::mapped_memory_cache::instance().find(file_name, true); + + if (memory) + { + mapped_region_ = *memory; + file_.buffer(static_cast(mapped_region_->get_address()),mapped_region_->get_size()); + } + else + { + throw std::runtime_error("could not create file mapping for "+file_name); + } +#endif + + } + + mapped_memory_file::file_source_type& mapped_memory_file::file() + { + return file_; + } + + bool mapped_memory_file::is_open() const + { + #if defined(MAPNIK_MEMORY_MAPPED_FILE) + return (file_.buffer().second > 0); + #else + return file_.is_open(); + #endif + } + + void mapped_memory_file::skip(std::streampos bytes) + { + file_.seekg(bytes, std::ios::cur); + } + + void mapped_memory_file::deleteFile(std::string const &file_name) + { +#ifdef MAPNIK_MEMORY_MAPPED_FILE + mapped_memory_cache::instance().remove(file_name); +#endif + if(util::exists(file_name)) { + util::remove(file_name); + } + + } + + mapped_memory_file::~mapped_memory_file() + { + } + + } +} diff --git a/test/unit/datasource/shapeindex.cpp b/test/unit/datasource/shapeindex.cpp index 22d391ad1..c0256a1d6 100644 --- a/test/unit/datasource/shapeindex.cpp +++ b/test/unit/datasource/shapeindex.cpp @@ -33,6 +33,7 @@ MAPNIK_DISABLE_WARNING_PUSH #include #include MAPNIK_DISABLE_WARNING_POP +#include namespace { @@ -107,10 +108,7 @@ TEST_CASE("invalid shapeindex") std::string path = "test/data/shp/boundaries.shp"; std::string index_path = path.substr(0, path.rfind(".")) + ".index"; // remove *.index if present - if (mapnik::util::exists(index_path)) - { - mapnik::util::remove(index_path); - } + mapnik::util::mapped_memory_file::deleteFile(index_path); // count features std::size_t feature_count = count_shapefile_features(path); @@ -132,10 +130,7 @@ TEST_CASE("invalid shapeindex") CHECK(feature_count_indexed == 0); } // remove *.index if present - if (mapnik::util::exists(index_path)) - { - mapnik::util::remove(index_path); - } + mapnik::util::mapped_memory_file::deleteFile(index_path); } } } @@ -159,10 +154,7 @@ TEST_CASE("shapeindex") std::string index_path = path.substr(0, path.rfind(".")) + ".index"; // remove *.index if present - if (mapnik::util::exists(index_path)) - { - mapnik::util::remove(index_path); - } + mapnik::util::mapped_memory_file::deleteFile(index_path); // count features std::size_t feature_count = count_shapefile_features(path); // create *.index @@ -180,10 +172,7 @@ TEST_CASE("shapeindex") // ensure number of features are the same REQUIRE(feature_count == feature_count_indexed); // remove *.index if present - if (mapnik::util::exists(index_path)) - { - mapnik::util::remove(index_path); - } + mapnik::util::mapped_memory_file::deleteFile(index_path); } } } diff --git a/test/unit/imaging/image_io_test.cpp b/test/unit/imaging/image_io_test.cpp index f22bf74ca..e059fda2b 100644 --- a/test/unit/imaging/image_io_test.cpp +++ b/test/unit/imaging/image_io_test.cpp @@ -19,6 +19,7 @@ MAPNIK_DISABLE_WARNING_PUSH #include #include MAPNIK_DISABLE_WARNING_POP +#include inline void make_directory(std::string const& dir) { boost::filesystem::create_directories(dir); @@ -205,10 +206,7 @@ SECTION("image_util : save_to_file/save_to_stream/save_to_string") CHECK(0 == std::memcmp(im2.bytes(), im.bytes(), im.width() * im.height())); } } - if (mapnik::util::exists(filename)) - { - mapnik::util::remove(filename); - } + mapnik::util::mapped_memory_file::deleteFile(filename); } } diff --git a/utils/mapnik-index/process_csv_file.cpp b/utils/mapnik-index/process_csv_file.cpp index 11739073a..1c89bc38d 100644 --- a/utils/mapnik-index/process_csv_file.cpp +++ b/utils/mapnik-index/process_csv_file.cpp @@ -28,15 +28,9 @@ #include #if defined(MAPNIK_MEMORY_MAPPED_FILE) -#include -MAPNIK_DISABLE_WARNING_PUSH -#include #include -#include -#include -MAPNIK_DISABLE_WARNING_POP -#include #endif +#include #include #include @@ -53,37 +47,11 @@ std::pair process_csv_file(T & boxes, s p.separator_ = separator; p.quote_ = quote; -#if defined(MAPNIK_MEMORY_MAPPED_FILE) - using file_source_type = boost::interprocess::ibufferstream; - file_source_type csv_file; - mapnik::mapped_region_ptr mapped_region; - boost::optional memory = - mapnik::mapped_memory_cache::instance().find(filename, true); - if (memory) - { - mapped_region = *memory; - csv_file.buffer(static_cast(mapped_region->get_address()),mapped_region->get_size()); - } - else - { - std::clog << "Error : cannot mmap " << filename << std::endl; - return std::make_pair(false, box_type(p.extent_)); - } -#else - #if defined(_WIN32) - std::ifstream csv_file(mapnik::utf8_to_utf16(filename),std::ios_base::in | std::ios_base::binary); - #else - std::ifstream csv_file(filename.c_str(),std::ios_base::in | std::ios_base::binary); - #endif - if (!csv_file.is_open()) - { - std::clog << "Error : cannot open " << filename << std::endl; - return std::make_pair(false, box_type(p.extent_)); - } -#endif + + util::mapped_memory_file csv_file{filename}; try { - p.parse_csv_and_boxes(csv_file, boxes); + p.parse_csv_and_boxes(csv_file.file(), boxes); return std::make_pair(true, box_type(p.extent_)); } catch (std::exception const& ex) From a3ccf2967fa6781cfcfe80682d4ce5cf978dcff0 Mon Sep 17 00:00:00 2001 From: Mathis Logemann Date: Mon, 24 Jan 2022 21:21:45 +0100 Subject: [PATCH 2/2] [scons] add mapped_memory_file to source list --- src/build.py | 1 + 1 file changed, 1 insertion(+) diff --git a/src/build.py b/src/build.py index 9227c1089..ac35d44f0 100644 --- a/src/build.py +++ b/src/build.py @@ -274,6 +274,7 @@ source = Split( renderer_common/render_thunk_extractor.cpp renderer_common/pattern_alignment.cpp util/math.cpp + util/mapped_memory_file.cpp value.cpp """ )