Merge pull request #1365 from joto/cleanup-persistent-cache

Cleanup persistent cache

Author: lonvia

src/middle-pgsql.cpp

@@ -270,8 +270,8 @@ class string_id_list
 
 }; // class string_id_list
 
-size_t
-middle_query_pgsql_t::local_nodes_get_list(osmium::WayNodeList *nodes) const
+std::size_t middle_query_pgsql_t::get_way_node_locations_db(
+    osmium::WayNodeList *nodes) const
 {
     size_t count = 0;
     string_id_list id_list;
@@ -330,10 +330,29 @@ void middle_pgsql_t::node_set(osmium::Node const &node)
     }
 }
 
+std::size_t middle_query_pgsql_t::get_way_node_locations_flatnodes(
+    osmium::WayNodeList *nodes) const
+{
+    std::size_t count = 0;
+
+    for (auto &n : *nodes) {
+        auto loc = m_cache->get(n.ref());
+        if (!loc.valid() && n.ref() >= 0) {
+            loc = m_persistent_cache->get(n.ref());
+        }
+        n.set_location(loc);
+        if (loc.valid()) {
+            ++count;
+        }
+    }
+
+    return count;
+}
+
 size_t middle_query_pgsql_t::nodes_get_list(osmium::WayNodeList *nodes) const
 {
-    return m_persistent_cache ? m_persistent_cache->get_list(nodes)
-                              : local_nodes_get_list(nodes);
+    return m_persistent_cache ? get_way_node_locations_flatnodes(nodes)
+                              : get_way_node_locations_db(nodes);
 }
 
 void middle_pgsql_t::node_delete(osmid_t osm_id)
@@ -756,7 +775,7 @@ middle_pgsql_t::middle_pgsql_t(options_t const *options)
   m_db_copy(m_copy_thread)
 {
     if (options->flat_node_cache_enabled) {
-        m_persistent_cache.reset(new node_persistent_cache{options, m_cache});
+        m_persistent_cache.reset(new node_persistent_cache{options});
     }
 
     log_debug("Mid: pgsql, cache={}", options->cache);

src/middle-pgsql.hpp

@@ -37,7 +37,8 @@ class middle_query_pgsql_t : public middle_query_t
     void exec_sql(std::string const &sql_cmd) const;
 
 private:
-    size_t local_nodes_get_list(osmium::WayNodeList *nodes) const;
+    std::size_t get_way_node_locations_flatnodes(osmium::WayNodeList *nodes) const;
+    std::size_t get_way_node_locations_db(osmium::WayNodeList *nodes) const;
 
     pg_conn_t m_sql_conn;
     std::shared_ptr<node_ram_cache> m_cache;

src/node-persistent-cache.cpp

@@ -6,44 +6,16 @@
 
 void node_persistent_cache::set(osmid_t id, osmium::Location location)
 {
-    if (id < 0) {
-        throw std::runtime_error{"Flatnode store cannot save negative IDs."};
-    }
     m_index->set(static_cast<osmium::unsigned_object_id_type>(id), location);
 }
 
 osmium::Location node_persistent_cache::get(osmid_t id) const noexcept
 {
-    if (id < 0) {
-        return osmium::Location{};
-    }
-
     return m_index->get_noexcept(
         static_cast<osmium::unsigned_object_id_type>(id));
 }
 
-std::size_t node_persistent_cache::get_list(osmium::WayNodeList *nodes) const
-{
-    std::size_t count = 0;
-
-    for (auto &n : *nodes) {
-        auto loc = m_ram_cache->get(n.ref());
-        if (!loc.valid() && n.ref() >= 0) {
-            loc = m_index->get_noexcept(
-                static_cast<osmium::unsigned_object_id_type>(n.ref()));
-        }
-        n.set_location(loc);
-        if (loc.valid()) {
-            ++count;
-        }
-    }
-
-    return count;
-}
-
-node_persistent_cache::node_persistent_cache(
-    const options_t *options, std::shared_ptr<node_ram_cache> ptr)
-: m_ram_cache(std::move(ptr))
+node_persistent_cache::node_persistent_cache(const options_t *options)
 {
     if (!options->flat_node_file) {
         throw std::runtime_error{"Unable to set up persistent cache: the name "

src/node-persistent-cache.hpp

@@ -6,29 +6,24 @@
 #include <osmium/index/map/dense_file_array.hpp>
 #include <osmium/osm/location.hpp>
 
-#include "node-ram-cache.hpp"
 #include "osmtypes.hpp"
 
 class options_t;
 
 class node_persistent_cache
 {
 public:
-    node_persistent_cache(options_t const *options,
-                          std::shared_ptr<node_ram_cache> ptr);
+    node_persistent_cache(options_t const *options);
     ~node_persistent_cache() noexcept;
 
     void set(osmid_t id, osmium::Location location);
     osmium::Location get(osmid_t id) const noexcept;
-    std::size_t get_list(osmium::WayNodeList *nodes) const;
 
 private:
-    // Dense node cache for unsigned IDs only
     using index_t =
         osmium::index::map::DenseFileArray<osmium::unsigned_object_id_type,
                                            osmium::Location>;
 
-    std::shared_ptr<node_ram_cache> m_ram_cache;
     int m_fd = -1;
     std::unique_ptr<index_t> m_index;
     bool m_remove_file = false;

tests/test-persistent-cache.cpp

@@ -29,11 +29,10 @@ TEST_CASE("Persistent cache", "[NoDB]")
 {
     options_t const options = testing::opt_t().flatnodes();
     testing::cleanup::file_t flatnode_cleaner{options.flat_node_file.get()};
-    auto ram_cache = std::make_shared<node_ram_cache>(); // dummy cache
 
     // create a new cache
     {
-        node_persistent_cache cache{&options, ram_cache};
+        node_persistent_cache cache{&options};
 
         // write in order
         write_and_read_location(cache, 10, 10.01, -45.3);
@@ -56,7 +55,7 @@ TEST_CASE("Persistent cache", "[NoDB]")
 
     // reopen the cache
     {
-        node_persistent_cache cache{&options, ram_cache};
+        node_persistent_cache cache{&options};
 
         // read all previously written locations
         read_location(cache, 10, 10.01, -45.3);