Tile pruning too agressive?

[yohanboniface]

1.0 master:

0.7:

Git bisect points to https://github.com/Leaflet/Leaflet/commit/a4e8f4e1a8fd10c0cd701997c85bafe9894b216f

@mourner @IvanSanchez any clue, or should I investigate? :)

[mourner]

Yeah, at some point I removed it and then didn't readd it back. Need to look whether it's worth doing at all.

[yohanboniface]

Yeah, at some point I removed it and then didn't readd it back. Need to look whether it's worth doing at all.

@mourner, not sure what "it" refers to in your sentence ;)

[IvanSanchez]

I'm unable to see how the bisected commit affects tile pruning :-S

Maybe it's time to see if #3551 is not a bad idea after all?

[mourner]

@yohanboniface I referred to retaining tiles on the current zoom when panning. I removed it when reworking the logic meaning to add it back some time later, but never did. I'm not sure we should though — or maybe we should but in a limited form (e.g. retain tiles only within X pixels from the current screen). Retaining already loaded tiles is different from #3551 though (which is loading yet unseen tiles).

[hyperknot]

Can be checked visually on #4206. I recommend keeping at least one tile in memory on each sides.

What is the downside of keeping more tiles? Higher memory usage?

[kaynz]

@hyperknot Bloated DOM.

[mourner]

I think this can be fixed after rc1.

[hyperknot]

One scenario where this will matter is smooth zooming with zoomSnap = 0, zoomAnimation = false and trackpad scrolling.

On current master, this triggers thousands of tile loads even with a simple zoom-out, zoom-in (might need trackpad to reproduce smoothness): http://playground-leaflet.rhcloud.com/cap/2/edit?output

[jmullo]

The amount of tiles retained on each side should be configurable.

[pke]

On mobile devices this effect is very annoying even in wifi env. The tiles load to slow if you just scroll around the map. It takes several seconds to load a new tile, displaying white background. Gmaps does solve this way better: It loads a very low res version of the tile around the (invisible) edges of the current view and then when they become visible loads and renders the higher resolution version. And it also keeps them in memory. So when you move back and forth on the map no further than one screen everything stays sharp and ready.

[fnicollet]

Hello,

Just wanted to add my "+1" to this issue with my use case. In my case, I handle online/offline scenarios. When offline, the tile content is grabbed asynchronously from a file on the hard drive, the binary stream (image) is opened, the content is encoded as a base64 string and I set the tile.src value. So the process of displaying a single tile is not just setting the src to an img and letting the browser download the image but in my case, it can involve some I/O operation, which are quite costly. If many tiles are requested, the browser starts lagging. I guess many mobile devs who use phonegap and such to create offline maps will have the same issue.

Behavious was acceptable in 0.7.7 but really laggy in 1.0.x. because of what @yohanboniface explains.

I am not sure what the best solution is here, having a configurable tile buffer kept in memory (@hyperknot) could work in some situations (panning) but zooming in and out would still suffer from the same issue. I think having some kind of tile queue kept in memory with a Time-To-Live or a "last seen" parameter would be a better solution. That queue would have a configurable max length (0 by default if you want to keep the existing behaviour) but like @pke says, it is annoying even in wifi envrionnements, so the default value should be more than 0 in my opinion.

Fabien

[fnicollet]

Hello,

I had a go at this issue. At first I tried having an array as a property of the GridLayer for unused tiles like in leaflet 0.7.7 but I realized that the "this._tiles" object already had a similar purpose and the change could just be in the value of the "retain" property on tiles. Current implementation says that only "current" tiles (= in the viewport) should be retained: https://github.com/Leaflet/Leaflet/blob/master/src/layer/tile/GridLayer.js#L390

I added a simple check based on diff between now and the "loaded" timing. If inferior to a value configurable in the options (no value by default = recycling disabled), the tile is retained

Here is the PR if you want to give it a try: https://github.com/Leaflet/Leaflet/pull/4648

Add the option to your tileLayer, such as tileRecycleDelay: 15, which means that tiles loaded in the last 15 seconds are retained.

What do you think? Fabien

[pke]

thanks @fnicollet for the effort! While its a good solution I think it does not help to adhere to the expectations user have from panning and zooming a map. When the user pans a map back and forth, even after 15secs she expects the the tiles that have been loaded in the surrounding to be still there. All native map implementations on iPhone and Android phones manage that just fine, without wasting a huge amount of memory. Granted, they use vector tiles, but still, they retain them in memory for much longer. I would say we could try the following prune trigger: The tile is either 2 view width/heights away from the current view borders and older than xx secs.

Also this patch of yours does not solve the problem of loading tiles that are only some pixels off the current view borders. They experience is then to see a white background when you try to pan around the borders.

[jmullo]

I think pruning based on tile age is useless. I would maybe have one configurable parameter for the number of tile rows loaded outside the visible area. And another configurable parameter for the number of loaded tiles kept in a FIFO buffer cache.

[fnicollet]

Yes, the patch doesn't solve the problem of loading tiles that are just off the border but @IvanSanchez "bufferSize" does solve this (different) problem, see his commit just before my message

You can't really compare vector maps and image tiles. The data size is different and the other issue with image tiles is the "bloated" DOM, which can make the map slow if too many DOM objects are on the map, wether or not they are currently displayed.

[IvanSanchez]

I wasn't happy enough with the available solutions, then I spent a bit of time coding algorithms. So now we have:

• 4614: the buffer solution. Loads more tiles than strictly needed.

• 4648: the timeout solution. Might unload tiles more aggresively than needed.

• 4649: the FIFO MRU solution. Not trivial to set a good value for the size of the FIFO. Works for pruning less tiles when zooming.

• 4650: the dual tileRange solution. Loads as normal, marks for pruning as the buffer solution.

IMHO the FIFO solution would be the best, but can use some better calculations (keep some number of off-screen pixels, account for map size and tile size, redo on a resize map event)

[yohanboniface]

I'm waiting for a fifth option to decide.

[yohanboniface]

Until a new version in verse from @IvanSanchez I'm on the FIFO too: it gives better results when a user only goes in one direction and back, or when zooming. Nice work anyway as usual Ivan :)

[fnicollet]

Amazing work @IvanSanchez :)

I would go for both the buffer and the FIFO MRU solution. The buffer allows for pre-loading, which is a nice to have on desktop but should be disabled on mobile because of bandwidth The FIFO MRU as a sizeable cache, which can be configured depending on the use-case as well. Like on mobile, it should be set to a reasonable amount (say 15) while on the desktop you could have a bigger value because your PC can handle it easily in RAM (say 100 for confort)

[hyperknot]

Really nice work @IvanSanchez. I think the solution is a combination of #4649 and #4650.

• also dynamically calculating tiles to keep based on map size. We might just do a simple floor(map_width * map_height) / (256 * 256) * 2 or something like that.

[IvanSanchez]

@yohanboniface My code sure has a pile of bugs, the kind most vile but I'll go the extra mile and I'll edit the source file (without need to compile) to better prune your tile.

This might take a while.

[yohanboniface]

<3 <3 <3

Github is missing a feature to merge a comment into the code.

[hyperknot]

Now we have a Leaflet T-Shirt: https://teespring.com/leaflet-t-shirt#pid=389&cid=100019&sid=front

[IvanSanchez]

I just noticed that the MRU FIFO (#4649) is not a viable option.

It keeps tiles of non-active zoom levels... normally that's not an issue, but for semitransparent tiles, or for GridLayers without a solid background, this will glitch heavily.

Shall we merge #4650 instead and let this be over?

[hyperknot]

Yes, #4650 looks very simple and perfect. I'd just call it noPruneRange since it's actually the non-pruning tiles.

[IvanSanchez]

@hyperknot Renamed.

[IvanSanchez]

Seems like everyone thinks that #4650 is the way to go. @hyperknot promised to whip up a few unit tests, so the plan is to wait a couple of days for that to happen.

[pke]

Thanks! I'd like to test this in my mapbox app. However mapbox still uses leaflet 0.7. How can I force it to use the newest version? fwiw: I am using webpack to bundle my app.

[hyperknot]

@pke you won't :-) Mapbox is using MapboxGL JS now. But you can use Mapbox maps in Leaflet rc1 very simply!

[pke]

@hyperknot mapbox-js is not MapboxGL. Its still supported isn't it? How would I load mapbox maps in leaflet?

[AndreasSchmid1]

Is it somehow possible to delay or cancel the "tileunload" event? Then everybody would be able to implement his own unloading strategy.

[perliedman]

@AndreasSchmid1 sorry, no, the pruning algorithm isn't customizable like that for now.