From b4704af51cf5fddef40f6a28a68765c292085d4e Mon Sep 17 00:00:00 2001 From: Jochen Topf Date: Fri, 16 Aug 2024 16:36:02 +0200 Subject: [PATCH] Experiments with dependency processing for changed nodes When a new version of a node has the same location as the old version of that node, we don't need to re-process ways and relations this node is a member of. See #1046 This is some preliminary experiments. It is not tested well. Currently only works with flat node file. Does this work in all cases? Can we be shure that the state reflected in the middle tables/the flat node file is the same as in the output tables? What happens if we re-start updating with an older diff file? What happens if a nodes is deleted and then re-surrected? --- src/middle-pgsql.cpp | 61 ++++++++++++++++++++++++++++++--- src/middle-pgsql.hpp | 6 +++- src/middle-ram.cpp | 4 ++- src/middle-ram.hpp | 2 +- src/middle.hpp | 2 +- src/osmdata.cpp | 4 +-- tests/test-osm-file-parsing.cpp | 6 ++-- 7 files changed, 72 insertions(+), 13 deletions(-) diff --git a/src/middle-pgsql.cpp b/src/middle-pgsql.cpp index 83e229cd9..e52fd043c 100644 --- a/src/middle-pgsql.cpp +++ b/src/middle-pgsql.cpp @@ -505,18 +505,34 @@ std::size_t middle_query_pgsql_t::get_way_node_locations_db( return count; } -void middle_pgsql_t::node(osmium::Node const &node) +bool middle_pgsql_t::node(osmium::Node const &node) { assert(m_middle_state == middle_state::node); if (node.deleted()) { node_delete(node.id()); - } else { - if (m_options->append) { - node_delete(node.id()); - } + return false; + } + + if (!m_options->append) { node_set(node); + return false; } + + if (m_persistent_cache) { + auto const loc = get_node_location(node.id()); + const bool location_has_changed = !loc.valid() || loc != node.location(); + + node_delete(node.id()); + node_set(node); + + return location_has_changed; + } + + node_delete(node.id()); + node_set(node); + + return true; } void middle_pgsql_t::way(osmium::Way const &way) { @@ -623,6 +639,36 @@ osmium::Location middle_query_pgsql_t::get_node_location(osmid_t id) const : get_node_location_db(id); } +osmium::Location middle_pgsql_t::get_node_location_db(osmid_t id) const +{ + auto const res = m_db_connection.exec_prepared("get_node", id); + if (res.num_tuples() == 0) { + return osmium::Location{}; + } + + return osmium::Location{(int)std::strtol(res.get_value(0, 1), nullptr, 10), + (int)std::strtol(res.get_value(0, 2), nullptr, 10)}; +} + +osmium::Location middle_pgsql_t::get_node_location_flatnodes(osmid_t id) const +{ + if (id >= 0) { + return m_persistent_cache->get(id); + } + return osmium::Location{}; +} + +osmium::Location middle_pgsql_t::get_node_location(osmid_t id) const +{ + auto const loc = m_cache->get(id); + if (loc.valid()) { + return loc; + } + + return m_persistent_cache ? get_node_location_flatnodes(id) + : get_node_location_db(id); +} + size_t middle_query_pgsql_t::nodes_get_list(osmium::WayNodeList *nodes) const { return m_persistent_cache ? get_way_node_locations_flatnodes(nodes) @@ -1321,6 +1367,11 @@ middle_pgsql_t::middle_pgsql_t(std::shared_ptr thread_pool, m_tables.relations() = table_desc{*options, sql_for_relations()}; m_users_table = table_desc{*options, sql_for_users(m_store_options)}; + +/* m_db_connection.exec(build_sql( + *m_options, "PREPARE get_node(int8) AS" + " SELECT id, lon, lat FROM {schema}\"{prefix}_nodes\"" + " WHERE id = $1"));*/ } void middle_pgsql_t::set_requirements( diff --git a/src/middle-pgsql.hpp b/src/middle-pgsql.hpp index 15d91bc76..4276b0ee5 100644 --- a/src/middle-pgsql.hpp +++ b/src/middle-pgsql.hpp @@ -104,7 +104,7 @@ struct middle_pgsql_t : public middle_t void wait() override; - void node(osmium::Node const &node) override; + bool node(osmium::Node const &node) override; void way(osmium::Way const &way) override; void relation(osmium::Relation const &rel) override; @@ -179,6 +179,10 @@ struct middle_pgsql_t : public middle_t void set_requirements(output_requirements const &requirements) override; private: + osmium::Location get_node_location(osmid_t id) const; + osmium::Location get_node_location_flatnodes(osmid_t id) const; + osmium::Location get_node_location_db(osmid_t id) const; + void node_set(osmium::Node const &node); void node_delete(osmid_t id); diff --git a/src/middle-ram.cpp b/src/middle-ram.cpp index 13c3d96eb..db9262b30 100644 --- a/src/middle-ram.cpp +++ b/src/middle-ram.cpp @@ -189,7 +189,7 @@ bool middle_ram_t::get_object(osmium::item_type type, osmid_t id, return true; } -void middle_ram_t::node(osmium::Node const &node) +bool middle_ram_t::node(osmium::Node const &node) { assert(m_middle_state == middle_state::node); assert(node.visible()); @@ -206,6 +206,8 @@ void middle_ram_t::node(osmium::Node const &node) (!node.tags().empty() || m_store_options.untagged_nodes)) { store_object(node); } + + return false; } void middle_ram_t::way(osmium::Way const &way) diff --git a/src/middle-ram.hpp b/src/middle-ram.hpp index 935491012..146c10bd5 100644 --- a/src/middle-ram.hpp +++ b/src/middle-ram.hpp @@ -58,7 +58,7 @@ class middle_ram_t : public middle_t, public middle_query_t void stop() override; - void node(osmium::Node const &node) override; + bool node(osmium::Node const &node) override; void way(osmium::Way const &way) override; void relation(osmium::Relation const &) override; diff --git a/src/middle.hpp b/src/middle.hpp index 2edacfe5c..c84cd0377 100644 --- a/src/middle.hpp +++ b/src/middle.hpp @@ -115,7 +115,7 @@ class middle_t virtual void wait() {} /// This is called for every added, changed or deleted node. - virtual void node(osmium::Node const &node) = 0; + virtual bool node(osmium::Node const &node) = 0; /// This is called for every added, changed or deleted way. virtual void way(osmium::Way const &way) = 0; diff --git a/src/osmdata.cpp b/src/osmdata.cpp index ae2ae05cb..bb3746a5b 100644 --- a/src/osmdata.cpp +++ b/src/osmdata.cpp @@ -51,7 +51,7 @@ void osmdata_t::node(osmium::Node const &node) } } - m_mid->node(node); + auto const has_changed_location = m_mid->node(node); if (node.deleted()) { m_output->node_delete(node.id()); @@ -70,7 +70,7 @@ void osmdata_t::node(osmium::Node const &node) // way or relation referencing it, so we don't have to add that node // to the list of changed nodes. If the input data doesn't contain // object versions this will still work, because then the version is 0. - if (node.version() != 1) { + if (node.version() != 1 && has_changed_location) { m_changed_nodes.push_back(node.id()); } } else if (has_tags_or_attrs) { diff --git a/tests/test-osm-file-parsing.cpp b/tests/test-osm-file-parsing.cpp index 64afdaafe..ce7311fcb 100644 --- a/tests/test-osm-file-parsing.cpp +++ b/tests/test-osm-file-parsing.cpp @@ -40,7 +40,7 @@ struct counting_middle_t : public middle_t void cleanup() {} - void node(osmium::Node const &node) override { + bool node(osmium::Node const &node) override { assert(m_middle_state == middle_state::node); if (m_append) { @@ -48,9 +48,11 @@ struct counting_middle_t : public middle_t if (!node.deleted()) { ++node_count.added; } - return; + return true; } ++node_count.added; + + return true; } void way(osmium::Way const &way) override {