From 545097cf06fa57167f2f8fe4b78c3d78f1c8bfe4 Mon Sep 17 00:00:00 2001 From: Michael Krasnyk Date: Fri, 22 Sep 2017 17:33:06 +0200 Subject: [PATCH] Don't use location cache if not needed --- features/options/extract/lua.feature | 40 ++++++++++++++----- include/extractor/extractor_config.hpp | 4 +- include/extractor/scripting_environment.hpp | 2 + .../extractor/scripting_environment_lua.hpp | 2 + src/extractor/extractor.cpp | 5 ++- src/extractor/scripting_environment_lua.cpp | 2 + src/tools/extract.cpp | 7 +++- .../mocks/mock_scripting_environment.hpp | 2 + 8 files changed, 50 insertions(+), 14 deletions(-) diff --git a/features/options/extract/lua.feature b/features/options/extract/lua.feature index 91209a2a9f8..7c7fd6eb094 100644 --- a/features/options/extract/lua.feature +++ b/features/options/extract/lua.feature @@ -6,7 +6,7 @@ Feature: osrm-extract lua ways:get_nodes() """ functions = require('testbot') - function way_function(profile, way, result) + functions.process_way = function(profile, way, result) for _, node in ipairs(way:get_nodes()) do print('node id ' .. node:id()) end @@ -14,7 +14,6 @@ Feature: osrm-extract lua ways:get_nodes() result.forward_speed = 1 end - functions.process_way = way_function return functions """ And the node map @@ -32,12 +31,36 @@ Feature: osrm-extract lua ways:get_nodes() And stdout should contain "node id 2" + Scenario: osrm-extract location-dependent data without add-locations-to-ways preprocessing and node locations cache + Given the profile file + """ + functions = require('testbot') + + functions.process_way = function(profile, way, result, relations) + print(way:get_location_tag('driving_side')) + end + + return functions + """ + And the node map + """ + a b + """ + And the ways + | nodes | + | ab | + And the data has been saved to disk + + When I try to run "osrm-extract --profile {profile_file} {osm_file} --location-dependent-data test/data/regions/null-island.geojson --use-locations-cache=false" + Then it should exit with an error + And stderr should contain "invalid location" + Scenario: osrm-extract location-dependent data Given the profile file """ functions = require('testbot') - function way_function(profile, way, result, relations) + functions.process_way = function(profile, way, result, relations) for _, key in ipairs({'answer', 'boolean', 'object', 'array'}) do print (key .. ' ' .. tostring(way:get_location_tag(key))) end @@ -45,7 +68,6 @@ Feature: osrm-extract lua ways:get_nodes() result.forward_speed = 1 end - functions.process_way = way_function return functions """ And the node map @@ -57,7 +79,7 @@ Feature: osrm-extract lua ways:get_nodes() | ab | And the data has been saved to disk - When I run "osrm-extract --profile {profile_file} {osm_file} --location-dependent-data test/data/regions/null-island.geojson" + When I run "osrm-extract --profile {profile_file} {osm_file} --location-dependent-data test/data/regions/null-island.geojson --use-locations-cache=false" Then it should exit successfully And stdout should contain "answer 42" And stdout should contain "boolean true" @@ -70,14 +92,13 @@ Feature: osrm-extract lua ways:get_nodes() """ functions = require('testbot') - function way_function(profile, way, result, relations) + functions.process_way = function(profile, way, result, relations) print('ISO3166-1 ' .. (way:get_location_tag('ISO3166-1') or 'none')) print('answer ' .. (way:get_location_tag('answer') or 'none')) result.forward_mode = mode.driving result.forward_speed = 1 end - functions.process_way = way_function return functions """ And the node locations @@ -95,7 +116,7 @@ Feature: osrm-extract lua ways:get_nodes() | ef | Null Island | And the data has been saved to disk - When I run "osrm-extract --profile {profile_file} {osm_file} --location-dependent-data test/data/regions/null-island.geojson --location-dependent-data test/data/regions/hong-kong.geojson" + When I run "osrm-extract --profile {profile_file} {osm_file} --location-dependent-data test/data/regions/null-island.geojson --location-dependent-data test/data/regions/hong-kong.geojson --use-locations-cache=false" Then it should exit successfully And stdout should not contain "1 GeoJSON polygon" And stdout should contain "2 GeoJSON polygons" @@ -108,13 +129,12 @@ Feature: osrm-extract lua ways:get_nodes() """ functions = require('testbot') - function way_function(profile, way, result, relations) + functions.process_way = function(profile, way, result, relations) print ('answer ' .. tostring(way:get_location_tag('answer'))) result.forward_mode = mode.driving result.forward_speed = 1 end - functions.process_way = way_function return functions """ And the node map diff --git a/include/extractor/extractor_config.hpp b/include/extractor/extractor_config.hpp index 6678d5b8eaa..8b106a97bac 100644 --- a/include/extractor/extractor_config.hpp +++ b/include/extractor/extractor_config.hpp @@ -68,7 +68,8 @@ struct ExtractorConfig final : storage::IOConfig ".osrm.icd", ".osrm.cnbg", ".osrm.cnbg_to_ebg"}), - requested_num_threads(0) + requested_num_threads(0), + use_locations_cache(true) { } @@ -88,6 +89,7 @@ struct ExtractorConfig final : storage::IOConfig bool use_metadata; bool parse_conditionals; + bool use_locations_cache; }; } } diff --git a/include/extractor/scripting_environment.hpp b/include/extractor/scripting_environment.hpp index 7b72cf70bd8..ade5e442ad2 100644 --- a/include/extractor/scripting_environment.hpp +++ b/include/extractor/scripting_environment.hpp @@ -70,6 +70,8 @@ class ScriptingEnvironment std::vector> &resulting_ways, std::vector> &resulting_relations, std::vector &resulting_restrictions) = 0; + + virtual bool HasLocationDependentData() const = 0; }; } } diff --git a/include/extractor/scripting_environment_lua.hpp b/include/extractor/scripting_environment_lua.hpp index 556e92e59cb..5ff8dbc5b59 100644 --- a/include/extractor/scripting_environment_lua.hpp +++ b/include/extractor/scripting_environment_lua.hpp @@ -91,6 +91,8 @@ class Sol2ScriptingEnvironment final : public ScriptingEnvironment std::vector> &resulting_relations, std::vector &resulting_restrictions) override; + bool HasLocationDependentData() const { return !location_dependent_data.empty(); } + private: LuaScriptingContext &GetSol2Context(); diff --git a/src/extractor/extractor.cpp b/src/extractor/extractor.cpp index 2c2825d489f..f34cb2a78a2 100644 --- a/src/extractor/extractor.cpp +++ b/src/extractor/extractor.cpp @@ -453,9 +453,10 @@ Extractor::ParseOSMData(ScriptingEnvironment &scripting_environment, osmium::io::Reader reader( input_file, osmium::osm_entity_bits::node | osmium::osm_entity_bits::way, read_meta); - // TODO: make location_cacher conditional const auto pipeline = - buffer_reader(reader) & location_cacher & buffer_transformer & buffer_storage; + scripting_environment.HasLocationDependentData() && config.use_locations_cache + ? buffer_reader(reader) & location_cacher & buffer_transformer & buffer_storage + : buffer_reader(reader) & buffer_transformer & buffer_storage; tbb::parallel_pipeline(num_threads, pipeline); } diff --git a/src/extractor/scripting_environment_lua.cpp b/src/extractor/scripting_environment_lua.cpp index d20767792f2..f6c9ce49348 100644 --- a/src/extractor/scripting_environment_lua.cpp +++ b/src/extractor/scripting_environment_lua.cpp @@ -302,6 +302,8 @@ void Sol2ScriptingEnvironment::InitContext(LuaScriptingContext &context) [](const osmium::Way &way) { return sol::as_table(way.nodes()); }, "get_location_tag", [&context](const osmium::Way &way, const char *key) { + if (context.location_dependent_data.empty()) + return sol::object(sol::nil); // HEURISTIC: use a single node (last) of the way to localize the way // For more complicated scenarios a proper merging of multiple tags // at one or many locations must be provided diff --git a/src/tools/extract.cpp b/src/tools/extract.cpp index ce1637bc777..bf87740277d 100644 --- a/src/tools/extract.cpp +++ b/src/tools/extract.cpp @@ -66,7 +66,12 @@ return_code parseArguments(int argc, boost::program_options::value>( &extractor_config.location_dependent_data_paths) ->composing(), - "GeoJSON files with location-dependent data"); + "GeoJSON files with location-dependent data")( + "use-locations-cache", + boost::program_options::value(&extractor_config.use_locations_cache) + ->implicit_value(true) + ->default_value(extractor_config.use_locations_cache), + "Use internal nodes locations cache for location-dependent data lookups"); bool dummy; // hidden options, will be allowed on command line, but will not be diff --git a/unit_tests/mocks/mock_scripting_environment.hpp b/unit_tests/mocks/mock_scripting_environment.hpp index 07c01c8397e..216f0ae8216 100644 --- a/unit_tests/mocks/mock_scripting_environment.hpp +++ b/unit_tests/mocks/mock_scripting_environment.hpp @@ -44,6 +44,8 @@ class MockScriptingEnvironment : public extractor::ScriptingEnvironment std::vector &) override final { } + + bool HasLocationDependentData() const { return false; }; }; } // namespace test