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.14k stars 135 forks source link

GoogleMap() subcomposition performs excess recompositions #480

Open bubenheimer opened 8 months ago

bubenheimer commented 8 months ago

android-maps-compose 4.3.0

The GoogleMap() implementation's subcomposition performs excess recompositions. Specifically, starting with the third composition, the subcomposition always performs an additional composition, i.e. it recomposes twice for every change.

I have a separate project replicating the GoogleMap() subcomposition structure to narrow down the issue: https://github.com/bubenheimer/dualsubcomposition

In this minimal project only the top-level subcomposition shows extra recompositions. In the GoogleMap() case, Composable functions beneath the top-level subcomposition also have extra recompositions, depending on what changes are being recomposed.

This is independent of Compose version used, I went all the way back to 1.0.5 for Compose runtime & compiler to see if there was a regression. Current versions are 1.5.6 (compiler), 1.5.4 (runtime). Kotlin version does not matter, either.

I was unable to find an issue reported on this. It would likely be helpful to request external help from Android Compose team.

Code is essentially this:

@Composable
private fun Parent() {
    var value by remember { mutableStateOf(0) }

    SideEffect { println("Parent recomposed, value: $value") }

    LaunchedEffect(Unit) {
        repeat(3) {
            delay(5_000)
            value += 1
        }
    }

    GoogleMapFake(value)
}

@Composable
private fun GoogleMapFake(value: Int) {
    SideEffect { println("GoogleMapFake() recomposed, value: $value") }

    val currentValue by rememberUpdatedState(value)

    val parentComposition = rememberCompositionContext()

    LaunchedEffect(Unit) {
        disposingComposition {
            newComposition(parentComposition) {
                SideEffect { println("Subcomposition recomposed, currentValue: $currentValue") }

                Child(currentValue)
            }
        }
    }
}

@Composable
private fun Child(value: Int) {
    SideEffect { println("Child recomposed, value: $value") }

    LaunchedEffect(value) { println("Child LaunchedEffect, value: $value") }
}

private suspend inline fun disposingComposition(factory: () -> Composition) {
    val composition = factory()
    try {
        awaitCancellation()
    } finally {
        composition.dispose()
    }
}

private fun newComposition(
        parent: CompositionContext,
        content: @Composable () -> Unit
) = Composition(MyApplier(), parent).apply { setContent(content) }

Output:

Parent recomposed, value: 0               
GoogleMapFake() recomposed, value: 0      
Subcomposition recomposed, currentValue: 0
Child recomposed, value: 0                
Child LaunchedEffect, value: 0            
Parent recomposed, value: 1               
GoogleMapFake() recomposed, value: 1      
Subcomposition recomposed, currentValue: 1
Child recomposed, value: 1                
Child LaunchedEffect, value: 1            
Parent recomposed, value: 2               
GoogleMapFake() recomposed, value: 2      
Subcomposition recomposed, currentValue: 2
Child recomposed, value: 2                
Child LaunchedEffect, value: 2            
Subcomposition recomposed, currentValue: 2
Parent recomposed, value: 3               
GoogleMapFake() recomposed, value: 3      
Subcomposition recomposed, currentValue: 3
Child recomposed, value: 3                
Child LaunchedEffect, value: 3            
Subcomposition recomposed, currentValue: 3

The problems starts once the value reaches 2, third composition.

(Example updated to make the intention clear)

bubenheimer commented 8 months ago

I think this issue is caused by rememberUpdatedState() updating states during composition, not during the apply phase. That's fine for state usage in a normal long-running lambda, but in our case there is a subcomposition in the long-running lambda and this peculiar rememberUpdatedState() behavior causes snapshot state to get out of whack.

I have a fix for this which I will turn into a PR once my other PRs are merged, to avoid mega merge conflicts.

bubenheimer commented 7 months ago

I've created an issue on the Google issue tracker: https://issuetracker.google.com/issues/322121228

Would be really great if one of the maintainers here could tap internal channels to get it prioritized.

I don't like my "fix", because it significantly delays recomposition of the subcomposition, to the next composition cycle.