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

DynamicAtlas improvements: Multi-Atlas loading and hotspots #208

Merged
merged 6 commits into from
Aug 31, 2018

Conversation

matthieun
Copy link
Collaborator

@matthieun matthieun commented Aug 29, 2018

Description:

Atlas performance improvements:

  • When a MultiAtlas underpinning a DynamicAtlas is already built from the same set of shards, avoid re-loading the MultiAtlas.
  • Streamlined the lineContaining, edgeContaining and areaCovering methods in AbstractAtlas to be faster.
  • Removed code smells in BareAtlas

Potential Impact:

  • DynamicAtlas creation should be faster when working with indefinite expansion disabled, and deferred loading enabled, and expansion beyond the initial shard set is not necessary.
  • Raw Atlas way sectioning should be faster when assigning Raw Atlas Points to either Points or Nodes.

Unit Test Approach:

  • Added unit test for DynamicAtlas and MultiAtlas creation in DynamicAtlasPreemptiveLoadTest
  • AbstractAtlas changes covered by PackedAtlasTest.testLocationContaining

Test Results:

All unit/integration tests pass. Raw Atlas Way Sectioning is 9x faster on average.


In doubt: Contributing Guidelines

Copy link
Contributor

@MikeGost MikeGost left a comment

Choose a reason for hiding this comment

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

This is a great optimization, thanks @matthieun! I'm wondering if there's any way we can test this behavior. From the current implementation - that doesn't look possible.

Copy link
Contributor

@MikeGost MikeGost left a comment

Choose a reason for hiding this comment

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

Staying consistent with our offline conversation - let's add a field in the DynamicAtlas that can keep track of the number of MultiAtlases built, so we can debug and highlight the number of calls being made. We can then rely on this field (if we expose it) in a test to verify the optimization in this PR.

@matthieun matthieun changed the title DNM: Avoid re-loading Multi-Atlas twice when shard set is unchanged DNM: DynamicAtlas improvements: Multi-Atlas loading and hotspots Aug 31, 2018
@matthieun matthieun changed the title DNM: DynamicAtlas improvements: Multi-Atlas loading and hotspots DynamicAtlas improvements: Multi-Atlas loading and hotspots Aug 31, 2018
Copy link
Contributor

@MikeGost MikeGost left a comment

Choose a reason for hiding this comment

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

Great improvements and tests, thanks @matthieun!

@MikeGost MikeGost merged commit 80d2723 into osmlab:dev Aug 31, 2018
@matthieun matthieun deleted the dynamicatlas branch August 31, 2018 22:04
@matthieun matthieun added this to the 5.1.10 milestone Aug 31, 2018
@jklamer
Copy link
Contributor

jklamer commented Sep 4, 2018

👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants