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

(MapBox Android) MapBox onStop freezes main thread #14554

Closed
AlexanderEggers opened this issue May 1, 2019 · 7 comments · Fixed by #14841
Closed

(MapBox Android) MapBox onStop freezes main thread #14554

AlexanderEggers opened this issue May 1, 2019 · 7 comments · Fixed by #14841
Labels
Android Mapbox Maps SDK for Android

Comments

@AlexanderEggers
Copy link
Contributor

AlexanderEggers commented May 1, 2019

I am using a MapView inside a Fragment. I have noticed that switching to a different Activity (and therefore calling the onStop of that Fragment that is called the onStop of the MapView), freezes the main thread for a while. Especially slower phones have quite some troubles with that.

Configuration

Android versions: 7.0, 8.1
Device models: Moto G5, Nexus 5, Nexus 5X
Mapbox SDK versions: 7.4.0-beta.2

@AlexanderEggers AlexanderEggers changed the title (MapBox Android) MapBox onDestroy freezes main thread (MapBox Android) MapBox onStop freezes main thread May 2, 2019
@tobrun tobrun added the Android Mapbox Maps SDK for Android label May 2, 2019
@tobrun tobrun added this to the release-mojito milestone May 2, 2019
@AlexanderEggers
Copy link
Contributor Author

AlexanderEggers commented May 2, 2019

Some additional observations: I have noticed that zooming from a low zoom level (like 1) to a much higher zoom level (like 22) and then switching to a new activity without waiting that the MapView settles or the tiles are being rendered increases the blocking time for the main thread. I was even able to reproduce this issue on pretty strong hardware like Samsung S8 or Pixel XL.

@AlexanderEggers
Copy link
Contributor Author

I did some more testing and tried out running the MapView in textureMode (app:mapbox_renderTextureMode="true"). I have noticed that in this mode, I have no main thread freezes at all.

@tobrun tobrun removed this from the release-mojito milestone May 6, 2019
@LukasPaczos
Copy link
Contributor

Thanks for the report @Mordag. Using the profiler I've noticed that it's the MapRenderer#nativeReset that blocks the thread as it synchronously resets the renderer pointer:

void MapRenderer::onRendererReset(JNIEnv&) {
// Make sure to destroy the renderer on the GL Thread
auto self = ActorRef<MapRenderer>(*this, mailbox);
self.ask(&MapRenderer::resetRenderer).wait();
}

and because surface callbacks are invoked on the main thread it seems to freeze:
glSurfaceView.getHolder().addCallback(new SurfaceHolderCallbackAdapter() {
@Override
public void surfaceCreated(SurfaceHolder holder) {
super.surfaceCreated(holder);
hasSurface = true;
}
@Override
public void surfaceDestroyed(SurfaceHolder holder) {
super.surfaceDestroyed(holder);
hasSurface = false;
nativeReset();
}
});

Profiled overhead was around 70-125ms on the Pixel 2 XL, depending on the GL thread load.

@tobrun @ivovandongen do you think we can safely move the renderer pointer reset to the GL thread?

@LukasPaczos
Copy link
Contributor

do you think we can safely move the renderer pointer reset to the GL thread?

Of course, this is already happening. What I wanted to ask is does it have to be blocking?

@tobrun
Copy link
Member

tobrun commented May 9, 2019

Of course, this is already happening. What I wanted to ask is does it have to be blocking?

Ideally we do not reset renderer at all and fix the root destructor crash with LocalGlyphRasterizer. That said, some manual testing with removing the wait invoke didn't show an issue, we can remove it to make it non blocking.

edit: re ideally we do not reset renderer -> #14618

@ivovandongen
Copy link
Contributor

IIRC we block there as to make sure all gl resources are cleaned up before the egl context is destroyed.

@LukasPaczos
Copy link
Contributor

This UI thread blocking is especially visible when scrolling in a recycler. Comparison between texture, which is not blocking, and surface:
ezgif com-video-to-gif (41)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Android Mapbox Maps SDK for Android
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants