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

Slow performance with lots of empty labels #7973

Closed
aMoniker opened this issue Feb 28, 2019 · 12 comments · Fixed by #12669
Closed

Slow performance with lots of empty labels #7973

aMoniker opened this issue Feb 28, 2019 · 12 comments · Fixed by #12669
Labels
performance ⚡ Speed, stability, CPU usage, memory usage, or power usage

Comments

@aMoniker
Copy link

mapbox-gl-js version: 0.50.0

browser: Chrome 74.0.3717.0

Steps to Trigger Behavior

  1. Create an area with 100k+ labels, each one an empty string. Ours came from a vector tile source.
  2. Render the labels in a Symbol style layer (even though nothing will appear)
  3. Zoom in and out of the area with all the labels quickly.

Expected Behavior

Works fine. Performant even, because no labels are rendering.

Actual Behavior

Execution thread locks up and browser becomes unresponsive for several seconds. A performance analysis with Chrome devtools reveals that the findMatches function in cross_tile_symbol_index.js is taking up most of the execution time:

screen shot 2019-02-28 at 15 58 08

I haven't read through enough of cross_tile_symbol_index to find out exactly why, but the comment here makes me guess that empty labels are causing indexes to be clobbered, resulting in lots of unnecessary recalculations.

@mourner mourner added the performance ⚡ Speed, stability, CPU usage, memory usage, or power usage label Mar 1, 2019
@peterqliu
Copy link
Contributor

@aMoniker thanks for cutting the issue. To ask the obvious, is this a use case we should spend time optimizing for? If we already know there's no text to render, it would make more sense to delay adding the layer.

@aMoniker
Copy link
Author

aMoniker commented Mar 5, 2019

@peterqliu Maybe. In our case we were controlling the visibility of labels in one layer by having our tiles server return empty strings for certain features depending on the zoom level.

We'll be handling this on the client side due to these performance issues for now, but it may be something we'd like to do again in the future. It's inconvenient for us since we're using multiple clients (iOS & GL-JS) and have to implement that logic twice (and keep both versions in sync).

@peterqliu
Copy link
Contributor

peterqliu commented Mar 6, 2019

It might also be worth filtering out all features with empty strings for that parameter. Could short-circuit a lot of unnecessary work depending on where the bottleneck is

@aMoniker
Copy link
Author

aMoniker commented Mar 6, 2019

That's a good idea. We'll try it if we end up going that route again.

@robhawkes
Copy link

I've been hitting this issue lately. I don't believe it's isolated to empty labels as in my case I'm outputting many thousands of symbols that have values for text-field and the performance is still really bad. The culprit is TileLayerIndex.findMatches() inside of CrossTileSymbolIndex – this method is running often and very slowly.

In an effort to debug this I tried setting the same label for all symbols and allowed overlap and ignore placement:

'layout': {
  'symbol-z-order': 'source',
  'text-field': 'Hello',
  'text-allow-overlap': true,
  'text-ignore-placement': true
}

I hoped this might bypass whatever CrossTileSymbolIndex is used for but unfortunately this wasn't the case as the performance didn't change.

In my case I need the ability to output many thousands of dense, overlapping symbols as – from what I can see – there's no other way to output a layer with simple shapes as icons (eg. like the circle layer, but triangles or squares). I'd love to find a way to either disable this aspect of the symbol layer or stop it taking so much time. I don't need anything fancy with symbols, just custom icons that otherwise behave like the circle layer.

Is there anything I can try?

@mourner
Copy link
Member

mourner commented Mar 19, 2019

@robhawkes nice lead! Can you set up a minimal live example with the problem exaggerated? I'll look into it.

@robhawkes
Copy link

@mourner I've finally found some time to put together a small test-case that shows the performance problem using a symbol layer with numerous features.

https://jsfiddle.net/robhawkes/9uavc0Lp/

If you load up that example and then record a performance profile in Chrome while zooming and panning then you should see something like the following:

Screen Shot 2019-04-01 at 14 22 11

For some reason the example in JSFiddle is pointing to a different slow-down, however it's in the same parent method as the one described above (Style._updatePlacement) so perhaps it's related. Either way, it's not fast.

It's worth pointing out that this example uses a GeoJSON source, whereas our real-world issue uses a vector-tile endpoint (that we host on Mapbox).

Let me know if I can try anything else to help debug or attempt to by-pass the issue.

For comparison, this is the same example but using a circle layer instead – while not expecting them to be identical performance, I would expect symbol layers not to be 25–50x slower for the same number of features:
Screen Shot 2019-04-01 at 14 28 34

@robhawkes
Copy link

I was also able to replicate the original issue with TileLayerIndex.findMatches() by zooming in quite far, panning around (perhaps not necessary) and then zooming out. The map will hang for quite some time and then eventually catch up.

Screen Shot 2019-04-01 at 14 40 05

@mourner
Copy link
Member

mourner commented Apr 2, 2019

@robhawkes thanks for the details! We'll see if there's anything to optimize there, although it will be tough.

For comparison, this is the same example but using a circle layer instead – while not expecting them to be identical performance, I would expect symbol layers not to be 25–50x slower for the same number of features:

Symbol layers are much more difficult to render — we have to transfer the geometry to the main thread, perform global collision detection across different tiles, and update the buffers continuously on every frame. Switching to the new approach significantly improved label density, reliability of collision detection, fade animations of labels, fixed bugs with collisions around tile edges, and enabled collision across multiple sources (see #5150 for a part of this work). It was fine perfomance-wise for the usual use case (e.g. Mapbox basemaps), but it's a trade-off and certainly introduced performance issues for some edge cases. I hope there's still room for improvement though.

@dollysingh3192
Copy link

Hi Mapbox Team,

Are there any plan of improvement to be handled in near future. I am also facing the same issue.

Screen Shot 2020-09-15 at 8 03 28 PM

@benrudolph
Copy link

benrudolph commented Aug 26, 2021

We are hitting this issue pretty hard as well; we have a bunch of logic to remove symbol layers that aren't strictly necessary.

Symbol layers are much more difficult to render — we have to transfer the geometry to the main thread, perform global collision detection across different tiles, and update the buffers continuously on every frame.

@mourner That makes sense and sounds tough. Is there a way to let mapbox know that we don't care about collision detection? We've tried setting icon-allow-overlap, but that hasn't seemed to make a difference. In our case, it's much more important that the map is performant and the labels render than if the map is slow and the labels don't collide

@mourner
Copy link
Member

mourner commented Apr 19, 2023

Fix coming in #12669 — sorry for getting to this so late!

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.

6 participants