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

Improve performance of symbol placement with globe #12105

Merged
merged 7 commits into from
Sep 28, 2022

Conversation

karimnaaji
Copy link
Contributor

When globe is enabled, we use a mock DEM data source cache as a placeholder to leverage the draping architecture (in order to capture tiled texture to render as raster).

For most cases, globe is enabled without terrain exaggeration and makes use of this mocked mode by default. This PR reduces unnecessary workload related to DEM sampling under this code path, which improves placement time by up to ~20ms. This will have the effect of running placement more often as this is a task deferred across multiple frames using pauseable placement.

Another useful metric could to be count how many times placement has been run across a fixture in benchmap.

Full benchmark fixture run can be seen at: https://sites.mapbox.com/benchmap-js-results/runs/834

Screenshot from 2022-07-21 13-08-35

Diagnostic metrics summary

sites mapbox com_benchmap-js-results_runs_834 (1)

Launch Checklist

  • briefly describe the changes in this PR
  • include before/after visuals or gifs if this PR includes visual changes
  • post benchmark scores
  • manually test the debug page
  • apply changelog label ('bug', 'feature', 'docs', etc) or use the label 'skip changelog'
  • add an entry inside this element for inclusion in the mapbox-gl-js changelog: <changelog>Improve symbol placement performance with globe projection</changelog>

Sorry, something went wrong.

@karimnaaji karimnaaji added the performance ⚡ Speed, stability, CPU usage, memory usage, or power usage label Jul 21, 2022
@karimnaaji karimnaaji self-assigned this Jul 21, 2022
@karimnaaji karimnaaji force-pushed the karim/noop-mock-source-cache branch from 838c545 to da4ff13 Compare September 26, 2022 10:24
@karimnaaji karimnaaji marked this pull request as ready for review September 26, 2022 10:24
@karimnaaji karimnaaji requested a review from a team as a code owner September 26, 2022 10:24
Copy link
Contributor

@SnailBones SnailBones left a comment

Choose a reason for hiding this comment

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

Nice work @karimnaaji! One small nit about code readability, otherwise it looks good to me!

@karimnaaji karimnaaji merged commit 7697762 into main Sep 28, 2022
@karimnaaji karimnaaji deleted the karim/noop-mock-source-cache branch September 28, 2022 10:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
performance ⚡ Speed, stability, CPU usage, memory usage, or power usage
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants