Make GeoJSONSource#setData faster #6738
Conversation
|
|
||
| const options: LoadGeoJSONParameters = extend({type: this.type}, this.workerOptions); | ||
| if (data) { | ||
| if (data !== undefined) { |
There was a problem hiding this comment.
We need to explicitly check against undefined because null is a valid GeoJSON value.
| for (const id of prevIds.values()) { | ||
| if (typeof id !== 'number' && this.promoteId == null) { | ||
| warnOnce(`GeoJSONSource "${this.id}": updateData is slower when using string GeoJSON feature IDs (e.g. "${id}"). Consider using promoteId or numeric IDs for better performance.`); | ||
| warnOnce(`GeoJSONSource "${this.id}": updateData is slower when using string GeoJSON feature IDs. Consider using promoteId or numeric IDs for better performance.`); |
There was a problem hiding this comment.
Providing an example ID in this warning message can cause log spew. warnOnce needs the warning message to be exactly the same across calls but the example ID could change in every call.
| // {} can be updated | ||
| if (data.type == null) { | ||
| return true; | ||
| } |
There was a problem hiding this comment.
We have a couple dozen unit tests that pass {} as a GeoJSON data value. This isn't technically valid per the spec but I chose to allow the value rather than change all the tests.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #6738 +/- ##
==========================================
+ Coverage 92.21% 92.23% +0.01%
==========================================
Files 285 285
Lines 23742 23735 -7
Branches 5050 5043 -7
==========================================
- Hits 21894 21891 -3
+ Misses 1848 1844 -4 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Looks good to me! |
3 participants
This PR brings some of the performance improvements we've added to
GeoJSONSource#updateDatain #4364 to theGeoJSONSource#setDatamethod.Specifically, it this PR removes the logic that passes the full serialized GeoJSON data from the worker thread to the main thread. In my benchmark, adding 100k points to a map, this makes
setDataabout 22% faster vs main.This changes some undocumented behavior of
Map#getStyle. Prior to this PR, if you add a GeoJSON source with a data URL, wait for it to load, and then callMap#getStyle, you'd get the actual loaded GeoJSON features back, not the original source spec with the URL. Similarly, if you calledsetDatawith some improperly wound GeoJSON geometries, and then calledMap#getStyle, you'd get the rewound geometries back. Even though this is undocumented behavior, I suspect at least a couple folks are relying on it. They will need to use theGeoJSONSource#getDatamethod instead.Launch Checklist
Include before/after visuals or gifs if this PR includes visual changes.Write tests for all new functionality.Document any changes to public APIs.CHANGELOG.mdunder the## mainsection.