Mapbox-gl-js: Improve performance when interleaving sources in layers

[averas]

mapbox-gl-js version: 0.18.0

This is an upstream issue related to performance issues found in mapbox-gl-draw (https://github.com/mapbox/mapbox-gl-draw/issues/296). It seems adding layers referencing empty sources still has quite substantial impact on performance.

Steps to Trigger Behavior

1. Go to http://codepen.io/anon/pen/qZJprY
2. Zoom in/out and get a feeling for the performance
3. Click the "Add Layers" button and do the same
4. Click the "Remove Layers" button and see that performance is restored again
   

   Expected Behavior

That adding layers referencing empty sources ought to have minimal if any impact on performance.

Actual Behavior

Performance is degraded.

[mourner]

Confirmed. Let's investigate further.

[ansis]

My guess is that this is caused by redrawing the clipping mask for each source. We never check if a source has data that needs to be drawn in that pass before redrawing the mask.

https://github.com/mapbox/mapbox-gl-js/issues/1042 will fix this by making it so that there's only one clipping mask. @mollymerp is starting work on this so it probably doesn't make sense to optimize the current implementation.

[averas]

@ansis Could you elaborate a bit more? I haven't seen any trace of similar behaviour in any of my own projects, where I often have several (up to 10) GeoJSON sources, which at any given point in time some have data and some don't. Is it the two sources in combination with the (fairly) many layers referencing them? I'm trying to get an understanding for what the input variables are and how the performance degradation function looks .. i.e. is it linear per source? linear per source x 1 layer? linear per source x n layers?

[ansis]

I haven't seen any trace of similar behaviour in any of my own projects, where I often have several (up to 10) GeoJSON sources, which at any given point in time some have data and some don't.

It sounds like my guess might be wrong. We'll need to dig deeper

[averas]

Any news on this? My impression is that this is even more evident now with earcut, and it has quite a significant impact on mapbox-gl-draw, which simply by being added has a major performance impact on rendering. In my own application it goes all the way from smooth to quite painful..

You can try out 0.19.0 on an updated codepen at http://codepen.io/anon/pen/MeaXGb
(Add/remove layers with empty source using the top left buttons)

[mourner]

@averas I think the initial guess by @ansis was right — profiling the codepen above and comparing profiles for a clear state and the one after clicking "add layers" many times, the only noticeable difference is _renderTileClippingMasks taking a ton of time on the main thread. I'm investigating deeper.

[averas]

@mourner Alright. If I get this right, could one first simple remedy be to just exclude any empty non-dynamic sources (such as GeoJSON) from the rendering cycle altogether?

[mourner]

@averas In addition to excluding anything that's empty, avoid interleaving layers of different sources at all costs. In this case, all layers go in the order cold-hot-cold-hot-cold-cold-hot-.....

The way the current rendering works, it groups adjacent layers with the same source, and renders clipping masks for each group. So e.g. when you have Streets with a single source, it collapses to basically one clipped rendering group (in addition to the background layer). Adding the Draw layers, this becomes 31 groups, each rendering the same clipping masks over and over.

@mcwhittemore refactoring this in GL Draw may lead to a huge rendering speedup, so I'm reopening https://github.com/mapbox/mapbox-gl-draw/issues/296 to explore this.

[mourner]

@lucaswoj @ansis @mollymerp while this can be fixed on the GL Draw side, what I described above is a big performance gotcha, and we should address this soon. Two options are:

• Wait for the full tile clipping refactor that goes as a part of #1042 @mollymerp but I'm not sure what the timeline is for this — could take a while
• Attempt a smaller refactor that would at least eliminate drawing of indentical clipping masks over an over — maybe caching them _per frame_ (by source maxzoom / coord id key / extent key) rather than _per group_.

What do you think?

[averas]

Great research @mourner, thanks!

[lucaswoj]

Attempt a smaller refactor that would at least eliminate drawing of indentical clipping masks over an over — maybe caching them per frame (by source maxzoom / coord id key / extent key) rather than per group.

This sounds like a relatively small lift. I'd love for us to look into it when there's time!

[mourner]

@lucaswoj let me take a stab at this.

[mourner]

@lucaswoj I attempted the "smaller" refactor, but after chat with @kkaefer, it turns out much trickier than I initially thought.

@kkaefer: we do that in native: we first collect all unique combinations and then assign IDs
then paint the entire mask, then draw all of the layers
but it has the issue of overflowing the 8 bit limit for the stencil buffer in some situations
@mourner: combinations of tile coordinate + source maxzoom, right?
@kkaefer: yes, and what child tiles/parent tiles are loaded in the source to cover up blank spots
https://github.com/mapbox/mapbox-gl-native/issues/962
the algorithm is in https://github.com/mapbox/mapbox-gl-native/blob/master/src/mbgl/algorithm/generate_clip_ids_impl.hpp
but this approach has issues; I’ve been thinking about switching to the JS model for native as well
ultimately, we shouldn’t be doing clipping at all
@mourner: is the main issue in the native approach the 8 bit overflow, or anything else?
and how often does that happen?
@kkaefer: it typically happens during loading, when you have a lot of overlapping tiles loaded, e.g. 3 out of 4 child tiles, so you have to allocate ids for the three children, and the parent
and the larger the rendering viewport gets, the more pronounced the issue is

[mourner]

1042 will fix this by making it so that there's only one clipping mask.

@ansis do you have a chance to describe how that would work exactly?

[lucaswoj]

From the rendering perspective, you wouldn't have a separate tile from each source. You would have a single tile with the data from all the sources.

The idea behind #1042 is that we will end up with a single Tile per coordinate (rather than a Tile per coordinate per source). A single Tile = a single clipping mask.

[timiyay]

Not sure where this at, but I wanted to vote for it making the to-do list.

The related mapbox-gl-draw bugs are blocking us from upgrading mapbox-gl-js. Upgrading draw requires upgrading gl, but the performance hits are huge:

https://github.com/mapbox/mapbox-gl-draw/issues/296
https://github.com/mapbox/mapbox-gl-draw/issues/329

We're seeing >= 5s draw times when toggling layers, even when no drawing activity has taken place.mapbox-gl-draw appears to have done all they can, and are blocked by this issue.

We're stuck on a specific SHA for mapbox-gl-js (mapbox-gl: git://github.com/mapbox/mapbox-gl-js.git#9369d5), as we need the earcut improvements to fix some gnarly rendering bugs our clients were getting: https://github.com/mapbox/mapbox-gl-js/issues/2300.

We can't upgrade to a release that includes earcut, as our version of mapbox-gl-draw doesn't support anything beyond 0.14.x or so. We can't upgrade mapbox-gl-draw until this draw-related bug is fixed.

https://github.com/mapbox/mapbox-gl-draw/issues/282
https://github.com/mapbox/mapbox-gl-js/issues/2401

Dependency 😈 .

[lucaswoj]

We're seeing >= 5s draw times

Oy vey! That is very slow. Are you sure the slowdown is related to interleaving sources? We'll need to do a little more investigation to figure out what's happening.

A fix for this issue is #1042, which is on our radar but not yet scheduled for any particular release.

[timiyay]

@lucaswoj I'm not 100% that it relates to interleaving sources, but I am 100% that the performance hit is caused by including mapbox-gl-draw. I followed the breadcrumb trail of GH issues to my conclusions.

I'm happy to do the work to provide more detailed evidence. Do you have any references for the kind of profiling you need to help shed light on the issue? My profiling-fu is low...

[lucaswoj]

@timiyay A Chrome CPU profile would likely be the most useful. If the results don't point to interleaving sources, please open a new Github issue.

[timiyay]

OK will do. Interestingly, I'm unable to replicate on a different machine (Macbook Air). Will do profiles on the MBA, and the 2010 Macbook Pro that showed the original issue.

[mourner]

I feel like we can close this since it has been addressed upstream in mapbox-gl-draw, and it's a pretty uncommon issue otherwise, without a clear way to fix it.