-
-
Notifications
You must be signed in to change notification settings - Fork 477
Introduce per geometry and overall limits on number of expire tiles #2449
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -27,20 +27,30 @@ | |
| #include "wkb.hpp" | ||
|
|
||
| expire_tiles_t::expire_tiles_t(uint32_t max_zoom, | ||
| std::shared_ptr<reprojection_t> projection) | ||
| : m_projection(std::move(projection)), m_maxzoom(max_zoom), | ||
| m_map_width(static_cast<int>(1U << m_maxzoom)) | ||
| {} | ||
| std::shared_ptr<reprojection_t> projection, | ||
| std::size_t max_tiles_geometry) | ||
| : m_projection(std::move(projection)), m_max_tiles_geometry(max_tiles_geometry), | ||
| m_maxzoom(max_zoom), m_map_width(static_cast<int>(1U << m_maxzoom)) | ||
| { | ||
| } | ||
|
|
||
| void expire_tiles_t::expire_tile(uint32_t x, uint32_t y) | ||
| { | ||
| // Only try to insert to tile into the set if the last inserted tile | ||
| // is different from this tile. | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Any particular reason to drop this optimisation?
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We don't have any benchmarks that prove that it makes sense, so it might have been premature. And it complicated the code. It is also unclear whether this should now be done for tiles inside a single geometry or for tiles "shared" between geometries which means there are different places where this could go. I opted to remove it here and maybe bring it back after all the other refactoring of the expire code is done if it seems useful then. |
||
| if (m_dirty_tiles.size() > m_max_tiles_geometry) { | ||
| return; | ||
| } | ||
|
|
||
| tile_t const new_tile{m_maxzoom, x, y}; | ||
| if (!m_prev_tile.valid() || m_prev_tile != new_tile) { | ||
| m_dirty_tiles.insert(new_tile.quadkey()); | ||
| m_prev_tile = new_tile; | ||
| m_dirty_tiles.insert(new_tile.quadkey()); | ||
| } | ||
|
|
||
| void expire_tiles_t::commit_tiles(expire_output_t *expire_output) | ||
| { | ||
| if (!expire_output || m_dirty_tiles.empty()) { | ||
| return; | ||
| } | ||
| expire_output->add_tiles(m_dirty_tiles); | ||
| m_dirty_tiles.clear(); | ||
| } | ||
|
|
||
| uint32_t expire_tiles_t::normalise_tile_x_coord(int x) const | ||
|
|
@@ -281,24 +291,6 @@ quadkey_list_t expire_tiles_t::get_tiles() | |
| return tiles; | ||
| } | ||
|
|
||
| void expire_tiles_t::merge_and_destroy(expire_tiles_t *other) | ||
| { | ||
| if (m_map_width != other->m_map_width) { | ||
| throw fmt_error("Unable to merge tile expiry sets when " | ||
| "map_width does not match: {} != {}.", | ||
| m_map_width, other->m_map_width); | ||
| } | ||
|
|
||
| if (m_dirty_tiles.empty()) { | ||
| using std::swap; | ||
| swap(m_dirty_tiles, other->m_dirty_tiles); | ||
| } else { | ||
| m_dirty_tiles.insert(other->m_dirty_tiles.cbegin(), | ||
| other->m_dirty_tiles.cend()); | ||
| other->m_dirty_tiles.clear(); | ||
| } | ||
| } | ||
|
|
||
| int expire_from_result(expire_tiles_t *expire, pg_result_t const &result, | ||
| expire_config_t const &expire_config) | ||
| { | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't that, strictly speaking, go into a mutex as well?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It isn't called in the place where there are multiple threads, but to be safe it is better to do that, you are right.