Allow limiting number of concurrent vector tile requests to prevent exhaustion of browser resources#13247
Open
jobblefrobble wants to merge 36 commits intomapbox:mainfrom
Open
Allow limiting number of concurrent vector tile requests to prevent exhaustion of browser resources#13247jobblefrobble wants to merge 36 commits intomapbox:mainfrom
jobblefrobble wants to merge 36 commits intomapbox:mainfrom
Conversation
…lback for a request that has actually been cancelled
… make array buffer generation an injected prop for testing
…quest-queue Adding vector tile request queue
…-queued-vector-tiles Add tests for queueing vector tiles
Author
|
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 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Launch Checklist
@mapbox/map-design-team@mapbox/static-apisif this PR includes style spec API or visual changes.@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.
Issue is here
Fix
The PR introduces a request queue for the loading of vector tiles, following the pattern used for fetching images (
getImageinsrc/util/ajax.ts).The solution is made slightly more complex than the image fetching by the inclusion of existing deduplication logic in the vector tile fetching.
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 🙇