Merge pull request #871 from lonvia/refactor-middle

Refactor getting read-only middle object

Author: lonvia

middle-pgsql.cpp

@@ -1167,32 +1167,40 @@ middle_pgsql_t::~middle_pgsql_t() {
 
 }
 
-std::shared_ptr<const middle_query_t> middle_pgsql_t::get_instance() const {
-    middle_pgsql_t* mid = new middle_pgsql_t();
-    mid->out_options = out_options;
-    mid->append = out_options->append;
-    mid->mark_pending = mark_pending;
-
-    //NOTE: this is thread safe for use in pending async processing only because
-    //during that process they are only read from
-    mid->cache = cache;
-    mid->persistent_cache = persistent_cache;
-
-    // We use a connection per table to enable the use of COPY */
-    for(int i=0; i<num_tables; i++) {
+std::shared_ptr<middle_query_t>
+middle_pgsql_t::get_query_instance(std::shared_ptr<middle_t> const &from) const
+{
+    auto *src = dynamic_cast<middle_pgsql_t *>(from.get());
+    assert(src);
+
+    // Return a copy of the original, as we need separate database connections.
+    std::unique_ptr<middle_pgsql_t> mid(new middle_pgsql_t());
+    mid->out_options = src->out_options;
+    mid->append = src->out_options->append;
+    mid->mark_pending = src->mark_pending;
+
+    // NOTE: this is thread safe for use in pending async processing only because
+    // during that process they are only read from
+    mid->cache = src->cache;
+    mid->persistent_cache = src->persistent_cache;
+
+    // We use a connection per table to enable the use of COPY
+    for (int i = 0; i < num_tables; i++) {
         mid->connect(mid->tables[i]);
         PGconn* sql_conn = mid->tables[i].sql_conn;
 
-        if (tables[i].prepare) {
-            pgsql_exec(sql_conn, PGRES_COMMAND_OK, "%s", tables[i].prepare);
+        if (mid->tables[i].prepare) {
+            pgsql_exec(sql_conn, PGRES_COMMAND_OK, "%s",
+                       mid->tables[i].prepare);
         }
 
-        if (append && tables[i].prepare_intarray) {
-            pgsql_exec(sql_conn, PGRES_COMMAND_OK, "%s", tables[i].prepare_intarray);
+        if (mid->append && mid->tables[i].prepare_intarray) {
+            pgsql_exec(sql_conn, PGRES_COMMAND_OK, "%s",
+                       mid->tables[i].prepare_intarray);
         }
     }
 
-    return std::shared_ptr<const middle_query_t>(mid);
+    return std::shared_ptr<middle_query_t>(mid.release());
 }
 
 size_t middle_pgsql_t::pending_count() const {

middle-pgsql.hpp

@@ -79,7 +79,9 @@ struct middle_pgsql_t : public slim_middle_t {
         struct pg_conn *sql_conn;
     };
 
-    std::shared_ptr<const middle_query_t> get_instance() const override;
+    std::shared_ptr<middle_query_t>
+    get_query_instance(std::shared_ptr<middle_t> const &mid) const override;
+
 private:
     void pgsql_stop_one(table_desc *table);
 

middle-ram.cpp

@@ -198,18 +198,9 @@ idlist_t middle_ram_t::relations_using_way(osmid_t) const
         "report it at https://github.com/openstreetmap/osm2pgsql/issues");
 }
 
-namespace {
-
-void no_delete(const middle_ram_t *)
+std::shared_ptr<middle_query_t>
+middle_ram_t::get_query_instance(std::shared_ptr<middle_t> const &mid) const
 {
-    // boost::shared_ptr thinks we are going to delete
-    // the middle object, but we are not. Heh heh heh.
-    // So yeah, this is a hack...
-}
-
-}
-
-std::shared_ptr<const middle_query_t> middle_ram_t::get_instance() const {
-    //shallow copy here because readonly access is thread safe
-    return std::shared_ptr<const middle_query_t>(this, no_delete);
+    // No copy here because readonly access is thread safe.
+    return std::static_pointer_cast<middle_query_t>(mid);
 }

middle-ram.hpp

@@ -115,7 +115,9 @@ struct middle_ram_t : public middle_t {
 
     size_t pending_count() const override;
 
-    std::shared_ptr<const middle_query_t> get_instance() const override;
+    std::shared_ptr<middle_query_t>
+    get_query_instance(std::shared_ptr<middle_t> const &mid) const override;
+
 private:
 
     void release_ways();

middle.hpp

@@ -73,8 +73,6 @@ struct middle_query_t {
      * \param way_id ID of the way to check
      */
     virtual idlist_t relations_using_way(osmid_t way_id) const = 0;
-
-    virtual std::shared_ptr<const middle_query_t> get_instance() const = 0;
 };
 
 /**
@@ -108,6 +106,9 @@ struct middle_t : public middle_query_t {
 
     virtual size_t pending_count() const = 0;
 
+    virtual std::shared_ptr<middle_query_t>
+    get_query_instance(std::shared_ptr<middle_t> const &mid) const = 0;
+
     const options_t* out_options;
 };
 

osmdata.cpp

@@ -188,7 +188,7 @@ namespace {
 
 struct pending_threaded_processor : public middle_t::pending_processor {
     typedef std::vector<std::shared_ptr<output_t>> output_vec_t;
-    typedef std::pair<std::shared_ptr<const middle_query_t>, output_vec_t> clone_t;
+    typedef std::pair<std::shared_ptr<middle_query_t>, output_vec_t> clone_t;
 
     static void do_jobs(output_vec_t const& outputs, pending_queue_t& queue, size_t& ids_done, std::mutex& mutex, int append, bool ways) {
         while (true) {
@@ -232,7 +232,7 @@ struct pending_threaded_processor : public middle_t::pending_processor {
     }
 
     //starts up count threads and works on the queue
-    pending_threaded_processor(std::shared_ptr<middle_query_t> mid,
+    pending_threaded_processor(std::shared_ptr<middle_t> mid,
                                const output_vec_t &outs, size_t thread_count,
                                int append)
         //note that we cant hint to the stack how large it should be ahead of time
@@ -249,7 +249,7 @@ struct pending_threaded_processor : public middle_t::pending_processor {
         clones.reserve(thread_count);
         for (size_t i = 0; i < thread_count; ++i) {
             //clone the middle
-            std::shared_ptr<const middle_query_t> mid_clone = mid->get_instance();
+            auto mid_clone = mid->get_query_instance(mid);
 
             //clone the outs
             output_vec_t out_clones;

tests/mockups.hpp

@@ -35,9 +35,10 @@ struct dummy_middle_t : public middle_t {
 
     idlist_t relations_using_way(osmid_t) const override  { return idlist_t(); }
 
-    std::shared_ptr<const middle_query_t> get_instance() const override
+    std::shared_ptr<middle_query_t>
+    get_query_instance(std::shared_ptr<middle_t> const &mid) const override
     {
-        return std::shared_ptr<const middle_query_t>();
+        return std::shared_ptr<middle_query_t>();
     }
 };
 
@@ -72,7 +73,11 @@ struct dummy_slim_middle_t : public slim_middle_t {
 
     idlist_t relations_using_way(osmid_t) const override  { return idlist_t(); }
 
-    std::shared_ptr<const middle_query_t> get_instance() const override  {return std::shared_ptr<const middle_query_t>();}
+    std::shared_ptr<middle_query_t>
+    get_query_instance(std::shared_ptr<middle_t> const &mid) const override
+    {
+        return std::shared_ptr<middle_query_t>();
+    }
 
     void nodes_delete(osmid_t) override  {};
     void node_changed(osmid_t) override  {};