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

Aggressively cancel requests when zooming and panning #1158

Closed
1ec5 opened this issue Mar 30, 2015 · 8 comments
Closed

Aggressively cancel requests when zooming and panning #1158

1ec5 opened this issue Mar 30, 2015 · 8 comments
Assignees
Labels
iOS Mapbox Maps SDK for iOS performance Speed, stability, CPU usage, memory usage, or power usage

Comments

@1ec5
Copy link
Contributor

1ec5 commented Mar 30, 2015

A typical app will set the viewport to a specific location or the world map on launch (perhaps as a workaround for #1145) and will immediately pan and zoom to the user location. In the iOS demo app, this happens if you had user tracking enabled the last time you closed the app.

From testing the demo app on an iPhone 4s running iOS 7.1 on a cellular connection – pretty much our worse-case scenario – it appeared that we were queueing up requests for vector tiles but failing to cancel unnecessary requests. If you allowed the app to pan and zoom to the user location, tiles would take upwards of a minute to load, much longer than if the app had gone directly to the user location in the first place. I have a hunch that this is because the app is attempting to fetch and render numerous irrelevant tiles from the original viewport and at intermediate stages along the pan/zoom animation. If this is the case, it could also affect performance during user-driven panning and zooming.

Even if we do cancel network requests for irrelevant tiles, is there any way to cancel their rendering as well?

/cc @ansis @jfirebaugh @kkaefer

@1ec5 1ec5 added iOS Mapbox Maps SDK for iOS performance Speed, stability, CPU usage, memory usage, or power usage labels Mar 30, 2015
@ansis
Copy link
Contributor

ansis commented Mar 30, 2015

In -js we abort network requests but we don't have a way of aborting parsing. I'm guessing that native is the same. This could be huge.

@jfirebaugh
Copy link
Contributor

I'll take a look at this.

@jfirebaugh jfirebaugh self-assigned this Mar 30, 2015
@jfirebaugh
Copy link
Contributor

It looks like we do have a mechanism to cancel requests (Environment::cancelRequest). One of routes where it gets called starts here, but in my tests that's essentially dead code. The referent TileDatas of the weak_ptrs in the tile_data map are always gone by the time this loop finds them -- I think because the only strong references to the TileDatas are the Tiles, which get cleared out of the tiles map first. But the call in ~TileData does get triggered.

@mb12
Copy link

mb12 commented Mar 30, 2015

Does Envirnoment::cancelRequest cancel both of the following:
1.) Network request
2.) Tile Parsing. (The individual bucket creation code needs to have tileParser.obsolete() check embedded for this to be cancelled). It only happens at the very end in createXXXXBucket. It needs to happen within the guts of this logic.

Can you add a debug log that prints the following times (Maybe its already there)?
1.) Time to fetch over network or sqlite cache for each tile.
2.) Time to create bucket for each tile.

One can then look at this debug spew to see if the client is doing more work than it needs to.

@mb12
Copy link

mb12 commented Mar 31, 2015

@jfirebaugh Can you please also review the code to reason how the following scenario is handled?

1.) More than 4 tiles are loaded from sqlite cache and get enqueued for parsing.
2.) These tiles than become obsolete due to zoom/pan.
3.) The work requests for obsolete tiles should either not run or get off the work queue fast.

@jfirebaugh
Copy link
Contributor

Yeah, there is no mechanism for canceling a queued or in-progress tile parse right now. I'm looking into that next.

@1ec5 1ec5 changed the title Agressively cancel requests when zooming and panning Aggressively cancel requests when zooming and panning Mar 31, 2015
@jfirebaugh
Copy link
Contributor

There is actually a mechanism for canceling tile parsing, but only at certain points (once per layer in the style).

There's a lot of refactoring that should happen here, but I don't see any easy wins. Going to jump off this for now.

@jfirebaugh
Copy link
Contributor

After landing #1691 I'm happy with the state of our request cancelling -- it is fairly aggressive and should happen soon after a tile is no longer needed.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
iOS Mapbox Maps SDK for iOS performance Speed, stability, CPU usage, memory usage, or power usage
Projects
None yet
Development

No branches or pull requests

4 participants