Hang on each tile load — VectorTileData, TileWorker::parseLayer #3636
Conversation
1ec5
added
iOS
|
Can you bisect this? |
|
@kkaefer Can you look into this? |
|
On it. |
|
👀 @jfirebaugh @kkaefer before I keep pushing this with utests et al. On my tests, adding a |
|
@tmpsantos, the hang reproduces on iOS despite your changes. Additionally, I now see the following log spew: The issue is easier to reproduce on a physical device than in the iOS Simulator. |
|
I need to solve this one: |
|
I believe the right approach here is twofold:
I'm skeptical of adding |
Not really. The first patch adds a debug mode so you can see the hangs. Build debug and remove the custom deleter, we do block a lot, specially on slow devices. |
tmpsantos
force-pushed
the
3636-hang_on_each_tile_load
branch
from
0fd8a87
to
ae4fe33
Compare
| parameters.style, | ||
| parameters.mode, | ||
| callback | ||
| }, [this](TileData* data) { tileDataDeleter.add(data); }); |
There was a problem hiding this comment.
why move from std::make_shared to new?
There was a problem hiding this comment.
That is sad, but std::make_shared doesn't support custom deleter. :(
Add a `::tryCancel()` method to the WorkTask so objects that own a WorkTask can try to cancel and if it will block, postpone the cancellation. On debug mode, the RunLoop will print a warning when we are blocking trying to cancel tasks on the main thread which we should never do.
We all the time are creating and destroying TileData objects and when we try to destroy, we first cancel any ongoing WorkRequest. When canceling a WorkRequest, there are three scenarios: - We cancel before the RunLoop queue of the Worker Thread executes the task. This should not block the Map Thread. - We cancel after the work was already executed (but we didn't pick the callback yet). This should not block either, the bad thing is the work is wasted, but we can live with that. - We cancel but there is a work ongoing. This is the worse case scenario, because we block until the work is completed by the Worker Thread. This is a design choice because the Worker might share objects with the producer thread. It gets worse if for the particular device, parsing is slow. This new method `::tryCancel` will return false if canceling is not possible without blocking, so we can defer.
Emit notifications when the work is completed. This will be used by the next patch.
We cannot simply delete the TileData at will because it might block. The custom deleter makes sure that it will only delete the object when it is non-blocking.
tmpsantos
force-pushed
the
3636-hang_on_each_tile_load
branch
from
ae4fe33
to
7c64965
Compare
FIXME and DONOTMERGE: - This will probably make text placement non-deterministic.
tmpsantos
force-pushed
the
3636-hang_on_each_tile_load
branch
from
7c64965
to
42e7ecb
Compare
|
As of 42e7ecb, I see similar hanging when panning around, radiating out from San José at z11, on an iPhone 6 running iOS 9.2. |
|
Preliminary fix is at #3711 |
|
Moving off the ios-v3.1.0 milestone now that #3711 has been merged. |
|
After #3711, things are looking slightly better but still a lot worse than in 3.0.1 in debug builds. The World Tour option in iosapp’s gear menu is more of a slideshow than an animation. This performance issue only impacts iOS devices, not OS X or the iOS Simulator under the same network conditions. Could it be specific to a particular architecture? Fortunately, the performance is comparable to 3.0.1 in release builds following #3711. |
#3724 improves this particular use case a lot, but we still have some other thread contention points to address. |
|
⇢ #3724 |
On iOS and to a lesser extent OS X, MGLMapView is unresponsive for a second or so every time a tile loads at mid zoom levels in the San Francisco Bay Area. This is a severe regression from probably late last week. These are the only threads that appear to be doing much of anything:
/cc @tmpsantos