Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Performance of traffic island quest #5146

Closed
mxxcon opened this issue Jul 15, 2023 · 21 comments
Closed

Performance of traffic island quest #5146

mxxcon opened this issue Jul 15, 2023 · 21 comments
Labels

Comments

@mxxcon
Copy link

mxxcon commented Jul 15, 2023

I just noticed when I tap on crossing island, surrounding traffic lights icons take 1-2 seconds to appear. This was never so slow. They would always appear instantly. I haven't actively used SC in a version or two so can't tell when exactly it started happening.

How to Reproduce
Tap on crossing island quest
Screenshot_20230715_022536_StreetComplete

Expected Behavior
Related icons instantly appear.

Versions affected
53.3 (possibly earlier too)

@mxxcon mxxcon added the bug label Jul 15, 2023
@mxxcon
Copy link
Author

mxxcon commented Jul 15, 2023

Here a clip of it happening. In the past those icons would show up instantly.
https://github.com/streetcomplete/StreetComplete/assets/847140/97ce8528-a7f8-4338-b6b3-2d9150e69d73

@Helium314
Copy link
Collaborator

This is really weirdly slow, especially considering that the data should alrealy be cached.
Is it also slow when you disable and enable the overlay again, or is it only for nearby elements (crossings in this case)?

@mxxcon
Copy link
Author

mxxcon commented Jul 15, 2023

So far I noticed this only with this quest.
Overlay setting makes no difference, if it's off or set to any overlay.

I'm on Samsung Galaxy S20FE running Android 13 with June 1 security updates

@westnordost
Copy link
Member

I also wonder why the form itself takes 2s to show up. Showing of the form is independent from showing of the icons on the map.

@mxxcon
Copy link
Author

mxxcon commented Jul 15, 2023

I just tried again and the same behavior happens after I clear app cache.

If you want to try it yourself, node 10676274442 makes it very obvious. It's northern crosswalk of intersection of Ocean Avenue and Glenwood Road in Brooklyn, NYC.

Screen_Recording_20230715_092856_StreetComplete.mp4

@Helium314
Copy link
Collaborator

Helium314 commented Jul 16, 2023

If you want to try it yourself, node 10676274442 makes it very obvious. It's northern crosswalk of intersection of Ocean Avenue and Glenwood Road in Brooklyn, NYC.

I can't reproduce it here, even on my 10 year old phone it takes less than 0.1 s to show the nearby crossings.

For https://www.openstreetmap.org/node/8544895124 (the node from your first video) it's definitely slower. Here 67 nearby elements are highlighted, which takes more noticeable 0.3 s. But your S20 should be like 10 times faster than my S4 Mini...

I also wonder why the form itself takes 2s to show up. Showing of the form is independent from showing of the icons on the map.

It's not really independent. Adding hightlighted elements to the map (which is most of the 0.3 s above) is done on the main (UI) thread (via viewLifecycleScope), so the form can't show up as long as the loop over the highlighted elements is running.
[Edit: running the loop on main thread doesn't seem necessary, at least it worked on Dispatchers.Default in a quick test]

@mxxcon
Copy link
Author

mxxcon commented Jul 16, 2023

Yes, node/8544895124 is really noticable.
I tried enabling GPUWatch from developer settings to see how my phone was performing and what it was doing. But as soon as it was turned on SC started rendering much faster. When I turned off GPUWatch SC that quest became slow again.
Another thing I noticed is if SC is downloading/refreshing data in the background, quest on that node renders much faster

@mnalis
Copy link
Member

mnalis commented Jul 17, 2023

When I turned off GPUWatch SC that quest became slow again. Another thing I noticed is if SC is downloading/refreshing data in the background, quest on that node renders much faster

Interesting, I'm wondering if that might be related to battery optimizations on Samsung... E.g. when it detects low load it switches you to lower-performance Cortex-A55, and when it senses the load starts to rise, it reenables high-power Cortex-A77 CPUs. Perhaps you can try playing with battery optimization settings, whitelisting StreetComplete to see if that helps?

@mxxcon
Copy link
Author

mxxcon commented Jul 17, 2023

But then wouldn't this be a problem with every quest and otherwise overall app performance, and not just crossing island quest?
I already have adaptive battery setting turned off and SC doesn't get killed or put to sleep. Just one specific quest is unusually slow.

@westnordost
Copy link
Member

I also wonder why the form itself takes 2s to show up. Showing of the form is independent from showing of the icons on the map.

It's not really independent. Adding hightlighted elements to the map (which is most of the 0.3 s above) is done on the main (UI) thread (via viewLifecycleScope), so the form can't show up as long as the loop over the highlighted elements is running.
[Edit: running the loop on main thread doesn't seem necessary, at least it worked on Dispatchers.Default in a quick test]

Oh, yeah. I was about to suggest that, but I see you edited the post. Posting the update to tangram-es was never intended to be on the main thread.

@Helium314
Copy link
Collaborator

Oh, yeah. I was about to suggest that, but I see you edited the post. Posting the update to tangram-es was never intended to be on the main thread.

So just change


to viewLifecycleScope.launch(Dispatchers.Default)?

What to do with the actual issue... I have no idea. It very much looks like some Samsung optimization gone wrong, especially since it doesn't happen when a profiling tool is enabled.

@mxxcon
Copy link
Author

mxxcon commented Jul 19, 2023

Maybe there's some suboptimal regex or tag matching going on that significantly slows things down?
Or perhaps release a debug build that would write profile log for this quest and I could upload it once I have some data there?

@Helium314
Copy link
Collaborator

Maybe there's some suboptimal regex or tag matching going on that significantly slows things down?

All the tag matching stuff is already done before you see the first icon, so we can exclude this.

On my device, the (by far) slowest part after finding the crossings is putting the icons on the map. It just takes a few milliseconds per icon, but e.g. 67*3 ms = 201 ms. Incidentally this is also the only part where the GPU is used (via the map library.

@mxxcon
Copy link
Author

mxxcon commented Jul 19, 2023

I just tried this quest on my LG V30 and noticed the same slow down, but it doesn't happen as consistently.

Recording_2023-07-19-08-42-17.1.mp4

@mxxcon
Copy link
Author

mxxcon commented Jul 19, 2023

And stranger still, on my LG V30 this slowness guess away when I start screen recording. I recorded it with my other phone but can't upload here. I posted it on OSMUS slack server in #streetcomplete channel.
https://osmus.slack.com/files/U022BGW29AL/F05HP2Z58G5/20230719_090033.mp4

@westnordost
Copy link
Member

Well, I think you can stop researching this because it will have something to do with tangram-es which is a bit of a black box (viewed from our side, it's open source alright) and not maintained anymore. So, we are not in a position to fix this anyway. What we can fix is to make sure that displaying of the stuff on the map is done in a separate thread as showing the form, so the slowdown does have much less of a negative effect.

So just change .... to viewLifecycleScope.launch(Dispatchers.Default)?

Err, I guess. I didn't look at the code yet. Would make sense to check for other places where data is put on the map whether it is done in a background thread.

@Helium314
Copy link
Collaborator

Overlay, quest pins and downloaded area managers already do it in background. Is there anything else?

@westnordost
Copy link
Member

Maybe the updates of the view direction and location?

@Helium314
Copy link
Collaborator

Direction is updated on Compass Dispatcher Thread, location and accuracy are updated on the main thread. Though the latter might be intentional

private val mainExecutor = ContextCompat.getMainExecutor(context)

@westnordost
Copy link
Member

Hm I guess that's okay then.

Helium314 added a commit that referenced this issue Jul 22, 2023
@westnordost
Copy link
Member

So, @Helium314 made it so that updates on the map do not block the initialization of the UI which should somewhat alleviate the UX issue described here, wherever the issue is coming from.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants