Performance of style mutations not good enough for hover interactivity

[kristfal]

Issue

Style mutations are expensive, and can be quite slow on mobile devices. When applying new style mutations faster than the worker is able to process an ongoing mutation, the new mutation is queued and applied after the previous one has completed.

This happens even when the same style property in the same layer is changed multiple times such as setFilter('foo', [==, 'bar', 5]) followed by setFilter('foo', [==, 'bar', 3]) followed by setFilter('foo', [==, 'bar', 11]) and so on...

If this process is repeated too quickly for an ongoing period in a performance constrained environment (mobile devices), browsers start freezing and crashing due to memory issues.

Proposal: New style mutations should be debounced, and only the latest change should be applied for a given layer and property.

mapbox-gl-js version: earcut final rebase 20th of April

Steps to Trigger Behavior

1. Add layer foo to the map,
2. Run the map in a performance constrained environment (We're experiencing this on a range of devices around the performance level of iPhone5, Moto G2, Samsung S3)
3. Repeat style mutations quickly, for instance setFilter('foo', [==, 'bar', 5]) followed by setFilter('foo', [==, 'bar', 3]) followed by setFilter('foo', [==, 'bar', 11])..

Expected Behavior

The map should apply the latest style mutation for a given mutation type and layer, and skip any previous queued mutations for the given layer and type to prevent the buildup of a "mutation backlog".

Actual Behavior

The map queues and applies all mutations, quickly degrading the performance of the map and if mutations are continued, the browser crashes.

[jfirebaugh]

This is related to automatic batching. cc @mourner

[mourner]

Hmm, this is actually pretty surprising since the current code is batching style mutations to happen once per frame — you can see how that works in style.js.

Can you set up a minimal JSFiddle test case that reproduces the slowdown? That would make it easier to see what's going on in your case.

[lucaswoj]

@kristfal, are you asking for mutations to be deduplicated such that if 10 calls to setFilter are made during one batch, only the final setFilter call is executed?

[mourner]

@lucaswoj this is how it works currently, so there most be some catch here...

[filiphhh]

Here is a JSFiddle that should illustrate the problem.
JSFiddle. Try moving you mouse through the boxes and you'll see it leaving a long trail where it will retrace your mouse movements with highlighted graticule boxes. Furthermore if you'd listen to the mouse events you'll see that the callbacks execute directly clearly showing that the style updates are pushed to a queue.

[mourner]

Thanks for a great test case!

Furthermore if you'd listen to the mouse events you'll see that the callbacks execute directly clearly showing that the style updates are pushed to a queue.

They're not queued — they happen exactly once when you move from one cell to the other, and you can confirm this by putting console.log statements in key places such as style.js setFilter and worker update layers.

I found that the underlying problem is not batching updates, it's that even with proper batching, updating layers is too slow — on each such update, workers have to reparse all GeoJSON tiles in view and repopulate all buffers.

@ansis @lucaswoj @jfirebaugh I'm not sure how to approach this. It's looking like setFilter paradigm for hover interactivity just can't be made fast by design. What are the alternatives?

[mourner]

Here's how the worker profiler looks after a few hovers:

The perf warning is "not optimized: optimized too many times" (more info).

[filiphhh]

@mourner Not to push some other framework, but for the use cases where I've had problems with setFilter being too slow I've personally resorted to projecting the coords to pixel coords and creating an SVG layer for the feature on the fly in my mousemove callback with D3, a la https://bl.ocks.org/shimizu/5f4cee0fddc7a64b55a9. This is fast enough to be able to hover over the features without a noticable delay. While I am not recommending that as a solution, doing something similar may be an acceptable workaround (Drawing the feature to a canvas). If it would be possible to fire an event when the setFilter has finished updating all the layers one could choose not doing any new setFilters until the first one is finished which would at least prohibit the "queue" effect.

[averas]

@filiphhh I sometimes also fall back to d3/SVG, but in the case of mapbox-gl this has so far mostly been related to (lack of) dynamic styling. Couldn't you achieve a setup similar to what you describe with two separate sources? One for the backdrop and one that you update with only the features shown when hovering? In that case the expensive parsing @mourner mentions does not have to happen and you make full use of geospatial indexes for the hovering. The only expensive part in this approach is the setData() call on the second source, especially if the GeoJSON is large/complex, but I've used this approach with up to tens of thousands of features with reasonable performance, and as I understand more effective means of setData() may be on the way.

I have several use cases involving filtering where I use this technique instead of setFilter(), such as:

[averas]

@filiphhh I forked your fiddle and updated it with the approach I suggested.. and yes.. the performance gain is huge..

https://jsfiddle.net/oyjLzfq4/1/

[mourner]

@averas whoa, that looks fantastic — thank you for chiming in!

It seems that there's nothing immediately actionable in this ticket besides general optimizations of tile parsing, so I'm going to close this while keeping a very similar ticket open: #2443

[andrewharvey]

I ran into this issue today originally using the setFilter approach but now swapping this out with the approach from @averas at #2492 (comment). It's still not perfect (the hovered feature lags behind the cursor on slow machines or if you move the cursor really fast, or with many tiny polygons), but it seems better than the setFilter approach.

Perhaps the example at https://www.mapbox.com/mapbox-gl-js/example/hover-styles/ should be changed?

EDIT: I think I spoke too soon. The GeoJSON setData approach doesn't really work at tile boundaries because queryRenderedFeatures geometry objects are clipped to tile boundaries.

[andrewharvey]

EDIT: I think I spoke too soon. The GeoJSON setData approach doesn't really work at tile boundaries because queryRenderedFeatures geometry objects are clipped to tile boundaries.

The can be worked around using map.querySourceFeatures to try to collect all the tile clipped geometries for the feature and using turf.union to join them back together. A bit of a hack, but it mostly works.

[Scarysize]

Would be nice to add a section into the docs for @averas solution. This issue is buried pretty deep here, though this is a relatively common problem.

[lucaswoj]

@Scarysize What do you think the best way to surface this would be? We've got the docs, examples, and guides. We can also fold this use case into the core API #200

[Scarysize]

We did have the issue in an situation exactly like this: https://www.mapbox.com/mapbox-gl-js/example/hover-styles/ and additionally with a point overlay (geojson) with ~5k points. I guess the latter one is the more extreme, so I would recommend an example with a lot of point features which can be hovered.

EDIT: Adding this feature to the core API would also be much appreciated 😃