Use the reloadTile worker method if GeoJSON tile is already loaded

[ezheidtmann]

During a style update, the source is reloaded. Because GeoJSONSource always calls loadTile in the worker, this means that any style change causes all tile PBFs to be regenerated. This makes hover effects on geojson sources very slow.

This PR fixes the performance issues by using the reloadTile worker method, just as in VectorTileSource. I believe we don't need to worry about the loading state because the GeoJSONSource#loadVectorData calls its callback synchronously, so there's only two branches here.

These changes perform very well in my use case but is relatively untested.

Symptomatically related to issues discussed here: #2874

Thanks a lot, yall!

Launch Checklist

• briefly describe the changes in this PR
• write tests for all new functionality
• document any changes to public APIs
• post benchmark scores
• manually test the debug page

[ezheidtmann]

jsfiddle showing extreme slowness http://jsfiddle.net/vk2knag7/1/

[ezheidtmann]

There was a bug when setData is called -- tiles were not reset; just pushed one possible workaround. I don't know a lot about mapbox-gl-js internals so I bet there's a more elegant way to do it.

[ezheidtmann]

CI failed on lint, fixed and re-running.

[Scarysize]

Nice! We ran into this issue with a circle layer & using map.setFilter. We ended up with just using source.setData with the single hovered GeoJSON feature on the highlight layer.

[mourner]

This is great!

• Is there a way to add tests to make sure this doesn't regress?
• Are there any race conditions to be aware of when calling setData and reloading tiles at the same time?

[ezheidtmann]

Thanks!

Re races: I don't see an opportunity for a race between those two; the geojson index and the this.loaded cache are changed at the same time, so I would guess as long as there wasn't a race before, there isn't one now. But as I say, I'm not really familiar with this code.

Re tests: Not sure how to test this but hopefully others with more experience can fill this in.

[mourner]

Hey @ezheidtmann, let's work together on shipping this PR! Want to rebase against the current master as the first step?

[ezheidtmann]

Hey @mourner sorry for the long silence, I've been buried in other work during Bike Month. I rebased and it looks good; I can test later today that my app still has acceptable performance.

Re tests: To guard against regressions, we can test that reloadTile is called appropriately, or we can test that the overall performance is acceptable (hard?), or maybe there is something even better we could do?

[mourner]

@ezheidtmann great, no worries! Re tests, I think it's enough to make sure tile is reloaded. We can cover benchmarking separately later.

[ezheidtmann]

I wrote a test. It took me a lot of time. I hope it is not absurd or wrong.

[mourner]

Wow, that was indeed quite difficult! Thanks for taking the time to do that — highly appreciated. And sorry that it too so long.

[ezheidtmann]

👍 Thanks!