Skip to content

Commit 97be62d

Browse files
committed
refactor getting read-only middle object
Rewrite get_instance() to receive the shared pointer as a parameter and get rid of the no_Delete() hack which essentially renders the shared pointer mute. Also get rid of const for middle_query_t. There is no point in adding this as the middle_query_t interface is completely const already.
1 parent 168ca9d commit 97be62d

File tree

7 files changed

+50
-41
lines changed

7 files changed

+50
-41
lines changed

middle-pgsql.cpp

Lines changed: 26 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1167,32 +1167,40 @@ middle_pgsql_t::~middle_pgsql_t() {
11671167

11681168
}
11691169

1170-
std::shared_ptr<const middle_query_t> middle_pgsql_t::get_instance() const {
1171-
middle_pgsql_t* mid = new middle_pgsql_t();
1172-
mid->out_options = out_options;
1173-
mid->append = out_options->append;
1174-
mid->mark_pending = mark_pending;
1175-
1176-
//NOTE: this is thread safe for use in pending async processing only because
1177-
//during that process they are only read from
1178-
mid->cache = cache;
1179-
mid->persistent_cache = persistent_cache;
1180-
1181-
// We use a connection per table to enable the use of COPY */
1182-
for(int i=0; i<num_tables; i++) {
1170+
std::shared_ptr<middle_query_t>
1171+
middle_pgsql_t::get_query_instance(std::shared_ptr<middle_t> const &from) const
1172+
{
1173+
auto *src = dynamic_cast<middle_pgsql_t *>(from.get());
1174+
assert(src);
1175+
1176+
// Return a copy of the original, as we need separate database connections.
1177+
std::unique_ptr<middle_pgsql_t> mid(new middle_pgsql_t());
1178+
mid->out_options = src->out_options;
1179+
mid->append = src->out_options->append;
1180+
mid->mark_pending = src->mark_pending;
1181+
1182+
// NOTE: this is thread safe for use in pending async processing only because
1183+
// during that process they are only read from
1184+
mid->cache = src->cache;
1185+
mid->persistent_cache = src->persistent_cache;
1186+
1187+
// We use a connection per table to enable the use of COPY
1188+
for (int i = 0; i < num_tables; i++) {
11831189
mid->connect(mid->tables[i]);
11841190
PGconn* sql_conn = mid->tables[i].sql_conn;
11851191

1186-
if (tables[i].prepare) {
1187-
pgsql_exec(sql_conn, PGRES_COMMAND_OK, "%s", tables[i].prepare);
1192+
if (mid->tables[i].prepare) {
1193+
pgsql_exec(sql_conn, PGRES_COMMAND_OK, "%s",
1194+
mid->tables[i].prepare);
11881195
}
11891196

1190-
if (append && tables[i].prepare_intarray) {
1191-
pgsql_exec(sql_conn, PGRES_COMMAND_OK, "%s", tables[i].prepare_intarray);
1197+
if (mid->append && mid->tables[i].prepare_intarray) {
1198+
pgsql_exec(sql_conn, PGRES_COMMAND_OK, "%s",
1199+
mid->tables[i].prepare_intarray);
11921200
}
11931201
}
11941202

1195-
return std::shared_ptr<const middle_query_t>(mid);
1203+
return std::shared_ptr<middle_query_t>(mid.release());
11961204
}
11971205

11981206
size_t middle_pgsql_t::pending_count() const {

middle-pgsql.hpp

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,9 @@ struct middle_pgsql_t : public slim_middle_t {
7979
struct pg_conn *sql_conn;
8080
};
8181

82-
std::shared_ptr<const middle_query_t> get_instance() const override;
82+
std::shared_ptr<middle_query_t>
83+
get_query_instance(std::shared_ptr<middle_t> const &mid) const override;
84+
8385
private:
8486
void pgsql_stop_one(table_desc *table);
8587

middle-ram.cpp

Lines changed: 4 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -198,18 +198,9 @@ idlist_t middle_ram_t::relations_using_way(osmid_t) const
198198
"report it at https://github.com/openstreetmap/osm2pgsql/issues");
199199
}
200200

201-
namespace {
202-
203-
void no_delete(const middle_ram_t *)
201+
std::shared_ptr<middle_query_t>
202+
middle_ram_t::get_query_instance(std::shared_ptr<middle_t> const &mid) const
204203
{
205-
// boost::shared_ptr thinks we are going to delete
206-
// the middle object, but we are not. Heh heh heh.
207-
// So yeah, this is a hack...
208-
}
209-
210-
}
211-
212-
std::shared_ptr<const middle_query_t> middle_ram_t::get_instance() const {
213-
//shallow copy here because readonly access is thread safe
214-
return std::shared_ptr<const middle_query_t>(this, no_delete);
204+
// No copy here because readonly access is thread safe.
205+
return std::static_pointer_cast<middle_query_t>(mid);
215206
}

middle-ram.hpp

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -115,7 +115,9 @@ struct middle_ram_t : public middle_t {
115115

116116
size_t pending_count() const override;
117117

118-
std::shared_ptr<const middle_query_t> get_instance() const override;
118+
std::shared_ptr<middle_query_t>
119+
get_query_instance(std::shared_ptr<middle_t> const &mid) const override;
120+
119121
private:
120122

121123
void release_ways();

middle.hpp

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -73,8 +73,6 @@ struct middle_query_t {
7373
* \param way_id ID of the way to check
7474
*/
7575
virtual idlist_t relations_using_way(osmid_t way_id) const = 0;
76-
77-
virtual std::shared_ptr<const middle_query_t> get_instance() const = 0;
7876
};
7977

8078
/**
@@ -108,6 +106,9 @@ struct middle_t : public middle_query_t {
108106

109107
virtual size_t pending_count() const = 0;
110108

109+
virtual std::shared_ptr<middle_query_t>
110+
get_query_instance(std::shared_ptr<middle_t> const &mid) const = 0;
111+
111112
const options_t* out_options;
112113
};
113114

osmdata.cpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -188,7 +188,7 @@ namespace {
188188

189189
struct pending_threaded_processor : public middle_t::pending_processor {
190190
typedef std::vector<std::shared_ptr<output_t>> output_vec_t;
191-
typedef std::pair<std::shared_ptr<const middle_query_t>, output_vec_t> clone_t;
191+
typedef std::pair<std::shared_ptr<middle_query_t>, output_vec_t> clone_t;
192192

193193
static void do_jobs(output_vec_t const& outputs, pending_queue_t& queue, size_t& ids_done, std::mutex& mutex, int append, bool ways) {
194194
while (true) {
@@ -232,7 +232,7 @@ struct pending_threaded_processor : public middle_t::pending_processor {
232232
}
233233

234234
//starts up count threads and works on the queue
235-
pending_threaded_processor(std::shared_ptr<middle_query_t> mid,
235+
pending_threaded_processor(std::shared_ptr<middle_t> mid,
236236
const output_vec_t &outs, size_t thread_count,
237237
int append)
238238
//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 {
249249
clones.reserve(thread_count);
250250
for (size_t i = 0; i < thread_count; ++i) {
251251
//clone the middle
252-
std::shared_ptr<const middle_query_t> mid_clone = mid->get_instance();
252+
auto mid_clone = mid->get_query_instance(mid);
253253

254254
//clone the outs
255255
output_vec_t out_clones;

tests/mockups.hpp

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -35,9 +35,10 @@ struct dummy_middle_t : public middle_t {
3535

3636
idlist_t relations_using_way(osmid_t) const override { return idlist_t(); }
3737

38-
std::shared_ptr<const middle_query_t> get_instance() const override
38+
std::shared_ptr<middle_query_t>
39+
get_query_instance(std::shared_ptr<middle_t> const &mid) const override
3940
{
40-
return std::shared_ptr<const middle_query_t>();
41+
return std::shared_ptr<middle_query_t>();
4142
}
4243
};
4344

@@ -72,7 +73,11 @@ struct dummy_slim_middle_t : public slim_middle_t {
7273

7374
idlist_t relations_using_way(osmid_t) const override { return idlist_t(); }
7475

75-
std::shared_ptr<const middle_query_t> get_instance() const override {return std::shared_ptr<const middle_query_t>();}
76+
std::shared_ptr<middle_query_t>
77+
get_query_instance(std::shared_ptr<middle_t> const &mid) const override
78+
{
79+
return std::shared_ptr<middle_query_t>();
80+
}
7681

7782
void nodes_delete(osmid_t) override {};
7883
void node_changed(osmid_t) override {};

0 commit comments

Comments
 (0)