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

Maplibre dynamic image loading #546

Merged

Conversation

westnordost
Copy link
Collaborator

@westnordost westnordost commented May 25, 2024

I implemented the dynamic image loading. If this turns out to be useful, maybe I should make other methods of *Components suspending, too, for consistency.

Some coarse quick performance tests on my phone:

On my phone, loading all images up-front took 2.0s. Posting all these to Maplibre (addImages) took on average 0.25s. Converting Pin and Style data structures to MapLibre features when loading more data into the layer took on average only about 0.02s with overlay on - this is why my earlier suggestion to move this onto a background thread by making this suspending didn't turn out to have any noticeable effect.

With this change, no images are loaded up-front, but instead on first display of data (in the background, one by one). It "feels" as if the startup is faster, but I didn't measure anything.

I didn't look at the memory.

What do you think?

@Helium314
Copy link
Owner

For me the startup time does not appear improve. Measuring the time between StreetCompleteApplication.onCreate (right after super.onCreate()) and first setting a single pin for a note quest stays around 4.2 s.
I really wonder why I don't see any improvement here. Possibly the work done mostly on a single thread?

Memory use is much better, native is now 36 MB after startup (not showing pins).
With all quests enabled and after some panning, zooming and opening quests it's around 86 MB, while without this PR in the same situation it's 166 MB.

@westnordost
Copy link
Collaborator Author

Well, I guess, naturally, if the time it takes to come up is 4.2s on the main thread, then there is this much room for things to be done on other threads before it would positively impact startup time. You wrote that the image generation takes 2.8s on your phone - as it starts immediately when the app starts in a background thread, the image generation should have no or very little impact.

Until we find a way to reduce the startup time below that, but it looks like most of that would be up to MapLibre people.

@Helium314
Copy link
Owner

Ah, I should have tested the release build. Here the startup time is clearly faster, reduced from 2.9 s to 1.3 s for the same time the debug takes 4.2 s.
So I guess we should really merge this.

@Helium314
Copy link
Owner

I got a crash when I tried to open the release build again:

2024-05-26 13:01:54.062 13467-13467 WM-Schedulers           de.westnordost.streetcomplete        D  Created SystemJobScheduler and enabled SystemJobService
2024-05-26 13:01:54.135 13467-13490 WM-WorkerFactory        de.westnordost.streetcomplete        E  Could not instantiate de.westnordost.streetcomplete.data.CleanerWorker
java.lang.NoSuchMethodException: de.westnordost.streetcomplete.data.CleanerWorker.<init> [class android.content.Context, class androidx.work.WorkerParameters]
at java.lang.Class.getConstructor0(Class.java:2332)
at java.lang.Class.getDeclaredConstructor(Class.java:2170)
at androidx.work.WorkerFactory.createWorkerWithDefaultFallback(Unknown Source:60)
at androidx.work.impl.WorkerWrapper.runWorker(Unknown Source:309)
at androidx.work.impl.WorkerWrapper.run(Unknown Source:8)
at androidx.work.impl.utils.SerialExecutorImpl$Task.run(Unknown Source:2)
at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1167)
at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:641)
at java.lang.Thread.run(Thread.java:919)
2024-05-26 13:01:54.135 13467-13490 WM-WorkerWrapper        de.westnordost.streetcomplete        E  Could not create Worker de.westnordost.streetcomplete.data.CleanerWorker

It's probably not related to this PR, but possibly to using a release build where "unused" classes are removed.

@Helium314 Helium314 merged commit 9a1388c into Helium314:maplibre May 26, 2024
@westnordost
Copy link
Collaborator Author

I think to fix this, you need to do a rebuild.

@westnordost westnordost deleted the maplibre-dynamic-image-loading branch August 14, 2024 22:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants