feat: add built-in caching #2082
Conversation
Fixed bugs in persistent registry writer Added cache size limiter Added `MapCachingOptions` to configure caching externally Minor other renaming & refactoring
Added `MapCachingOptions.overrideFreshAge`
Improved performance by moving tile file writing into long-lived isolate worker to reduce overheads
Renamed `CachedTileInformation` to `CachedMapTileMetadata` Improved documentation
|
Hey @JaffaKetchup, since you asked for feedback about this, I'll try and add some thoughts here. Disclaimer: I read through the code at a high level, not in fine details, so apologies if I missed some things there. I'm not sure why there is a concept of "loading" for the caching mechanism, is it because we need to read an index JSON file first? I find that option to not be great because the JSON will grow larger and larger as the user has more and more tiles, and as you pointed out, might get corrupted, and I'm hoping we don't need that, and I'm hoping we can build this in a way that doesn't have an initialization delay by using the file system directly. Do we need to store anything more than those pieces of information for a given cached tile (I'm hoping not)?
I've had to tackle a similar problem in my app before because I implemented offline maps which are expensive to process (it means generating a PNG image from vector data), so I cache offline built tiles so I don't have to reprocess them later. I have implemented a simple approach where I cache tiles based on file named Here are some ideas I hope would help this work better:
I might not have the full context here, so this might be a naive approach that doesn't work for some reason, but I hope this helps :-) |
|
Hey @androidseb, thanks for your thoughts :)
We need to store:
So unfortunately some form of additional storage is required for the caching headers
This might not work well. If a user is using two different tile servers with the same coords, we need to differentiate that. We could create a parent directory for each URL, but I'm not sure I can see the value.
I'm using whatever directory is returned by path_provider's
I did consider this kind of storage mechanism, but how would you store the expiry time? In another file, one per tile? I want this to cache based off HTTP headers primarily, to be compliant with the OSM requirements for example.
Would this be in an isolate? I've tried to minimise I/O on main thread (see below). But yeah, the method I've written for enforcing the max cache size would be much better if it didn't delay the caching - this is difficult to do performantly however with the current setup. Maybe a change in setup would better allow this.
The idea is to eliminate some of the overheads. Although you've suggested removing the registry file, in its current form, opening, writing and closing the file for every single update would be quite costly. The async methods do unblock the main thread, but come with their own overheads. Using long-lived isolates avoids some of this. I'm not sure whether the tile file writer isolate makes a noticeable difference, but it seemed to have some good effect (not properly profiled), and theoretically it should avoid some overheads. I'm definitely on board with looking into ways to remove the registry, but I just don't see a great way around it to store all of the required metadata - except maybe using two files per tile, but that introduces additional overheads as a file needs to be read before the initial caching decision at fetch time can be made (rather than just reading from memory). Let me know what you think! |
|
Thanks, I think I understand this a little better now. About storage
About using files directly
I agree splitting URLs per directory will make things difficult, but we could maybe figure out a unique file naming function for that, even if it means including a file-name-friendly version of the URL in the file name, I don't think it would necessarily be an issue. About the android cache folder
Oh OK, good, I had missed that, my comment is not relevant then. About cache expiry based on headers
Yes, having a About I/O
What do you mean by costly? More costly than fetching the data from the web with an HTTP request over the network? The tiles are fairly small and light in volume (~50kb for a 256x256 tile) - I can't imagine fetching cached tile for the entire screen of a device would exceed 10MB of reading on the disk, and that would almost always be faster than re-fetching them from the network anyways, right?
What kind of overhead are you thinking about? I'm not super knowledgeable about these things so I'd be curious to have insights about this. I have a hunch that isolates might have noticeable value in intensive I/O operations, but given the size and number of the tiles we're dealing with here, I feel like it will not make a difference for the user, especially if you compare it with network-based requests. It might be worth keeping them for later if needed, instead of preemptively making them part of the architecture, but it's just a thought.
I think using two files could work. I don't see this introducing overhead if the in-memory cache is building itself in the background from the files. What I mean by that is that you could implement your cache controller entity so that:
With this approach, you might fetch cached files information from the disk when needing them, but that disk read was going to happen at some point anyways during the indexing. If you have a cache of 10k files, it might mean that on cache initialization, you read the cache info from 10k files, which is really not ideal... An alternative approach to this could be to lazy-access the cached files so you only read / discard them when attempting to fetch the cache entry. To keep track of the cache size without having to scan all files, you could have a unique file storing the total size value, and that would only be a few bytes. A few more thoughts about this
|
|
Just to be clear, you're suggesting removing the persistent registry? I'm happy with this in theory, but in practise, I think it could be a little more difficult. I think having the single registry is better in this respect - reading 10k files (which would only cover 5k tiles) doesn't seem like the best option. If we reduce the duration of the size limiter (see below), we can bring the initialisation time down significantly. We can also provide an optional 'preload' method to be called in
I'm not quite sure what you mean by this exactly, could you explain a little more? If you mean we just check the metadata file for every tile load, that is an option, but it would make tile loading much slower.
This sounds like a good idea. The size limiter is by far the largest slow down in initialisation. But if we are over the limit, ordering the tiles by size and last modified is also really slow. Is there a better way to do this? Is there any way we can get the best of both worlds? We don't need to be too careful with RAM particularly IMO. Is there a possibility of somehow using both a centralised registry and decentralised metadata file structure - or is that asking for too much (I feel like it might be)? In terms of file names, why is a hash (or any other generation function) not good? It means no worrying about particular URLs or coordinates. I would propose the file names
I was referring only to writing to the persistent registry. Opening it for each update would be unnecessary.
From my (admittedly non-profiled) experience when writing FMTC, putting I/O in a separate thread definitely reduced jank. Also https://dart.dev/tools/linter-rules/avoid_slow_async_io exists - although I will admit I thought it applied to all synchronous I/O methods, not just the listed ones. See also dart-lang/sdk#19156. One thing that's more difficult when dealing with this kind of traffic is that the tile updates are relatively small each (but not totally insignificant), but there's a lot of them and they come in very quick bursts. If you've got an open HTTP connection, making a bunch of tile requests to fill the screen will result in a burst of responses.
Yes, or by any other server. The OSM ones in particular seem a little strange (serving tiles repeatedly that have already expired), but that's an issue at the source.
If we keep using a registry, then yeah, we don't want to update it for each individual tile (especially if we're using JSON as we need to write and flush the whole file each time). My initial idea was to write it and queue up any updates that occur whilst writing, then write again, but I couldn't get this working well, so I just resorted to a 50ms debounce. |
Yes, or at least I'm hoping we can achieve this somehow. But maybe there is no good way around it...
Yes that's what I mean. I understand this is "much slower" than direct RAM access, but we're still talking about sub-16-milliseconds frame time to even be noticeable (even in the bursts you mentioned?) by the user hopefully? I/O being "slow" is not a problem if the user can't notice. It isn't "slow" as in "consuming more CPU + battery" slow, it's slow as in "extra I/O idle delays" slow.
Yeah getting the best of both worlds would be good 😄. I think the top priority here should be avoid sending extra HTTP requests when we have the cache. The long initialization time is an incentive to skip the cache at the beginning, but that's unfortunately the moment where the user gets the most tiles (the entire screen), and at scale, this implementation would mean we're effectively not minimizing (only reducing) the out of the box impact of flutter_map on the OSM servers. To be clear, my suggestion of the direct file system approach was primarily driven by the intent to get around the initialization time, because I feel like we should not skip the cache, and I would like to find a solution where the user doesn't have to wait several seconds before seeing tiles. At the very least, the hybrid approach could have the long initialization time, but during that time, instead of using direct network requests as a backup, we could use direct file system requests as a backup, because we know where the tile cached file will be located?
That's fine, I think hashes for file names would work too. I thought having a file name structure where you can trace back which tile a specific file is linked to just from its name could be useful, but I can't come up with any concrete use case, so it's probably not useful at the moment.
Interesting, I wonder if this is related to the debugging setup specifically.
I think it depends what you mean by "slow". I may be mistaken, but the way I understand it, there are two kinds of "slow" we're looking in this context:
For example, the link you shared mentions "Reading files asynchronously is 40% slower", but that's the total round-trip time for a single operation, because that operation is waiting a lot for things to come back between threads. If you were running 10 of those operations at the same time, they would probably not take 10x the time to execute, probably closer to the same time as a single operation. Because most of the time needed to perform the operation is waiting between threads. In our specific setup with cache, the file operations I'm talking about are all affected by "I/O wait slowness", so fetching a single tile's data might take 150ms, but completing a quick burst of 100 of these operations running in parallel might not take much longer than 200ms to complete... And I'm using 150ms as an example, but it's probably less, and most certainly faster than a network-based request. Don't take my word for it, we should look into it, but I'm just saying it could be worth considering.
For the registry serialization, there might be good libraries to handle updates more efficiently, I've used sqflite in my project, but I feel like adding an SQL dependency to flutter_map for this is overkill. Maybe hive could be an interesting choice, even if it's to look at the code to find out how it's implemented. One more note: it looks like you're abstracting the cache implementation so we can write our own implementation of it, I think that's good, because it will make contributions to alternate cache-focused plugins (those that need extra dependencies) easier down the line. |
|
@androidseb So just because I'm not certain what we're suggesting exactly, we're suggesting using both a registry and an individual metadata file for each tile? I know that's perhaps what I was suggesting, but I've had a think about it, and that sounds really difficult to manage. This would be my new suggestion:
How does that sound to you (and @fleaflet/maintainers)? Basically, if the user doesn't call to preload, the delay is just the time taken to parse the registry. In terms of the registry format, I wanted to keep things as JSON because it's so simple. But maybe using an actual database is a good idea. I absolutely do not want to introduce a non-SQL database into FM - I've had far too many bad experiences with that:
Also to note is that we never close the persistent registry, and we flush on every write. This is because we don't know when to close it, so we hope the platform does it for us when the program & isolate are terminated. That's another reason for using an isolate there: performing a hot restart does not seem to close the file handle if open on the main thread, which throws errors when we try to open it again - it does seem to terminate isolates though, which closes the file handle. I'm not sure about whether to use JSON or sqflite here. And if we are using sqflite, would it make sense to store tiles as blobs in the database?+
I agree with the first paragraph. But the only thing we regularly read is tiles, which I do just use It's different for writing I think. Only one handle can be used to write at the same time. Here's the implementation of They both need to open the The sync version is implemented
I can't find the implementation of I'm not sure how If this is the case, then potentially the delay waiting around for threads/isolates to start/stop doesn't exist. Therefore, potentially the only delay is waiting for files to open and close for every individual write, which would be the same for sync and async methods (as we basically do the isolate work for the sync methods)? Therefore, assuming we use an open However, this does go back to this from above:
Making a non-isolate approach work in debug mode seems to be difficult (when devs are frequently hot restarting), because we either need to close the file before restarting, which is impossible to do reliably or maintain access to its handle so we can use it again without opening, which I don't know how I would do. |
Sounds likely indeed.
Yes, this sounds good to me - after thinking about this more, I realized you only really need to ensure the out of the box version of flutter_map provides good mitigation for the OSM servers, a little startup delay is probably no big deal, especially if alternative cache plugins can easily be built on the side (and those can be more feature-rich and use much more bloat). As a library consumer, I'm very interested in the cool map features you and the team are building, so keeping the cache implementation simple means it will have a lower maintenance overhead and allow you to focus more on map features more than "boring" problems that others can solve very deeply with a plugin.
That sounds appealing having everything in a single file. Be aware that Android devices are plagued with the infamous "Row too big to fit into CursorWindow" error: if your database row exceeds 2MB in size, you'll risk having issues storing / reading database rows - I would personally stay away from storing data blobs into a DB, but it's more of a gut feeling, I can't provide a strong rationale around this, especially since tiles usually weight around 50KB, and even a raw 4-Bytes-Per-Pixel 512x512 tile is about 1MB. I would give the opinionated recommendation to use files to store the data outside the DB, but it's obviously a little more complex to have truly "atomic" operations, so maybe just a bad bias I have towards DB data blobs.
Yeah, I think most OSes don't allow you to open a file in write mode multiple tiles, there can be only one "write" file descriptor at a time.
Yeah it might be, the reason I was reluctant to use isolates is the added (admittedly reasonably small) complexity related to the messaging between the isolate thread and main, but the outcome for the machine and performances is probably the same.
I know sqlite doesn't have that "locked file" problem in debug mode when hot-restarting, but it's probably using isolates under the hood... |
|
why not use lmdb(3x faster than sqlite ,and 10x faster than file-systems) instead of file-system ? lmdb is generally :
thanks |
|
Yeah I have some experience with lmdb through Isar. For what I was trying to do with it, I had too many issues to put it in the core - maybe it would be more stable for this kind of use case. But there's also the consideration to be made to keep the package lightweight - chances are, a production app might already use a library which depends on sqflite. |
|
isar is very bad . checkout http://pub.dev/packages/dart_lmdb2 this pkg. needs just 150 k.b. ,while being super-fast cmp.ed to FS and SQLite |
|
That does look like an encouraging package, but I would want more existing usage for it before adding it as a transitive dependency to all our users. |
|
why not internalize/fork it ? or maybe port it to dart ,using google-gemini then review ,because c is a lot like dart ,so it should be smooth ,hence it could become 10 - 15 kB ,compared to sqlite's 4 MB |
Allowed `NetworkTileProvider` to accept any `MapCachingProvider` implementation, defaulting to `BuiltInMapCachingProvider` Used size monitor to check cache size before applying limit if necessary
That is infeasible. Our maintenance resources are stretched as it is, without having to manage an entirely new database project with no real low-level database experience.
We cannot just use a generative AI agent to perform code review. I personally stay away from generative AI for development, but even besides that, we cannot entrust the stability of such a large and complex project to AI. @androidseb Looking at sqflite, we would need to use the layer to add compatibility for more platforms which uses sqlite3. So we may as well use sqlite3. Looking at that, I'm very unsure about adding sqlite3_flutter_libs, as this will put the package size way up (includes its own sqlite binaries), and making it work using the OS provided sqlite seems like a good way to introduce difficult to debug issues. For the time being, I'm going to keep pursuing JSON. The size monitor has made a very large reduction to startup times, so that's good. I'm going to see if I can then make the size reducer run in the background without delaying the startup. |
|
I've also changed the structure quite a bit in the latest commits. Now users can provide their own caching providers to |
Refactored workers into independent source files Added size monitor auto-repair Added read-only option
|
Again, thanks for doing this work, this looks good and I'll look into leveraging these changes in a future app update. I think it would be good to test worst case scenarios to get an idea, so what I have in mind for testing (when I get to it, you might do this way before me 😄) is something like this:
And then if I see the cache is fast enough, all good, otherwise I'll have to figure out ways to adjust the implementation, so I'll probably contribute back either core library improvements to your existing cache implementation, or if heavier dependencies are needed, improvements to an existing cache plugin, or the creation of a new one, but hopefully that won't even be needed. |
Other performance improvements to initialisation
|
@androidseb I've changed the registry format from JSON to a FlatBuffer. I've also moved some stuff within the worker isolates around, and added a shortcut to check the size monitor size on initialisation without starting an isolate if it's not necessary to run the limiter. Here's running in profile mode on my Windows again (which I note puts the
That's much better IMO. Even without moving the initialisation delay, I would say that it's barely noticeable until it reaches ~500ms. Maybe we drop our default limit down to 800MB? |
|
Awesome progress, I had not heard about FlatBuffer, learned something there. The new performance looks even better. Some thoughts:
Yes I would tend to agree.
It would be good to know the performance on a real mobile device, PCs are generally faster... If the performance is very different between platforms, we might want to make that default limit value platform-dependent? Mobile devices are generally more constrained in computing and storage resources. |
Integrate basic stats and loading into example app
|
I've exposed the tile count at initialisation from the API. In the example app I've added this count to the menu header, alongside the time it took to initialise. If you have free time, it should now be relatively easy to test! Just download the APK from the actions output for the latest commit. |
|
@androidseb Testing on my Android (https://www.gsmarena.com/xiaomi_redmi_note_13_pro-12581.php).
I haven't gone higher, but I feel comfortable saying that it's still relatively performant. Not quite 100ms:10k, but still close. One thing I have found out is that FlatBuffers can be made to work with a binary search. So you wouldn't require a seperate in-memory registry - until you consider that you have to write in order every time, which does require a full sort of every single tile for every single update, or an in-memory registry. So it may be possible to implement it so that reads can happen using binary search whilst a registry is built in the background. I had a start at implementing that, but it makes things much much more complex (especially since there is no way to validate the FlatBuffer, so any read at any time might not work as expected if there's a real corruption) and for probably an overall small gain in the end. Maybe I can look into it once I have more time if there's a real need. I'm going to drop the cache limit down to 800 MB. |
Use `DisabledMapCachingProvider` as a mixin (for built-in caching provider on web) Removed `WidgetsFlutterBinding.ensureInitialized()` from internals Improved documentation
|
I'm just running some more tests comparing against JSON. I realised it may have not been a fair comparison before - I was not running in profile/release mode I don't think. As it turns out, I've just loaded 50k tiles with a JSON registry consistently in 300ms on Windows. Yes, the registry size is definitely larger, but with more efficient encoding, I think we can get that down, and maybe the time too. I may have made a mistake switching to flatbufs 😭 - more testing on Android to come tomorrow before I make a decision whether to move back or not. But it's strange that for both methods, the difference between debug, profile, and release is so large. Like easily 3-5x slower. |
Removed `CachedMapTileMetadata.lastModifiedLocally` Store `CachedMapTileMetadata.staleAt` & `.lastModified` as milliseconds since epoch to improve performance/efficiency Compress/obfuscate `CachedMapTileMetadata` JSON representation
|
Back to JSON, getting a ratio of 40k:100ms on Windows and 10k:100ms on Android (with a slightly noticeably worse ratio at lower numbers (where the constant time of the isolate spawning has more of an overall effect). Debug mode is ~10x slower. The size limiter on Windows does 680 MB/40.5k tiles down to 1 MB in a total time of a little under 4 seconds. Not really an easy way to make this faster, it's just a lot of I/O. I also removed one thing we store which wasn't actually very useful very often, which has also brought the JSON size ratio down to a slightly better than 10k:1 MB. I think I'm going to leave it there for now. I can't think of any way to get that down any more, given it's now clearly very device dependent. |
Prepared NTIP for abortable HTTP requests Merged caching & non-caching NTIPs Improved NTIP tests
…he same file as the tile's bytes Removed return of cache length from `isInitialised` & usage in example app
|
@androidseb Sorry for disturbing you again, but I think you'll be happy to know there is now practically 0 startup delay! No more JSON registry, instead the metadata is stored within the same file as the tile bytes. It can extract and unpack all the info for each tile read in usually less than a few milliseconds, basically not noticable - there's pretty much no overhead (as there might be using two files) compared to just reading bytes. It also means the size reducer can run in the background and not delay startup. |
There was a problem hiding this comment.
@JaffaKetchup no worries, I'm happy if I can help - amazing news about the updated performance, 0 startup time sounds great, thanks for your work 💯
I've taken a quick look at the code because I was curious to see how you achieved this, I added a couple of comments for consideration in case it helps, feel free to resolve them if they're not relevant.
...tile_provider/network/caching/built_in/impl/native/workers/tile_and_size_monitor_writer.dart
Show resolved
Hide resolved
...tile_provider/network/caching/built_in/impl/native/workers/tile_and_size_monitor_writer.dart
Outdated
Show resolved
Hide resolved
Added format spec Minor improvements
|
Thanks :) |
Improved read speeds (reduced unnecessary waiting) Heavily refactored writer, size monitor, and size limiter Added `CachedMapTileReadFailureException` Removed `BuiltInMapCachingProvider.isInitialised`
See https://docs.fleaflet.dev/layers/tile-layer/built-in-caching & https://github.com/fleaflet/flutter_map/blob/caching/lib/src/layer/tile_layer/tile_provider/network/caching/built_in/impl/native/README.md.
Adds 'simple' built-in caching to
NetworkTileProviderfor native platforms. On the web (or where the caching mechanism cannot be setup for whatever reason), theNetworkTileProviderfalls back to its existing implementation.Unlike existing caching methods, this requires absolutely no additional code to use. Also, plugins can create their own caching providers to integrate with existing compatible tile providers.
Through testing, it seems to work reasonably well. The OSM tile server does seem to spit out some weird HTTP headers which seems to ask for unnecessary requests. I have opened a thread on the OSM Slack to try to figure out what's going on - no response, and it seems to work with other servers, so I'm opting to ignore it.