Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Pre-calculate distance values #5251

Merged
merged 3 commits into from
Oct 30, 2018
Merged

Pre-calculate distance values #5251

merged 3 commits into from
Oct 30, 2018

Conversation

danpat
Copy link
Member

@danpat danpat commented Oct 29, 2018

After a long and sordid history, we added annotations=distances to the table plugin back in #4990 and #5019

These PRs implemented on-the-fly distance calculations - the table results are obtained, then the paths are unpacked to calculate the distances.

The upside to this is that it costs nothing in terms of resources (no extra preprocessing, no extra RAM) if you don't use this feature.

The downside is that it's a bit slow. With CH, large matrices that take 1-2 seconds to complete returning just durations can take upwards of 60 seconds to complete when distances are included.

@niemeier-PSI submitted #2764 way back which took the other approach - pre-calculate edge distances, and store them as an additional edge property. This approach is nice and fast, but has the downside that it requires quite a bit more RAM.

This PR is a re-implementation of #2764 - and I will open a follow-up ticket to make this data optional.

Tasklist

@danpat danpat force-pushed the danpat_cache_distances branch 3 times, most recently from 363eac2 to ef302e8 Compare October 30, 2018 04:48
Copy link
Member

@ghoshkaj ghoshkaj left a comment

Choose a reason for hiding this comment

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

Looks good to me. Farewell unpacking distances code 😿. But hello speedier distance calculations.

Copy link
Contributor

@danpaz danpaz left a comment

Choose a reason for hiding this comment

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

A couple of nitpicks, otherwise LGTM.

danpat and others added 3 commits October 30, 2018 15:41
…ated distances on shortcuts, avoiding unpacking cost.

Adds approx 10% to total data size.  Speeds up large table requests by 2 orders of magnitude.

Co-authored-by: Kajari Ghosh <ghoshkaj@gmail.com>
…t we can avoid accumulating distances if they're not requested.
@danpat danpat force-pushed the danpat_cache_distances branch from 4135b0e to a67c4bf Compare October 30, 2018 22:41
@danpat danpat merged commit cb1db64 into master Oct 30, 2018
@DennisOSRM DennisOSRM deleted the danpat_cache_distances branch November 6, 2022 14:11
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants