-
-
Notifications
You must be signed in to change notification settings - Fork 881
feat: add built-in caching #2082
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
base: master
Are you sure you want to change the base?
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
Here's some very rough performance stats:
If we want to get the startup times down, I think the only option is to use a database at this point. @androidseb @fleaflet/maintainers I think it's a good time for fresh feedback! I can try implementing a DB - but I'm a bit cautious about making it work seamlessly everywhere and keeping the weight down. What do you think about the new implementation style? Are there any other ways to get some more performance out? Thanks :) |
LMDB is just 10k lines-of-code in c ,should be roughly 5-7 k l.o.c. in dart maybe 10 kB in final binary note that LMDB is more of storage-engine than a d.b. ,hence more suited to the project flutter seems to be the future ,so i think this would also help the community thanks |
may i get some details ,regarding the requirements ? will the data be add-only ,and never/rarely removed ,or simply re-creating the data-file after a specific threshold is reached (of wastage ,due to removed data) (approach used by both realm and sqlite(auto-vacum)) ? |
lib/src/layer/tile_layer/tile_provider/network/caching/tile_metadata.dart
Outdated
Show resolved
Hide resolved
Great, I think this makes perfect sense. Keeping the base package light while allowing for flexibility to implement more advanced and heavier caching packages makes perfect sense to me. As long as we can mitigate the impact on OSM servers while getting decent UX results that will not turn down library consumers, and have the straightforward and low-cost-to-maintain JSON approach... As I wrote earlier, I'd rather see you spend your time on valuable mapping features, we plugin developers can tackle these separate complicated advanced caching problems in separate plugin packages :-)
Are you saying there is about half a second delay on the startup time? This seems very acceptable if you compare this to fetching tiles from the network... Just wondering, did you test this in a production release on a real physical device? From my experience, the Android emulator can be deceivingly fast.
I agree, keeping it lightweight no DB is the way to go if we can have an acceptable tradeoff. A 470ms startup time seems very acceptable to me. Apologies I don't have time to test things out (things are fairly busy for me at the moment), I've quickly glanced through the code, but at a high level the approach you're proposing makes sense to me. I hope these inputs help. |
That is kinda surprising that it's that small, I would've expected it to be larger! I would be interested in a Dart port, but it's unfortunately not something that I would be comfortable doing outside of 'for fun' at the moment. Now that this functionality allows swapping of the caching provider, it would be easy to change in future.
Sure. This is a non-critical cache which stores lots of 10-50KB blobs very frequently, plus some metadata. So the requirements are to have very fast creates (or at least handle a burst well) and updates. Deletion is not important - it is currently implemented, but should never be used (it's used when the registry somehow gets out of sync, which shouldn't happen). I think LMDB could be something to explore in future, but it's not feasible at the moment. But maybe I have some free time at some point :D Thanks for the suggestions though, it's certainly useful to consider.
I'm testing this on a Windows build. I think it's fair to say the longest time is the parsing of the JSON, so I think it would be fairly stable across devices. Also see dart-lang/sdk#51596 & dart-lang/sdk#51779. This is also the delay before we make the first network requests. So it looks longer to the user. But I would be comfortable saying it would go unnoticed in all feasible registry sizes when awaited in the app startup, and maybe noticed but usually less than 1 sec if startup is performed just before the first tile load. Acceptable hopefully - maybe some stronger hints in documentation. There is also a balance between the max size we impose (by default) and time spent 'size limiting'. We won't need to size limit (which is the expensive operation) for longer with a higher max size, but parse times will keep growing. It's current at 1GB - wdyt about changing this? Thanks for all your feedback :) |
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. |
See https://docs.fleaflet.dev/layers/tile-layer/built-in-caching.
Adds 'simple' built-in caching to
NetworkTileProvider
for native platforms. On the web (or where the caching mechanism cannot be setup for whatever reason), theNetworkTileProvider
falls back to its existing implementation.Unlike existing caching methods, this requires absolutely no additional code to use, although an additional one liner can be useful. It's easily extensible.
The first tile load waits for the caching mechanism to be set up (let's say 300ms) - this means a grey tile until the mechanism is ready. Other tiles loaded simultaneously must also wait. Once the mechanism has loaded, all other tiles do not have to wait. This does not have to be the case for all 'caching providers'.
Caching uses a JSON registry stored on the filesystem, which is loaded into memory as a
Map
twice (in two different isolates) on startup. Tiles are stored directly as files using a UUID. A size monitor containing 8 bytes holding the size of the cached tiles in bytes is also kept in sync.No guarantees are made as to the safety of the cached data. It's cached in an OS-controlled caching directory by default, and a corrupted registry will cause the entire cache to be reset.
The cache cannot be cleared through the API. The cache can optionally be size-limited (defaults to 800 MB) - although this is applied at startup and can be relatively slow.
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.