cache parsed tiles

[ansis]

implements #1157

The cache stores the 20 most recently used but not visible tiles.

On iOS it clears the cache completely when it receives a memory warning.

20 isn't a carefully picked number. It's just what -js uses. It seems reasonable. Each tile seems to be roughly ~1.5 MB. I haven't measured carefully, but that's 1/20th of how much the simulator's memory usage drops by when the warning is triggered.

Is 20 good or could we cache even more?

[gundersen]

💥

[mb12]

The cache capacity should be a function of device dimension in dip.

One estimate for lower bound for cache capacity would be to have at least enough number tiles to cover the screen in worst configuration (map rotated and most of the tiles covering the viewport partially). This is to ensure fast pause/resume when the app goes to background or if the Activity is paused. For e.g for a screen with 320 x 480 dip, this number would be 4.

The cache capacity should at least maintain the following invariant.
cache capacity >= number of required tiles returned by coveringTiles

[mb12]

For iPad Air, in certain configurations, we would need > 20 tiles just to cover the screen. Is it possible to make the cache capacity dynamic so that cache capacity is always >= the number of tiles returned by coveringTiles?
The configuration where I got > 20 for iPad air was the scenario where in the client renders map at zoom level z, using tiles at zoom level z+1.

[jfirebaugh]

null-check style before dereferencing

[incanus]

that's 1/20th of how much the simulator's memory usage drops by when the warning is triggered.

Just FYI, device testing is a must here — the simulator has as much RAM as your desktop, so 4, 8, 16GB? Plus it has swap. Memory behavior is completely different on device.

[incanus]

To put some numbers in perspective:

• High water mark for master GL testing on retina iPad mini: 49.7MB
• High water mark for this branch as of 9228566: 66.3MB

That's a pretty big jump, considering this is just the map view in someone else's app.

I'll keep doing some more testing on it.

[mourner]

Wow, didn't even notice when I closed this accidentally.

[ansis]

@incanus do you want to look into choosing a good cache size to balance speed, memory usage and screen size?

[incanus]

Sure, I can take a look.

[incanus]

So one thing presents itself here is the call to cache.clear() in Source::invalidateTiles. The invalidation is meant for annotation tiles, the number of which will be between 0 and X, where X is the number of tiles to cover the screen.

However, annotation tiles are generally much simpler than normal base map tiles (in the case of points, definitely, but this will get more complex with shapes, but still waaaaaay simpler). So we should probably cache vector and live tiles differently. Live tiles are pure collections of projected x, y coordinates that get binned into 4096 extent tiles, whereas vector tiles are actually manipulating protocol buffers to do the same.

Memory usage for live and vector tiles will differ wildly — I'll get some numbers together.

[incanus]

To put it another way: with the 10,000 demo point annotations added to the map, you never see the tiles refresh without caching, so we probably just don't need to cache LiveTile objects at all.

[incanus]

Per chat, @ansis pointed out that a cache is per-source, but caching LiveTile objects twice seems wasteful, even for low numbers of tiles, and especially when they could feature thousands of features and the AnnotationManager only ever parses them at annotation add/remove time.

This would be the diff to exclude annotation live tiles from caching:

diff --git a/src/mbgl/map/source.cpp b/src/mbgl/map/source.cpp
index 865472f..484c789 100644
--- a/src/mbgl/map/source.cpp
+++ b/src/mbgl/map/source.cpp
@@ -244,7 +244,7 @@ TileData::State Source::addTile(Map &map, Worker &worker,
         new_tile.data.reset();
     }

