search

mapbox / mapbox-gl-js

Interactive, thoroughly customizable maps in the browser, powered by vector tiles and WebGL
https://docs.mapbox.com/mapbox-gl-js/
Other
11.17k stars 2.22k forks source link

Pass Feature state to workers for initial tile evaluation #7122

Open asheemmamoowala opened 6 years ago

asheemmamoowala commented 6 years ago

7097 reports multiple warnings while evaluating tiles with styles usingfeature-state expressions. Initial tile parsing is done without access to the feature state and this requires evaluating expressions once again and updating paint buffers when the tile is being prepared for rendering on the main thread.

This increases the TTFR when using setFeatureState and feature-state expressions on large data sets.

Example: https://bl.ocks.org/ryanbaumann/733ba99c5ca1d9d15259081b395e4b00/414e974f854c20c263edddc6c9cc934d0956a9ac

mourner commented 6 years ago

My thoughts so far on this are that it would be too expensive to pass the entire state to each worker for every tile? Especially because state may change between the first batch insert, and when the tile is actually being loaded.

One way I could see this working is storing feature state map-wide in each worker, similar to what we do with layer index. So we would broadcast any updates to feature state to all workers (in addition to updating paint buffers on the main thread), and then all the newly loaded tiles in the workers would use this state to populate its paint buffers. In the data join case like Ryan's above, this should increase time-to-first-render, but more than make up for it by making the map jank-free.

mourner commented 6 years ago

The biggest issue with the approach above is that syncing state between the main thread and the workers could get tricky.

shayke commented 5 years ago

Hello,

I've tried implementing this by myself like so:

  1. Save state on all workers by exposing source_state.js to worker.js.
  2. setFeatureState, removeFeatureState, coalesceChanges all broadcast a message to all workers to perform the same operation on their local state (using mapId + sourceId).

This works pretty well for non-existing tile loads but I still see performance issues which should probably handled differently:

  1. When a tile is loaded from TileCache I need to update its paint buffers on the main thread to be synced with current state. I wasn't sure exactly what's the correlation between that and the coalesceChanges. Any advice here would help!
  2. Tiles are still being re-parsed even though they never change (there is no different between tile loaded from the network and from the browser cache). Although this ticket will solve FTTR when zooming, we are still doing a lot of heavy work on the worker and the map will only be interactive (as in using hover + feature-state) after all workers have finished. So zoom if fast but the user has to wait for full load in order to hover on stuff. Any ideas?

Thanks

asheemmamoowala commented 5 years ago

@shayke Thanks for working on this. Are you working towards contributing a PR?

When a tile is loaded from TileCache I need to update its paint buffers on the main thread to be synced with current state. I wasn't sure exactly what's the correlation between that and the coalesceChanges. Any advice here would help!

When a tile is loaded from the TileCache, it gets the entire feature state re-applied to it her: https://github.com/mapbox/mapbox-gl-js/blob/a54bbfb1d3b4a0c504faad6491a5b6565c05e81f/src/source/source_cache.js#L648-L653

The coalesce changes method is used to get the deltas between frames and apply it to tiles that are already on screen. This reduces the amount of delta changes that need to be applied to the buffers.

There is a over-arching issue of synchronization here that will require a solution as well. In your approach, the worker would receive the feature state data (gen A) and apply it to the tile once it is parsed. Additional setFeatureState calls on the main thread will accumulate (gen B, gen C) and need to be applied to the tile when it is about to be rendered. This is currently done by coalesceChanges. In the case of tiles that received their initial state on the worker, the main thread needs to know what gen of feature state was already applied in order to compute a specific delta to update the tile right before display.

One way to do this would be to use immutable maps (similar to immutablejs) to represent feature state that can generate diffs/deltas quickly.

Tiles are still being re-parsed even though they never change (there is no different between tile loaded from the network and from the browser cache).

When are you seeing this behavior, or what is triggering it?

Although this ticket will solve FTTR when zooming, we are still doing a lot of heavy work on the worker and the map will only be interactive (as in using hover + feature-state) after all workers have finished. So zoom if fast but the user has to wait for full load in order to hover on stuff. Any ideas?

If the feature state data is small or not changing continuously, this should not be an issue. What kind of or size of feature state data are you testing with?

Even if your code is not ready, putting up an RFC with the proposed changes, or a PR with notes on how you plan to build this will help us provide direct feedback. Looking forward to seeing your work on this.