Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add caching in Map for loaded tiles in TileLayer to improve performance and reduce requests. #1931

Closed
wants to merge 1 commit into from

Conversation

ReinisSprogis
Copy link
Contributor

By trying to resolve flash on seamless scroll, I had an idea to try load tile images faster by caching them in Map<String,Uint8List> where key is (x:y:z) and Uint8List is image data. Then before requesting tiles from tile provider, I check if key exists and if it does, then I load it from there skipping tile request all together. It seems like it is faster to look up in Map, than in disc cache, but regarding that more tests need to be done.
One thing I certainly noticed improvement on was on tile requests. While looking at network requests, it did request images from (disc cache), that was loaded previously, but a lot of them still was requested from server repeatedly, at least so appears when zooming in and out or panning. But with Map approach there are no requests if the same tiles are loaded.
Well this was just an accidental experiment, see what you get out of it and maybe it has some potential.

In video you can see comparison on request. This is zooming in second time (after already loaded images once).
As you can see in current version requests are made some from disk cache, some from server even once already loaded. While loading from Map, there are no requests.
https://www.youtube.com/watch?v=r4ub9n4H_LA

Here you can see a comparison between both versions on zoom. As you can see storing Images in Map does load images faster. As there are less lower resolution tiles showing and less visible gray tiles. However on original version, those low resolution tiles and gray tiles could be explained by some of the tile requests on server not stored in (disc cache)?
https://www.youtube.com/watch?v=Y3zLNNlTQms

…reduce redundant tile loading. The cached tiles are stored in a Map where the key is the tile coordinates (x:y:z) and the value is the tile image as a byte list. This allows for quick retrieval of already loaded tiles, avoiding unnecessary network requests. The caching logic is implemented in the `_TileLayerState` class using a `Map<String, Uint8List>` called `_cachedTiles`. Each time a tile is loaded, it is added to the cache using its coordinates as the key. When a tile is requested, the cache is checked first before initiating a new load. This change improves the efficiency of the TileLayer component and reducing requests.
@ReinisSprogis
Copy link
Contributor Author

Now when tested on windows app, While zooming in and out without using Map<String,Uint8List> caching, I got error

flutter: DioException [bad response]: This exception was thrown because the response has a status code of 429 and RequestOptions.validateStatus was configured to throw for this status code.
flutter: The status code of 429 has the following meaning: "Client error - the request contains bad syntax or cannot be fulfilled"
flutter: Read more about status codes at https://developer.mozilla.org/en-US/docs/Web/HTTP/Status
flutter: In order to resolve this exception you typically have either to verify and fix your request code or you have to fix the server code.

Guessing too many requests? But I went through those zoom levels multiple times already and they are already loaded, so it shouldn't be sending any new requests.
Also I noticed that without Map<String,Uint8List> caching sometimes map get stuck on gray screen until gesture is done to load tiles. Something I didn't observe using Map<String,Uint8List> caching.

I'm just thinking how many images can be added to that Map<String,Uint8List> before it could start causing memory issues. Guessing, but tens of thousands should be alright in terms of search time for key. However if it gets too big, potentially could run out of memory. Maybe some limit could be introduced to avoid this.

@JaffaKetchup
Copy link
Member

We already integrate with the native Flutter session caching mechanism ImageCache. We can verify this behaviour by analysing network requests, but wherever possible, it should already be OK.

@ReinisSprogis
Copy link
Contributor Author

Does it need to be enabled? I suppose this can be closed then. But give it a try, it seems like there is an improvement.

@JaffaKetchup
Copy link
Member

Since v6 (#1629), the internal ImageProvider implementation provides a key (itself) and implements a valid (AFAIK) equality and hash code for the key (on itself), based on the URL.

@override
SynchronousFuture<MapNetworkImageProvider> obtainKey(
ImageConfiguration configuration,
) =>
SynchronousFuture(this);
@override
bool operator ==(Object other) =>
identical(this, other) ||
(other is MapNetworkImageProvider &&
fallbackUrl == null &&
url == other.url);
@override
int get hashCode =>
Object.hashAll([url, if (fallbackUrl != null) fallbackUrl]);

AFAIK this should be enough (which I think is why the tiles do load again so quickly - but there is some delay).

I'm a little reluctant to introduce another level of cache on top.

We can test whether the current one is working by breaking the equality/hash code, and checking whether performance has gotten worse.

@JaffaKetchup
Copy link
Member

I'm going to close this for now, as I believe we already do enough caching, and I don't want to add another layer without evidence that the current layer is broken. If it is significantly faster, then please provide some benchmarks/testing videos, and I'll re-open this. Thanks for the work so far!

@timur-harin

This comment has been minimized.

@timur-harin

This comment has been minimized.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants