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

Add parameters to limit number of settled nodes during preparation, much faster preparation for certain graphs #37

Merged
merged 12 commits into from
Sep 13, 2021

Conversation

easbar
Copy link
Owner

@easbar easbar commented Sep 12, 2021

Fixes #34.

Long story short: When there are large-weight edges the preparation time can become much slower because witness searches explore too many nodes. Preventing this and still not adding too many shortcuts is non-trivial, so this PR introduces some parameters that can be used to tune performance depending for different types of graphs. It also provides some default values that should work reasonably well in most cases.

todo:

  • update benchmark values in README.md
  • add benchmark values to mini-performance tests in lib.rs and maybe further tune parameters

@easbar easbar mentioned this pull request Sep 12, 2021
7 tasks
Copy link
Contributor

@dabreegster dabreegster left a comment

Choose a reason for hiding this comment

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

Awesome, thanks!

/// calculated for all nodes initially. Since this does not take much time normally you should
/// probably keep the default.
pub max_settled_nodes_initial_relevance: usize,
/// The maximum number of settled nodes per witness search performed when updating priorities
Copy link
Contributor

Choose a reason for hiding this comment

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

These explanations are great, thank you! I kind of want to ask for something similar for hierarchy_depth_factor and edge_quotient_factor -- would users ever want to deviate from defaults?

Copy link
Owner Author

Choose a reason for hiding this comment

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

Good point, I added some documentation for these as well. I also ran some measurements for varying values of hierarchy_depth_factor, see here: https://gist.github.com/easbar/a7980e834206b6065e848ba29e228fdd#hierarchy-depth-factor

For the abstreet maps you sent reducing hierarchy_depth_factor from 0.1 to 0.01 might actually reduce preparation times further (but also might yield somewhat slower queries, ~10% according to these experiments, but that could also just be noise).

@easbar
Copy link
Owner Author

easbar commented Sep 13, 2021

That sounds unexpected for bike/car/walk. Most nodes are directed road segments, and from a quick glance at a reachability debugger, there are only a few roads not reachable:

Hm, strange. I might also check if the graphs are strongly connected. I agree that looking at your reachability debugger 25% of failed paths are unexpected.

The new default settings are indeed a huge performance boost.

I'm glad to hear this. Note that for your maps I think Params::new(0.01, 100, 10, 100) and ParamsWithOrder::new(100) work better than the defaults. I'll send a PR once this is merged.

For now, there's no recorded set of realistic user edits, but I'll keep an eye on the re-preparation times as I test things.

Yes, a set of realistic edits would be helpful to analyze this further for sure.

@easbar easbar merged commit 36b9fec into master Sep 13, 2021
@easbar easbar deleted the settled_nodes_limit branch September 13, 2021 06:14
@easbar easbar restored the settled_nodes_limit branch September 13, 2021 06:27
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.

Why does the preparation time vary so much for different maps?
2 participants