From b9e1d3116c4f1d524f6fa1c95d4fa984c599eb83 Mon Sep 17 00:00:00 2001 From: Dennis Luxen Date: Wed, 8 Oct 2014 17:01:53 +0200 Subject: [PATCH 1/5] implements and fixes #997 - ServerPaths object can be populated by passing base path to OSRM - reduces code/functionality duplication in node-OSRM --- Library/OSRM_impl.cpp | 7 +- Library/OSRM_impl.h | 1 + Util/ProgramOptions.h | 191 +++++++++++++++++++++++------------------- routed.cpp | 18 +--- 4 files changed, 113 insertions(+), 104 deletions(-) diff --git a/Library/OSRM_impl.cpp b/Library/OSRM_impl.cpp index f18c812ef..0bbdc7da7 100644 --- a/Library/OSRM_impl.cpp +++ b/Library/OSRM_impl.cpp @@ -46,6 +46,7 @@ namespace boost { namespace interprocess { class named_mutex; } } #include "../Server/DataStructures/SharedBarriers.h" #include "../Server/DataStructures/SharedDataFacade.h" #include "../Util/make_unique.hpp" +#include "../Util/ProgramOptions.h" #include "../Util/SimpleLogger.h" #include @@ -57,8 +58,8 @@ namespace boost { namespace interprocess { class named_mutex; } } #include #include -OSRM_impl::OSRM_impl(const ServerPaths &server_paths, const bool use_shared_memory) - : use_shared_memory(use_shared_memory) +OSRM_impl::OSRM_impl(const ServerPaths &paths, const bool use_shared_memory) + : server_paths(paths) { if (use_shared_memory) { @@ -67,6 +68,8 @@ OSRM_impl::OSRM_impl(const ServerPaths &server_paths, const bool use_shared_memo } else { + // populate base path + populate_base_path(server_paths); query_data_facade = new InternalDataFacade(server_paths); } diff --git a/Library/OSRM_impl.h b/Library/OSRM_impl.h index e518036e9..38b919a8d 100644 --- a/Library/OSRM_impl.h +++ b/Library/OSRM_impl.h @@ -56,6 +56,7 @@ class OSRM_impl private: void RegisterPlugin(BasePlugin *plugin); PluginMap plugin_map; + ServerPaths server_paths; bool use_shared_memory; SharedBarriers *barrier; // base class pointer to the objects diff --git a/Util/ProgramOptions.h b/Util/ProgramOptions.h index c6e5448a6..d36bcfd8e 100644 --- a/Util/ProgramOptions.h +++ b/Util/ProgramOptions.h @@ -45,6 +45,109 @@ const static unsigned INIT_OK_START_ENGINE = 0; const static unsigned INIT_OK_DO_NOT_START_ENGINE = 1; const static unsigned INIT_FAILED = -1; +inline void populate_base_path(ServerPaths & server_paths) +{ + // populate the server_path object + auto path_iterator = server_paths.find("base"); + BOOST_ASSERT(server_paths.end() != path_iterator); + std::string base_string = path_iterator->second.string(); + SimpleLogger().Write() << "populating base path: " << base_string; + + path_iterator = server_paths.find("hsgrdata"); + if (path_iterator != server_paths.end() && + !boost::filesystem::is_regular_file(path_iterator->second)) + { + path_iterator->second = base_string + ".hsgr"; + } + else + { + throw OSRMException(base_string + ".hsgr not found"); + } + + path_iterator = server_paths.find("nodesdata"); + if (path_iterator != server_paths.end() && + !boost::filesystem::is_regular_file(path_iterator->second)) + { + path_iterator->second = base_string + ".nodes"; + } + else + { + throw OSRMException(base_string + ".nodes not found"); + } + + path_iterator = server_paths.find("edgesdata"); + if (path_iterator != server_paths.end() && + !boost::filesystem::is_regular_file(path_iterator->second)) + { + path_iterator->second = base_string + ".edges"; + } + else + { + throw OSRMException(base_string + ".edges not found"); + } + + path_iterator = server_paths.find("geometries"); + if (path_iterator != server_paths.end() && + !boost::filesystem::is_regular_file(path_iterator->second)) + { + path_iterator->second = base_string + ".geometry"; + } + else + { + throw OSRMException(base_string + ".geometry not found"); + } + + path_iterator = server_paths.find("ramindex"); + if (path_iterator != server_paths.end() && + !boost::filesystem::is_regular_file(path_iterator->second)) + { + path_iterator->second = base_string + ".ramIndex"; + } + else + { + throw OSRMException(base_string + ".ramIndex not found"); + } + + path_iterator = server_paths.find("fileindex"); + if (path_iterator != server_paths.end() && + !boost::filesystem::is_regular_file(path_iterator->second)) + { + path_iterator->second = base_string + ".fileIndex"; + } + else + { + throw OSRMException(base_string + ".fileIndex not found"); + } + + path_iterator = server_paths.find("namesdata"); + if (path_iterator != server_paths.end() && + !boost::filesystem::is_regular_file(path_iterator->second)) + { + path_iterator->second = base_string + ".names"; + } + else + { + throw OSRMException(base_string + ".namesIndex not found"); + } + + path_iterator = server_paths.find("timestamp"); + if (path_iterator != server_paths.end() && + !boost::filesystem::is_regular_file(path_iterator->second)) + { + path_iterator->second = base_string + ".timestamp"; + } + + SimpleLogger().Write() << "HSGR file:\t" << server_paths["hsgrdata"]; + SimpleLogger().Write(logDEBUG) << "Nodes file:\t" << server_paths["nodesdata"]; + SimpleLogger().Write(logDEBUG) << "Edges file:\t" << server_paths["edgesdata"]; + SimpleLogger().Write(logDEBUG) << "Geometry file:\t" << server_paths["geometries"]; + SimpleLogger().Write(logDEBUG) << "RAM file:\t" << server_paths["ramindex"]; + SimpleLogger().Write(logDEBUG) << "Index file:\t" << server_paths["fileindex"]; + SimpleLogger().Write(logDEBUG) << "Names file:\t" << server_paths["namesdata"]; + SimpleLogger().Write(logDEBUG) << "Timestamp file:\t" << server_paths["timestamp"]; +} + + // generate boost::program_options object for the routing part inline unsigned GenerateServerProgramOptions(const int argc, const char *argv[], @@ -169,94 +272,6 @@ inline unsigned GenerateServerProgramOptions(const int argc, if (!use_shared_memory && option_variables.count("base")) { - path_iterator = paths.find("base"); - BOOST_ASSERT(paths.end() != path_iterator); - std::string base_string = path_iterator->second.string(); - - path_iterator = paths.find("hsgrdata"); - if (path_iterator != paths.end() && - !boost::filesystem::is_regular_file(path_iterator->second)) - { - path_iterator->second = base_string + ".hsgr"; - } - else - { - throw OSRMException(base_string + ".hsgr not found"); - } - - path_iterator = paths.find("nodesdata"); - if (path_iterator != paths.end() && - !boost::filesystem::is_regular_file(path_iterator->second)) - { - path_iterator->second = base_string + ".nodes"; - } - else - { - throw OSRMException(base_string + ".nodes not found"); - } - - path_iterator = paths.find("edgesdata"); - if (path_iterator != paths.end() && - !boost::filesystem::is_regular_file(path_iterator->second)) - { - path_iterator->second = base_string + ".edges"; - } - else - { - throw OSRMException(base_string + ".edges not found"); - } - - path_iterator = paths.find("geometries"); - if (path_iterator != paths.end() && - !boost::filesystem::is_regular_file(path_iterator->second)) - { - path_iterator->second = base_string + ".geometry"; - } - else - { - throw OSRMException(base_string + ".geometry not found"); - } - - path_iterator = paths.find("ramindex"); - if (path_iterator != paths.end() && - !boost::filesystem::is_regular_file(path_iterator->second)) - { - path_iterator->second = base_string + ".ramIndex"; - } - else - { - throw OSRMException(base_string + ".ramIndex not found"); - } - - path_iterator = paths.find("fileindex"); - if (path_iterator != paths.end() && - !boost::filesystem::is_regular_file(path_iterator->second)) - { - path_iterator->second = base_string + ".fileIndex"; - } - else - { - throw OSRMException(base_string + ".fileIndex not found"); - } - - path_iterator = paths.find("namesdata"); - if (path_iterator != paths.end() && - !boost::filesystem::is_regular_file(path_iterator->second)) - { - path_iterator->second = base_string + ".names"; - } - else - { - throw OSRMException(base_string + ".namesIndex not found"); - } - - path_iterator = paths.find("timestamp"); - if (path_iterator != paths.end() && - !boost::filesystem::is_regular_file(path_iterator->second)) - { - path_iterator->second = base_string + ".timestamp"; - } - return INIT_OK_START_ENGINE; } if (use_shared_memory && !option_variables.count("base")) diff --git a/routed.cpp b/routed.cpp index e317797c5..ca64671a5 100644 --- a/routed.cpp +++ b/routed.cpp @@ -108,20 +108,10 @@ int main(int argc, const char *argv[]) { SimpleLogger().Write(logDEBUG) << "Loading from shared memory"; } - else - { - SimpleLogger().Write() << "HSGR file:\t" << server_paths["hsgrdata"]; - SimpleLogger().Write(logDEBUG) << "Nodes file:\t" << server_paths["nodesdata"]; - SimpleLogger().Write(logDEBUG) << "Edges file:\t" << server_paths["edgesdata"]; - SimpleLogger().Write(logDEBUG) << "Geometry file:\t" << server_paths["geometries"]; - SimpleLogger().Write(logDEBUG) << "RAM file:\t" << server_paths["ramindex"]; - SimpleLogger().Write(logDEBUG) << "Index file:\t" << server_paths["fileindex"]; - SimpleLogger().Write(logDEBUG) << "Names file:\t" << server_paths["namesdata"]; - SimpleLogger().Write(logDEBUG) << "Timestamp file:\t" << server_paths["timestamp"]; - SimpleLogger().Write(logDEBUG) << "Threads:\t" << requested_thread_num; - SimpleLogger().Write(logDEBUG) << "IP address:\t" << ip_address; - SimpleLogger().Write(logDEBUG) << "IP port:\t" << ip_port; - } + + SimpleLogger().Write(logDEBUG) << "Threads:\t" << requested_thread_num; + SimpleLogger().Write(logDEBUG) << "IP address:\t" << ip_address; + SimpleLogger().Write(logDEBUG) << "IP port:\t" << ip_port; #ifndef _WIN32 int sig = 0; sigset_t new_mask; From c3b54a63c3d08e034db2913ca537a235f21b75e1 Mon Sep 17 00:00:00 2001 From: Dennis Luxen Date: Wed, 8 Oct 2014 17:25:38 +0200 Subject: [PATCH 2/5] remove ServerPaths member from OSRM_impl. pass it by value to c'tor --- Library/OSRM.h | 2 +- Library/OSRM_impl.cpp | 5 ++--- Library/OSRM_impl.h | 3 +-- 3 files changed, 4 insertions(+), 6 deletions(-) diff --git a/Library/OSRM.h b/Library/OSRM.h index 5e3480249..372e82c39 100644 --- a/Library/OSRM.h +++ b/Library/OSRM.h @@ -46,7 +46,7 @@ class OSRM std::unique_ptr OSRM_pimpl_; public: - explicit OSRM(const ServerPaths &paths, const bool use_shared_memory = false); + explicit OSRM(ServerPaths paths, const bool use_shared_memory = false); ~OSRM(); void RunQuery(RouteParameters &route_parameters, http::Reply &reply); }; diff --git a/Library/OSRM_impl.cpp b/Library/OSRM_impl.cpp index 0bbdc7da7..b1c7ef2fb 100644 --- a/Library/OSRM_impl.cpp +++ b/Library/OSRM_impl.cpp @@ -58,8 +58,7 @@ namespace boost { namespace interprocess { class named_mutex; } } #include #include -OSRM_impl::OSRM_impl(const ServerPaths &paths, const bool use_shared_memory) - : server_paths(paths) +OSRM_impl::OSRM_impl(ServerPaths server_paths, const bool use_shared_memory) { if (use_shared_memory) { @@ -158,7 +157,7 @@ void OSRM_impl::RunQuery(RouteParameters &route_parameters, http::Reply &reply) // proxy code for compilation firewall -OSRM::OSRM(const ServerPaths &paths, const bool use_shared_memory) +OSRM::OSRM(ServerPaths paths, const bool use_shared_memory) : OSRM_pimpl_(osrm::make_unique(paths, use_shared_memory)) { } diff --git a/Library/OSRM_impl.h b/Library/OSRM_impl.h index 38b919a8d..803a5b065 100644 --- a/Library/OSRM_impl.h +++ b/Library/OSRM_impl.h @@ -48,7 +48,7 @@ class OSRM_impl using PluginMap = std::unordered_map; public: - OSRM_impl(const ServerPaths &paths, const bool use_shared_memory); + OSRM_impl(ServerPaths paths, const bool use_shared_memory); OSRM_impl(const OSRM_impl &) = delete; virtual ~OSRM_impl(); void RunQuery(RouteParameters &route_parameters, http::Reply &reply); @@ -56,7 +56,6 @@ class OSRM_impl private: void RegisterPlugin(BasePlugin *plugin); PluginMap plugin_map; - ServerPaths server_paths; bool use_shared_memory; SharedBarriers *barrier; // base class pointer to the objects From 9455ea0547dc46afbf1833e551d77a5b8eb0c0b5 Mon Sep 17 00:00:00 2001 From: Dennis Luxen Date: Wed, 8 Oct 2014 17:31:13 +0200 Subject: [PATCH 3/5] fix initialization of use_shared_memory --- Library/OSRM_impl.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Library/OSRM_impl.cpp b/Library/OSRM_impl.cpp index b1c7ef2fb..bbc09c5f5 100644 --- a/Library/OSRM_impl.cpp +++ b/Library/OSRM_impl.cpp @@ -58,7 +58,7 @@ namespace boost { namespace interprocess { class named_mutex; } } #include #include -OSRM_impl::OSRM_impl(ServerPaths server_paths, const bool use_shared_memory) +OSRM_impl::OSRM_impl(ServerPaths server_paths, const bool use_shared_memory) : use_shared_memory(use_shared_memory) { if (use_shared_memory) { From 4c64f5fe6270c622c787ac8396f655236c88885f Mon Sep 17 00:00:00 2001 From: Dennis Luxen Date: Wed, 8 Oct 2014 19:30:42 +0200 Subject: [PATCH 4/5] remove shared memory indicator member, use unique_ptr for barriers --- Library/OSRM_impl.cpp | 12 ++++-------- Library/OSRM_impl.h | 5 +++-- 2 files changed, 7 insertions(+), 10 deletions(-) diff --git a/Library/OSRM_impl.cpp b/Library/OSRM_impl.cpp index bbc09c5f5..d1e843298 100644 --- a/Library/OSRM_impl.cpp +++ b/Library/OSRM_impl.cpp @@ -58,11 +58,11 @@ namespace boost { namespace interprocess { class named_mutex; } } #include #include -OSRM_impl::OSRM_impl(ServerPaths server_paths, const bool use_shared_memory) : use_shared_memory(use_shared_memory) +OSRM_impl::OSRM_impl(ServerPaths server_paths, const bool use_shared_memory) { if (use_shared_memory) { - barrier = new SharedBarriers(); + barrier = osrm::make_unique(); query_data_facade = new SharedDataFacade(); } else @@ -88,10 +88,6 @@ OSRM_impl::~OSRM_impl() { delete plugin_pointer.second; } - if (use_shared_memory) - { - delete barrier; - } } void OSRM_impl::RegisterPlugin(BasePlugin *plugin) @@ -111,7 +107,7 @@ void OSRM_impl::RunQuery(RouteParameters &route_parameters, http::Reply &reply) if (plugin_map.end() != iter) { reply.status = http::Reply::ok; - if (use_shared_memory) + if (barrier) { // lock update pending boost::interprocess::scoped_lock pending_lock( @@ -132,7 +128,7 @@ void OSRM_impl::RunQuery(RouteParameters &route_parameters, http::Reply &reply) } iter->second->HandleRequest(route_parameters, reply); - if (use_shared_memory) + if (barrier) { // lock query boost::interprocess::scoped_lock query_lock( diff --git a/Library/OSRM_impl.h b/Library/OSRM_impl.h index 803a5b065..a4af62cb6 100644 --- a/Library/OSRM_impl.h +++ b/Library/OSRM_impl.h @@ -36,6 +36,7 @@ struct RouteParameters; #include "../DataStructures/QueryEdge.h" +#include #include #include @@ -56,8 +57,8 @@ class OSRM_impl private: void RegisterPlugin(BasePlugin *plugin); PluginMap plugin_map; - bool use_shared_memory; - SharedBarriers *barrier; + // will only be initialized if shared memory is used + std::unique_ptr barrier; // base class pointer to the objects BaseDataFacade *query_data_facade; }; From 4c0846734ea5889dd59f1754acd0ef5aae9a2dbf Mon Sep 17 00:00:00 2001 From: Dennis Luxen Date: Thu, 9 Oct 2014 13:30:11 +0200 Subject: [PATCH 5/5] rework the population and checking of base paths --- Util/ProgramOptions.h | 104 ++++++++++++++++++++---------------------- 1 file changed, 49 insertions(+), 55 deletions(-) diff --git a/Util/ProgramOptions.h b/Util/ProgramOptions.h index d36bcfd8e..0a8dcff0b 100644 --- a/Util/ProgramOptions.h +++ b/Util/ProgramOptions.h @@ -45,96 +45,92 @@ const static unsigned INIT_OK_START_ENGINE = 0; const static unsigned INIT_OK_DO_NOT_START_ENGINE = 1; const static unsigned INIT_FAILED = -1; -inline void populate_base_path(ServerPaths & server_paths) +inline void populate_base_path(ServerPaths &server_paths) { // populate the server_path object auto path_iterator = server_paths.find("base"); - BOOST_ASSERT(server_paths.end() != path_iterator); - std::string base_string = path_iterator->second.string(); - SimpleLogger().Write() << "populating base path: " << base_string; + // if a base path has been set, we populate it. + if (path_iterator != server_paths.end()) + { + const std::string base_string = path_iterator->second.string(); + SimpleLogger().Write() << "populating base path: " << base_string; + + server_paths["hsgrdata"] = base_string + ".hsgr"; + BOOST_ASSERT(server_paths.find("hsgrdata") != server_paths.end()); + server_paths["nodesdata"] = base_string + ".nodes"; + BOOST_ASSERT(server_paths.find("nodesdata") != server_paths.end()); + server_paths["edgesdata"] = base_string + ".edges"; + BOOST_ASSERT(server_paths.find("edgesdata") != server_paths.end()); + server_paths["geometries"] = base_string + ".geometry"; + BOOST_ASSERT(server_paths.find("geometries") != server_paths.end()); + server_paths["ramindex"] = base_string + ".ramIndex"; + BOOST_ASSERT(server_paths.find("ramindex") != server_paths.end()); + server_paths["fileindex"] = base_string + ".fileIndex"; + BOOST_ASSERT(server_paths.find("fileindex") != server_paths.end()); + server_paths["namesdata"] = base_string + ".names"; + BOOST_ASSERT(server_paths.find("namesdata") != server_paths.end()); + server_paths["timestamp"] = base_string + ".timestamp"; + BOOST_ASSERT(server_paths.find("timestamp") != server_paths.end()); + } + + // check if files are give and whether they exist at all path_iterator = server_paths.find("hsgrdata"); - if (path_iterator != server_paths.end() && + if (path_iterator == server_paths.end() || !boost::filesystem::is_regular_file(path_iterator->second)) { - path_iterator->second = base_string + ".hsgr"; - } - else - { - throw OSRMException(base_string + ".hsgr not found"); + if (path_iterator == server_paths.end()) + { + SimpleLogger().Write() << "hsgrdata unset"; + } + if (!boost::filesystem::is_regular_file(path_iterator->second)) + { + SimpleLogger().Write() << "not a regular file"; + } + + throw OSRMException(".hsgr not found: " + path_iterator->second.string()); } path_iterator = server_paths.find("nodesdata"); - if (path_iterator != server_paths.end() && + if (path_iterator == server_paths.end() || !boost::filesystem::is_regular_file(path_iterator->second)) { - path_iterator->second = base_string + ".nodes"; - } - else - { - throw OSRMException(base_string + ".nodes not found"); + throw OSRMException(".nodes not found"); } path_iterator = server_paths.find("edgesdata"); - if (path_iterator != server_paths.end() && + if (path_iterator == server_paths.end() || !boost::filesystem::is_regular_file(path_iterator->second)) { - path_iterator->second = base_string + ".edges"; - } - else - { - throw OSRMException(base_string + ".edges not found"); + throw OSRMException(".edges not found"); } path_iterator = server_paths.find("geometries"); - if (path_iterator != server_paths.end() && + if (path_iterator == server_paths.end() || !boost::filesystem::is_regular_file(path_iterator->second)) { - path_iterator->second = base_string + ".geometry"; - } - else - { - throw OSRMException(base_string + ".geometry not found"); + throw OSRMException(".geometry not found"); } path_iterator = server_paths.find("ramindex"); - if (path_iterator != server_paths.end() && + if (path_iterator == server_paths.end() || !boost::filesystem::is_regular_file(path_iterator->second)) { - path_iterator->second = base_string + ".ramIndex"; - } - else - { - throw OSRMException(base_string + ".ramIndex not found"); + throw OSRMException(".ramIndex not found"); } path_iterator = server_paths.find("fileindex"); - if (path_iterator != server_paths.end() && + if (path_iterator == server_paths.end() || !boost::filesystem::is_regular_file(path_iterator->second)) { - path_iterator->second = base_string + ".fileIndex"; - } - else - { - throw OSRMException(base_string + ".fileIndex not found"); + throw OSRMException(".fileIndex not found"); } path_iterator = server_paths.find("namesdata"); - if (path_iterator != server_paths.end() && + if (path_iterator == server_paths.end() || !boost::filesystem::is_regular_file(path_iterator->second)) { - path_iterator->second = base_string + ".names"; - } - else - { - throw OSRMException(base_string + ".namesIndex not found"); - } - - path_iterator = server_paths.find("timestamp"); - if (path_iterator != server_paths.end() && - !boost::filesystem::is_regular_file(path_iterator->second)) - { - path_iterator->second = base_string + ".timestamp"; + throw OSRMException(".namesIndex not found"); } SimpleLogger().Write() << "HSGR file:\t" << server_paths["hsgrdata"]; @@ -147,7 +143,6 @@ inline void populate_base_path(ServerPaths & server_paths) SimpleLogger().Write(logDEBUG) << "Timestamp file:\t" << server_paths["timestamp"]; } - // generate boost::program_options object for the routing part inline unsigned GenerateServerProgramOptions(const int argc, const char *argv[], @@ -158,7 +153,6 @@ inline unsigned GenerateServerProgramOptions(const int argc, bool &use_shared_memory, bool &trial) { - // declare a group of options that will be allowed only on command line boost::program_options::options_description generic_options("Options"); generic_options.add_options()("version,v", "Show version")("help,h", "Show this help message")(