search

googlemaps / android-maps-compose

Jetpack Compose composables for the Maps SDK for Android
https://developers.google.com/maps/documentation/android-sdk/maps-compose
Apache License 2.0
1.16k stars 142 forks source link

Unnecessary map initialization performance penalty caused by delaying composing GoogleMap() subcomposition until after GoogleMap object is available #501

Open bubenheimer opened 10 months ago

bubenheimer commented 10 months ago

android-maps-compose 4.3.0

From code inspection I've noticed that there is an undue performance penalty caused by delaying the GoogleMap() subcomposition until after the GoogleMap object has been made available via suspending MapView.awaitMap().

While the GoogleMap object is required during the "apply changes" phase of the subcomposition, it should not be required to perform the initial composition of the subcomposition, prior to "apply changes".

Initial composition phase of subcomposition and MapView.awaitMap() should and can be performed in parallel.

bubenheimer commented 10 months ago

Just as an aside thought, not directly linked to this issue: the initial composition phase of the GoogleMap() subcomposition theoretically would not need to be performed on the main thread, which would free the main thread for other concurrent initialization tasks. This may be too risky to attempt for now.

bubenheimer commented 10 months ago

Another element to consider here: MapsInitializer may need to be a part of an app's map initialization pipeline: the GoogleMap() subcomposition composition phase should not be delayed until after MapsInitializer is done; it can run in parallel.

bubenheimer commented 9 months ago

@wangela Just FYI: the 4.3.3 release says that it fixes this issue, but that is not the case. I think this mixup happened because I mentioned this issue in one of the commits for 4.3.3 ("see #501", I was not aware that this has "fixes" semantics, maybe some problem with release automation, or maybe I shouldn't do this in commit messages?).

wangela commented 8 months ago

@bubenheimer Where are you seeing that the release says it fixes this issue 501? I only see it mentioning that it fixes issue 466 which was tagged intentionally on your PR number 478.

bubenheimer commented 8 months ago

@wangela That link says it "closes" #501

wangela commented 8 months ago

Strange, may be human error. I've deleted that mention from the release notes.

bubenheimer commented 6 months ago

I've added two related issues on the Google issue tracker:

https://issuetracker.google.com/issues/340494390 https://issuetracker.google.com/issues/340476594