Skip to content
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

Fix slow road merging #1847

Merged
merged 3 commits into from
Feb 26, 2019
Merged

Fix slow road merging #1847

merged 3 commits into from
Feb 26, 2019

Conversation

zerebubuth
Copy link
Member

The tile 9/454/201 was taking a huge amount of time to render - several hours. This is due to the changes I made in #1703 to prevent intersections between lines being re-added as part of MultiLineString operations. According to the OGC spec, LineStrings within a MultiLineString may only meet at points, but this means we end up with way more points in the output than we need to accurately describe the geometry. Therefore, we split them up into several features, where each MultiLineString in each feature only contains non-overlapping LineStrings.

The algorithm I was using to do this wasn't efficient - it simply looped over all the features looking for intersections with previous features. And it worked OK on cities with a lower road density and lots of mergeable cross-shaped junctions, such as New York.

However, in Tokyo, where the tile was taking hours to render, there are 400,000 lines in the tile and the road network is not a grid, meaning there's less opportunity for merging.

Therefore, I replaced the previous algorithm with one that is more efficient for larger numbers of tiles. It works by indexing all the shapes rather than doing direct comparison. For large inputs, the shapes are additionally sorted by bounding box area and the index split between small and large. This means that the small-to-small comparisons are done more quickly (any given two small shapes are much less likely to intersect than a small and a large, or a large and a large).

Finally, we have been stripping name off roads at low zooms, but were not stripping name:* translations or official_name and similar "name-like" properties. This PR adds an all_name_variants parameter to drop_properties which tells it to treat name as if it's all name:* and variants. One drop_properties which was only dropping name was converted to drop_names instead. Dropping these might (slightly) increase mergeability and will decrease the number of properties stored in the tile.

…hat 'name' really means 'all name-like properties'. This is useful when names have translations, and we don't want to have to spell out all the variants and translations by hand.
…MultiLineString to separate MultiLineStrings. This was previously a naive O(n^2) algorithm, but is now a naive O(n log n) algorithm.
@zerebubuth
Copy link
Member Author

Oh, I probably should have mentioned the times; previously, the 9/454/201 was taking several hours to generate - I don't know exactly how many because I gave up waiting. After this PR, it still takes 30 minutes, which isn't awesome but much better than it was.

# if there's a large number of geoms, switch to the split index and sort
# so that the spatially largest objects are towards the end of the list.
# this should make it more likely that earlier queries are fast.
if len(mls.geoms) > 15000:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we want to pull this value from config? Or is this a good number that's not likely to change?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point, thanks. I think it's a pretty good number to start with, but we might end up tweaking it later. I've made it configurable in 5889c36, with the default 15,000.

@zerebubuth zerebubuth merged commit 9a6dd8f into master Feb 26, 2019
@zerebubuth zerebubuth deleted the zerebubuth/slow-road-merging branch February 26, 2019 16:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants