Disentangle processor from dependency manager

Author: joto

src/dependency-manager.cpp

@@ -43,18 +43,14 @@ bool full_dependency_manager_t::has_pending() const noexcept
     return !m_ways_pending_tracker.empty() || !m_rels_pending_tracker.empty();
 }
 
-void full_dependency_manager_t::process_pending(pending_processor &proc)
-{
-    osmid_t id;
-    while (id_tracker::is_valid(id = m_ways_pending_tracker.pop_mark())) {
-        proc.enqueue_way(id);
-    }
+idlist_t full_dependency_manager_t::get_ids(id_tracker& tracker) {
+    idlist_t list;
+    list.reserve(tracker.size());
 
-    proc.process_ways();
-
-    while (id_tracker::is_valid(id = m_rels_pending_tracker.pop_mark())) {
-        proc.enqueue_relation(id);
+    osmid_t id;
+    while (id_tracker::is_valid(id = tracker.pop_mark())) {
+        list.push_back(id);
     }
 
-    proc.process_relations();
+    return list;
 }

src/dependency-manager.hpp

@@ -8,17 +8,6 @@
 #include <cassert>
 #include <memory>
 
-struct pending_processor
-{
-    virtual ~pending_processor() = default;
-
-    virtual void enqueue_way(osmid_t id) = 0;
-    virtual void enqueue_relation(osmid_t id) = 0;
-
-    virtual void process_ways() = 0;
-    virtual void process_relations() = 0;
-};
-
 /**
  * The job of the dependency manager is to keep track of the dependencies
  * between OSM objects, that is nodes in ways and members of relations.
@@ -69,23 +58,24 @@ class dependency_manager_t
     virtual bool has_pending() const noexcept { return false; }
 
     /**
-     * Process all pending objects.
-     *
-     * \param pf Processor that we should feed the objects to and that
-     *        will handle the actual processing.
-     *
-     * \post !has_pending()
+     * Get the list of pending way ids. After calling this, the internal
+     * list is cleared.
      */
-    virtual void process_pending(pending_processor &) {}
+    virtual idlist_t get_pending_way_ids() { return {}; }
+
+    /**
+     * Get the list of pending relation ids. After calling this, the internal
+     * list is cleared.
+     */
+    virtual idlist_t get_pending_relation_ids() { return {}; }
 };
 
 /**
  * The job of the dependency manager is to keep track of the dependencies
  * between OSM objects, that is nodes in ways and members of relations.
  *
  * Whenever an OSM object changes, this class is notified and remembers
- * the ids for later use. Later on the class can be told to process the
- * ids it has remembered.
+ * the ids for later use.
  */
 class full_dependency_manager_t : public dependency_manager_t
 {
@@ -111,49 +101,19 @@ class full_dependency_manager_t : public dependency_manager_t
 
     bool has_pending() const noexcept override;
 
-    void process_pending(pending_processor &proc) override;
-
-    /**
-     * Get access to the pending way ids. This is for debugging only.
-     *
-     * Note that the list of pending way ids will be empty after calling
-     * this.
-     *
-     * \tparam TOutputIterator Some output iterator type, for instance
-     *         created with std::back_inserter(some vector).
-     * \param it output iterator to which all ids should be written. *it
-     *        must be of type osmid_t.
-     */
-    template <typename TOutputIterator>
-    void get_pending_way_ids(TOutputIterator &&it)
+    idlist_t get_pending_way_ids() override
     {
-        osmid_t id;
-        while (id_tracker::is_valid(id = m_ways_pending_tracker.pop_mark())) {
-            *it++ = id;
-        }
+        return get_ids(m_ways_pending_tracker);
     }
 
-    /**
-     * Get access to the pending relation ids. This is for debugging only.
-     *
-     * Note that the list of pending relation ids will be empty after calling
-     * this.
-     *
-     * \tparam TOutputIterator Some output iterator type, for instance
-     *         created with std::back_inserter(some vector).
-     * \param it output iterator to which all ids should be written. *it
-     *        must be of type osmid_t.
-     */
-    template <typename TOutputIterator>
-    void get_pending_relation_ids(TOutputIterator &&it)
+    idlist_t get_pending_relation_ids() override
     {
-        osmid_t id;
-        while (id_tracker::is_valid(id = m_rels_pending_tracker.pop_mark())) {
-            *it++ = id;
-        }
+        return get_ids(m_rels_pending_tracker);
     }
 
 private:
+    idlist_t get_ids(id_tracker& tracker);
+
     std::shared_ptr<middle_t> m_object_store;
 
     id_tracker m_ways_pending_tracker;

src/osmdata.cpp

@@ -162,7 +162,7 @@ void osmdata_t::flush() const
 
 namespace {
 
-struct pending_threaded_processor : public pending_processor
+struct pending_threaded_processor
 {
     using output_vec_t = std::vector<std::shared_ptr<output_t>>;
     using pending_queue_t = idlist_t;
@@ -244,10 +244,6 @@ struct pending_threaded_processor : public pending_processor
         }
     }
 
-    void enqueue_way(osmid_t id) override { m_queue.push_back(id); }
-
-    void enqueue_relation(osmid_t id) override { m_queue.push_back(id); }
-
     template <typename FUNCTION>
     void process_queue(char const *type, FUNCTION &&function)
     {
@@ -301,10 +297,15 @@ struct pending_threaded_processor : public pending_processor
         }
     }
 
-    void process_ways() override { process_queue("way", do_ways); }
+    void process_ways(idlist_t &&list)
+    {
+        m_queue = std::move(list);
+        process_queue("way", do_ways);
+    }
 
-    void process_relations() override
+    void process_relations(idlist_t &&list)
     {
+        m_queue = std::move(list);
         process_queue("relation", do_rels);
 
         // Collect expiry tree information from all clones and merge it back
@@ -357,7 +358,8 @@ void osmdata_t::stop() const
     if (opts->append && m_dependency_manager->has_pending()) {
         pending_threaded_processor ptp(m_mid, m_outs, opts->num_procs);
 
-        m_dependency_manager->process_pending(ptp);
+        ptp.process_ways(m_dependency_manager->get_pending_way_ids());
+        ptp.process_relations(m_dependency_manager->get_pending_relation_ids());
     }
 
     for (auto &out : m_outs) {

tests/test-middle.cpp

@@ -1071,8 +1071,7 @@ TEMPLATE_TEST_CASE("middle: change nodes in way", "", options_slim_default,
         mid->flush();
 
         REQUIRE(dependency_manager.has_pending());
-        idlist_t way_ids;
-        dependency_manager.get_pending_way_ids(std::back_inserter(way_ids));
+        idlist_t const way_ids = dependency_manager.get_pending_way_ids();
         REQUIRE_THAT(way_ids, Catch::Equals<osmid_t>({20}));
 
         check_way(mid, way20);
@@ -1104,8 +1103,7 @@ TEMPLATE_TEST_CASE("middle: change nodes in way", "", options_slim_default,
             mid->flush();
 
             REQUIRE(dependency_manager.has_pending());
-            idlist_t way_ids;
-            dependency_manager.get_pending_way_ids(std::back_inserter(way_ids));
+            idlist_t const way_ids = dependency_manager.get_pending_way_ids();
             REQUIRE_THAT(way_ids, Catch::Equals<osmid_t>({20, 22}));
 
             check_way(mid, way20);
@@ -1207,8 +1205,7 @@ TEMPLATE_TEST_CASE("middle: change nodes in relation", "", options_slim_default,
         mid->flush();
 
         REQUIRE(dependency_manager.has_pending());
-        idlist_t rel_ids;
-        dependency_manager.get_pending_relation_ids(std::back_inserter(rel_ids));
+        idlist_t const rel_ids = dependency_manager.get_pending_relation_ids();
 
         REQUIRE_THAT(rel_ids, Catch::Equals<osmid_t>({30}));
         check_relation(mid, rel30);
@@ -1228,11 +1225,9 @@ TEMPLATE_TEST_CASE("middle: change nodes in relation", "", options_slim_default,
         mid->flush();
 
         REQUIRE(dependency_manager.has_pending());
-        idlist_t way_ids;
-        dependency_manager.get_pending_way_ids(std::back_inserter(way_ids));
+        idlist_t const way_ids = dependency_manager.get_pending_way_ids();
         REQUIRE_THAT(way_ids, Catch::Equals<osmid_t>({20}));
-        idlist_t rel_ids;
-        dependency_manager.get_pending_relation_ids(std::back_inserter(rel_ids));
+        idlist_t const rel_ids = dependency_manager.get_pending_relation_ids();
         REQUIRE_THAT(rel_ids, Catch::Equals<osmid_t>({31}));
         check_relation(mid, rel31);
     }