-    if (!new_tile.data) {
+    if (!new_tile.data && info.type != SourceType::Annotations) {
         new_tile.data = cache.get(id.to_uint64());
     }

@@ -403,12 +403,13 @@ void Source::update(Map &map,
     // Remove tiles that we definitely don't need, i.e. tiles that are not on
     // the required list.
     std::set<TileID> retain_data;
-    util::erase_if(tiles, [&retain, &retain_data, &tileCache](std::pair<const TileID, std::unique_ptr<Tile>> &pair) {
+    auto& type = this->info.type;
+    util::erase_if(tiles, [&retain, &retain_data, &type, &tileCache](std::pair<const TileID, std::unique_ptr<Tile>> &pair) {
         Tile &tile = *pair.second;
         bool obsolete = std::find(retain.begin(), retain.end(), tile.id) == retain.end();
         if (!obsolete) {
             retain_data.insert(tile.data->id);
-        } else if (tile.data->ready()) {
+        } else if (type != SourceType::Annotations && tile.data->ready()) {
             tileCache.add(tile.id.to_uint64(), tile.data);
         }
         return obsolete;

[incanus]

Per chat, I wasn't getting at first that the cache is actually a reference counting mechanism and not a copy, so there is no additional memory overhead in "caching" live tiles and a bonus to doing so with complex geometries that would need to get re-parsed.

[incanus]

So I think regardless of what the size ends up being, it should vary based on device and map view size, or at least the initial size. That proves tricky with where Source creates its cache, but I'm looking into it. We will probably have the client OS bindings pass this down — for example, it would certainly want to pass a different value for a high-end retina iPad vs. an iPad 3, even though they have the same pixel size and ratio.

[incanus]

We do need to make an exclusion for raster sources, though — memory usage is really high when we start caching uncompressed imagery in RAM.

[incanus]

Homing in on some recommendations here after device testing. Will push soon.

[ljbade]

@ansis @incanus Make a ticket to hook up the Android low memory warning to this when you close this for iOS

[incanus]

Something else I need to look into: memory usage doesn't drop at all when switching styles.

[friedbunny]

Supporting iOS 7 means that there is still a crop of severely constrained 512MB devices out there: iPhone 4/4S, iPod Touch 5G, and iPad 2/Mini. I've noticed memory warnings on my iPad Mini as is, not really doing much of anything. Drawing detailed tiles on Emerald with my iPhone 4 is dog slow but memory usage seems low ~30-35MB (master, release build), so it's mostly a CPU problem that could probably benefit from more caching.

(Though, clearly it'll be a long time — probably longer than the life of this library — before meaningful support for 512MB iOS devices can go away. E.g., Apple still sells the iPad Mini and it seems unlikely that they'd drop it in iOS 9.)

[incanus]

Yep, I have been working on some cache sizes that take into account system memory and screen size. If a particular piece of hardware performs well with a certain cache size and 512MB of RAM, it follows that a device with 1GB of RAM should be able to handle more cache headroom.

[incanus]

My test devices have been:

• iPad 3
• iPhone 4s
• iPod touch
• iPad mini 2
• iPhone 5s

[incanus]

Things are looking pretty good here. This is blocked by #1267, which over-retains old style's sources (and thus their caches) and will hamper our performance testing as we are trying multiple styles out.

But we've now got a resizable cache which makes a conservative guess as to initial size at the C++ level based on screen size and zoom range of the map. This gets us past startup and allows for per-platform size updating later when the view system is ready.

floor( (width / tile size) * (height / tile size) * (zoom range) * (0.5) )

For the demo app:

iPad: floor( (768 / 512) * (1024 / 512) * (19) * (0.5) ) = 28
iPhone 5s: floor( (320 / 512) * (568 / 512) * (19) * (0.5) ) = 6

That's the conservative size based on knowing nothing about the processor or system memory, nor performance quality of the device (contrast an Apple device with a relatively cheap Android one, for example).

At the platform level, we can make smarter decisions, and this heuristic seems to work well:

zoom range = max - min + 1
CPU factor = number of cores
memory factor = 0.5 per 512MB
size factor = (width / tile size) * (height / tile size)

floor( zoom * cpu * memory * size * 0.5 )

iPad 3: 57
iPad mini: 57
iPhone 4s: 6
iPod touch: 6
iPhone 5s: 13

Considering tile coverage scenarios, where at most you can show six tiles on an iPhone:

And maybe eleven if you hit it just right on iPad:

Going lower than the twenty initially outlined by @ansis still seems conservative, but I think it's better to gauge perceived performance from this first and grow to use more memory if needed later. To reiterate, these are tiles which we had around, parsed, that were (generally) more than one zoom level higher or more than ten zoom levels lower than the current viewport when we were determining new, native-zoom tiles to load (there's some bias in the algorithm, but that's close enough).

Also, we have a new menu entry for dumping the memory cache in testing:

Though, dumping memory after switching styles still leaks since we're blocked by #1267:

[incanus]

Per #1255 (comment), I don't think we should be caching uncompressed PNG raster tiles in memory, so 6026f17 excludes them.

[mb12]

@incanus Can you please review the caching for raster sources? Raw image data is deleted immediately from CPU after copying to GPU (Raster::bind).

mapbox-gl-native/src/mbgl/util/raster.cpp

Line 57 in 3bd180d

img.reset();

mapbox-gl-native/src/mbgl/util/raster.cpp

Line 78 in 3bd180d

img.reset();

[incanus]

@mb12 Raster empties img after GL upload and img is a pointer to a util::Image. This caching has to do with TileData (in this case, RasterTileData), which has a RasterBucket, which in turn has a Raster.

However, the img is set from RasterTileData's data object on "parse". The data comes from the network download and sticks around in RasterTileData, in the cache, so that's the expensive thing that's still being held onto, the equivalent to VectorTileData's parsed out geometries.

This cache, then, is also caching PNG/JPG data blobs, and I don't think that's a good use of memory, especially since raster tiles aren't the focus here. I will do some testing though to compare sizes of rasters and vectors in memory.

[mb12]

Wouldn't performance of use cases where in both raster and vector in the same style (e.g. Hybrid satellite) be affected if caching is disabled for one source but not other?
Is it possible to have this work well at least when the app is in foreground so that simple panning back and forth across tiles do not cause re-loads like they did without caching?

[incanus]

Wouldn't performance of use cases where in both raster and vector in the same style (e.g. Hybrid satellite) be affected if caching is disabled for one source but not other?

Possibly, but raster isn't a focus right now. In subsequent betas, we will definitely be enabling and tuning the Hybrid mode.

Is it possible to have this work well at least when the app is in foreground so that simple panning back and forth across tiles do not cause re-loads like they did without caching?

Are you seeing this with raster, vector, or both? Memory caching should help this (pending whichever types we decide to cache).

[incanus]

Things are looking good now with #1267 fixed.

1. Period of caching starting with initial, small viewport, increasing memory use followed by a style change from Streets to Emerald, which properly dumps cache.
2. Period of caching on Emerald, followed by a style change to Light, again dumping cache to levels. Memory use is slightly lower since Light uses less than Emerald.
3. Period of caching before switching to Dark, which uses about the same amount of memory as Light.
4. Period of caching before manually dumping cache, which goes to pre-caching levels just as if a style change had happened.

Going to work on squashing now.

[incanus]

→ #1281