mapbox-gl-js
mapbox-gl-js copied to clipboard
Allow limiting number of concurrent vector tile requests to prevent exhaustion of browser resources
Launch Checklist
- [x] Make sure the PR title is descriptive and preferably reflects the change from the user's perspective.
- [x] Add additional detail and context in the PR description (with screenshots/videos if there are visual changes).
- [x] Manually test the debug page.
- [x] Write tests for all new functionality and make sure the CI checks pass.
- [ ] Document any changes to public APIs. (Pending the actual config changes)
- [ ] Post benchmark scores if the change could affect performance.
- [ ] Tag
@mapbox/map-design-team@mapbox/static-apisif this PR includes style spec API or visual changes. - [ ] Tag
@mapbox/gl-nativeif this PR includes shader changes or needs a native port.
Issue
This PR is to resolve an issue I raised around high numbers of vector tile sources on the map causing browser resources to be exhausted. Causing some tile requests to fail to be sent, and also any other concurrent requests from the browser to be stopped too.
Fix
-
The PR introduces a request queue for the loading of vector tiles, following the pattern used for fetching images (
getImageinsrc/util/ajax.ts).- Requests will continue to be made, up until the number of concurrent requests becomes greater than our limit
- After which subsequent requests are attached to the queue
- When a request resolves, we advance the queue and make the next request
- When all the callbacks for a particular request have been cancelled we remove the request from the queue
-
The solution is made slightly more complex than the image fetching by the inclusion of existing deduplication logic in the vector tile fetching.
- A unit test has been added to make sure request deduping is still working as expected
- Unit tests for the queueing behaviour have also been added
Help needed
Currently the queue limit is hardcoded to 50.
I had intended to set this as global config in a similar way to the image fetching mapboxgl.maxParallelImageRequests = 10 - however because the vector tile fetching mostly happens inside workers, it seemed like that the user-supplied config value was not being picked up.
If there's an established pattern for injecting config into the workers I would like to follow that! Otherwise any general guidance would be much appreciated 🙇
@stepankuzmin
Hi! Apologies for the direct tag but was unsure how to proceed, and you happened to have interacted with the original feature request for this. Please let me know if there's someone more appropriate to take a look at this 🙂
Since merging in the latest changes from main I'm seeing some test failures in the render tests (all tests were previously passing). I'm not very familiar with the render test setup, so if you've got any guidance around how to debug these failures that would be much appreciated! 🙏
It seems like the linux one failure is actually just a failure to download firefox, but less sure about the safari tests