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

Address rendering performance regressions after DDS #3342

Closed
mourner opened this issue Oct 11, 2016 · 2 comments · Fixed by #3489
Closed

Address rendering performance regressions after DDS #3342

mourner opened this issue Oct 11, 2016 · 2 comments · Fixed by #3489
Assignees
Labels
performance ⚡ Speed, stability, CPU usage, memory usage, or power usage

Comments

@mourner
Copy link
Member

mourner commented Oct 11, 2016

mapbox-gl-js version: v0.23+

Digging into rendering performance, I discovered that #2938 regressed it more severely than was demonstrated by the benchmarks, which averages frame duration with too many "easy" zooms.

With a simple isolated benchmark that renders a 1440x900 map of Mapbox Streets on z13 (the most detailed one) in DC, CPU duration of a frame is almost 2 times bigger in v0.23 compared to v0.22.

This leads to a huge increase in dropped frames when panning and zooming, especially for full-screen maps of cities. The effect should also be much more pronounced on weaker machines (I'm testing on my MBP).

Other DDS PRs regress performance too, but to a lesser extent because CPU performance is line-dominated according to the Chrome profiler.

I think that we need to prioritize addressing this technical debt now, before it grows out of control with more DDS properties support.

There are several things that seem to be contributing to the regression:

  • bucket.setUniforms does a full recalculation of DDS-enabled properties (getPaintValue), even though they don't differ from values in layer.paint (that are recalculated on every frame too). Replacing uniform.getValue(...) with layer.paint[uniform.paintProperty] generally works, with a few glitches to fix (fallback values like fill-outline-color falling back to fill-color are not calculated during the main style recalculation phase).
  • The big block of uniform sets and useProgram in draw_line that come before painter.enableTileClippingMask(coord); now execute individually for each tile instead of once for a layer, although in non-DDS case the program options are the same (and maybe in case of DDS as well — I'm not sure). We need to refactor this code so that it is back to being executed once per-layer.

@lucaswoj I'll need your help to tackle this as I don't fully understand all the quirks of DDS code/vision yet, but I'm sure we can go back to v0.22 performance for Mapbox styles while keeping the DDS features.
cc @jfirebaugh @mollymerp

@mourner mourner added performance ⚡ Speed, stability, CPU usage, memory usage, or power usage high priority labels Oct 11, 2016
@jfirebaugh
Copy link
Contributor

On the first point, it sounds like we need to bite off #3044 and refactor so that we have a single canonical representation of paint values that can be updated once per frame (or less if possible) and then shared by bucket.setUniforms and others.

@mourner
Copy link
Member Author

mourner commented Oct 11, 2016

refactor so that we have a single canonical representation of paint values that can be updated once per frame (or less if possible)

@jfirebaugh I made such a refactor before DDS — it's intended to be canonical and calculated once per frame max; just need to fix the new DDS code to conform to this intention.

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 a pull request may close this issue.

3 participants