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

Performance regression from pre-3 to pre-8 #3069

Closed
parrots opened this issue Nov 18, 2015 · 9 comments
Closed

Performance regression from pre-3 to pre-8 #3069

parrots opened this issue Nov 18, 2015 · 9 comments
Assignees
Labels
iOS Mapbox Maps SDK for iOS performance Speed, stability, CPU usage, memory usage, or power usage
Milestone

Comments

@parrots
Copy link

parrots commented Nov 18, 2015

Pre-3 fixed the flickering issue for adding/removing annotations on iOS, and the performance was stellar. A matter of milliseconds to make annotation changes and see them in the UI.

Using pre-8 (may have happened earlier, haven't updated since pre-3) annotation changes now take ~1 second to reflect in the UI. There is no flickering or main-thread hanging, that's still all good, but the redraw now takes over a second to complete async.

@parrots
Copy link
Author

parrots commented Nov 18, 2015

From @incanus via email conversations: I bet it’s related to 656cb4c

@1ec5
Copy link
Contributor

1ec5 commented Nov 18, 2015

A rebase will say for sure, but #2951 is also a possibility.

@1ec5 1ec5 added iOS Mapbox Maps SDK for iOS performance Speed, stability, CPU usage, memory usage, or power usage labels Nov 18, 2015
@1ec5 1ec5 added this to the ios-v3.0.0 milestone Nov 18, 2015
@incanus
Copy link
Contributor

incanus commented Nov 19, 2015

I'm definitely seeing this in @parrots's app using 3.0.0-pre.8. Working on profiling now.

@incanus
Copy link
Contributor

incanus commented Nov 19, 2015

And definitely still not seeing the problem with 3.0.0-pre.3 on the same codebase.

@incanus
Copy link
Contributor

incanus commented Nov 19, 2015

Simply moving 656cb4c back to enableSetNeedsDisplay = YES and having invalidate call setNeedsDisplay again is enough to make this go away, so it does seem to be related to 656cb4c / CADisplayLink.

Trying to figure out what update case we're missing here and how to fix it.

/cc @adam-mapbox

@incanus
Copy link
Contributor

incanus commented Nov 19, 2015

Ah! The CADisplayLink is only added to NSDefaultRunLoopMode, but scrolling animations and such by Apple block that run loop so that normal activities suspend and scrolls are performant.

@incanus
Copy link
Contributor

incanus commented Nov 19, 2015

@incanus incanus self-assigned this Nov 19, 2015
@incanus
Copy link
Contributor

incanus commented Nov 19, 2015

NAILED IT

👉 #3072

@incanus
Copy link
Contributor

incanus commented Nov 19, 2015

Merged #3072 into release branch & cherry-picked to master.

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

3 participants