Skip to content
This repository has been archived by the owner on Aug 8, 2023. It is now read-only.

Ambient caching disabled when the database contains 50 MB+ of offline resources #4411

Closed
jfirebaugh opened this issue Mar 21, 2016 · 17 comments · Fixed by #15622
Closed

Ambient caching disabled when the database contains 50 MB+ of offline resources #4411

jfirebaugh opened this issue Mar 21, 2016 · 17 comments · Fixed by #15622
Assignees
Labels
Core The cross-platform C++ core, aka mbgl offline performance Speed, stability, CPU usage, memory usage, or power usage

Comments

@jfirebaugh
Copy link
Contributor

Once an offline database contains 50 MB or more of offline resources, it effectively no longer permits any ambient caching. Any attempt to add an ambiently-cached resource to the database will evict any existing ambiently-cached resources, fail to get the database below 50 MB, and log the error "Unable to make space for entry".

The 50 MB limit should be treated as the size limit for ambiently-cached resources only, not as a size limit on the complete database.

@RomainQuidet
Copy link
Contributor

Confirmed, seeing a lot of logs
2016-04-04 11:45:17.778 Mapbox GL[6813:244371] [WARNING] {DefaultFileSource}[Database]: Unable to make space for entry

When the 50MB limit is reached, I have the feeling that the map lags a lot, showing new tiles is really slow.

@halset
Copy link
Contributor

halset commented Jun 11, 2016

Is this too expensive to run instead of the pragma trick?

sqlite> select coalesce(sum(length(data)),0) from tiles where id not in (select tile_id from region_tiles);
0
Run Time: real 0.013 user 0.005565 sys 0.004869

Here are the opposite query.

sqlite> select coalesce(sum(length(data)),0) from tiles where id in (select tile_id from region_tiles);
116253071
Run Time: real 0.020 user 0.010811 sys 0.006703

@tmpsantos
Copy link
Contributor

The 50 MB limit should be treated as the size limit for ambiently-cached resources only, not as a size limit on the complete database.

Unless we split this into two different databases, I see this as a feature not as a bug. Otherwise we would have to calculate "offline size + ambient size" to make sure it fits into the space you want to reserve for caching.

It is convenient to download a 600 MB offline data and set the total size to 1 GB and forget about it. The current implementation will make sure we stay within limits.

@jfirebaugh
Copy link
Contributor Author

I think the behavior most end user / developers would expect is:

  • Use up to X MB of space for ambiently cached data.
  • Store all the requested offline regions, without limit.

That's not possible right now.

@halset
Copy link
Contributor

halset commented Oct 23, 2016

@tmpsantos you wrote "It is convenient to download a 600 MB offline data and set the total size to 1 GB and forget about it". Is there an API to set a custom max cache size from the iOS API?

@friedbunny
Copy link
Contributor

friedbunny commented Oct 23, 2016

@halset Nope, maximum cache size is currently defined as a constant in mbgl.

It does look like DefaultFileSource supports a custom maximumCacheSize and you could try adding that to the iOS initializer, but I don’t know about the larger implications of doing this or if it’s advisable on a pre-existing source/database.

@halset
Copy link
Contributor

halset commented Oct 24, 2016

@friedbunny thanks.

Trying to figure out the best way to fix this for mye users with more than 50MB of offline data.

A more correct way to calculate the ambient cache size will lead to a more expensive evict. And that will lead too slower put for non-regional tiles/resources.

Would it be possible to run the evict on a different thread and not that often? Or perhaps not at all? Perhaps there could be a separate mode where the application developer takes the responsibility to clean up the ambient cache? Either by number of MB or by deleting old tiles.

@ericrwolfe
Copy link
Contributor

Is there an advantage to running evict() on every put?

I'm thinking we could run evict less frequently with a more accurate (and slightly more expensive) size query e.g. #4411 (comment). We could call evict, say, every 100th call to put (or, more likely, randomly on 1% of calls to put).

@jfirebaugh
Copy link
Contributor Author

jfirebaugh commented Jan 25, 2017

If it's a question of constant time factors, that would be fine. What I want to avoid is making the eviction algorithm linear (or worse) in the size of the tiles/resources tables. Those tables can get big, a linear factor alone is problematic, and eviction can easily compose with operations that do a lot of puts to go n².

@ericrwolfe
Copy link
Contributor

ericrwolfe commented Jan 25, 2017

By limiting the query to only those resources in the ambient cache and not in the offline regions as is hinted at in #4411 (comment), the query time should never really grow beyond what's performed when the 50mb ambient cache is full (~1000 resources).

@jfirebaugh
Copy link
Contributor Author

I think those queries would have to do a table scan on either tiles or region_tiles -- that's the linear factor I'm referring to.

@ericrwolfe
Copy link
Contributor

ericrwolfe commented Jan 25, 2017

Ah yes, you're right. I can't think of a solution that avoids a full table scan without storing size statistics in a separate metadata table.

Nonetheless, as it's currently implemented, once a single offline region is stored and the 50mb database size limit is exceeded, the inner while loop inside of evict is called every single time and full table scans effectively happens anyway as resources are deleted.

Until we can come up with a better solution, an occasional full table scan via a reduced evict rate would still be orders of magnitude better than what's happening currently.

@halset
Copy link
Contributor

halset commented Jan 26, 2017

I would prefer the ability for the developer to implement a evict policy. In my hugely offline app, I use a branch with an empty evict method. That could be the no-evict strategy.

@kkaefer kkaefer added Core The cross-platform C++ core, aka mbgl performance Speed, stability, CPU usage, memory usage, or power usage labels May 30, 2017
halset added a commit to halset/mapbox-gl-native that referenced this issue Jun 25, 2018
* deleteRegion also deletes tiles/resources only belonging to that region
tsemerad pushed a commit to alseageo/mapbox-gl-native that referenced this issue Sep 5, 2018
* skipping resources/tiles evict. see mapbox#4411 (comment)
* deleteRegion also deletes tiles/resources only belonging to that region
@guyinn
Copy link

guyinn commented Sep 27, 2018

Any update on this issue?

@rolandpeelen
Copy link

Hi Guys, this doesn't really seem fixed seeing this discussion? I'm working on an app that predominantly relies on offline data (downloading x amount around a gpx track) for offroad riding into places where there is little to no internet. It's quite vital to be able to store more than 50mb of storage.

How would I go about making this work?

@halset
Copy link
Contributor

halset commented Feb 18, 2019

@rolandpeelen Hello. I am in a similar situation. I work around the problem by not doing any evicting at all. That works fine for my app. halset@dbe4786

@natalia-osa
Copy link

Hi, I also am facing this issue. I get a lot of these logs with each scroll using iOS API (around 10 per second). Happens only during scrolling. The app also crashes eventually after some scrolling, I'm now checking if that's connected with this console output or that's not related.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Core The cross-platform C++ core, aka mbgl offline performance Speed, stability, CPU usage, memory usage, or power usage
Projects
None yet
Development

Successfully merging a pull request may close this issue.