fix: compose performance (#30)

* fix compose performance Signed-off-by: Arthur Bleil <[email protected]> * fix equals logic Signed-off-by: Arthur Bleil <[email protected]> * fix code smell Signed-off-by: Arthur Bleil <[email protected]> Signed-off-by: Arthur Bleil <[email protected]>
ZupIT · Aug 29, 2022 · 624e54e · 624e54e
1 parent d9bc063
commit 624e54e
Show file tree

Hide file tree

Showing 3 changed files with 103 additions and 60 deletions.
diff --git a/compose/src/main/java/br/zup/com/nimbus/compose/ComponentData.kt b/compose/src/main/java/br/zup/com/nimbus/compose/ComponentData.kt
@@ -2,9 +2,51 @@ package br.zup.com.nimbus.compose
 
 import androidx.compose.runtime.Composable
 import androidx.compose.runtime.Immutable
+import androidx.compose.runtime.Stable
 import com.zup.nimbus.core.tree.ServerDrivenNode
 
+private fun componentListsAreEqual(list: List<*>, comparable: List<*>): Boolean {
+    return (list.size == comparable.size && list.toSet() == comparable.toSet())
+}
+
+private fun componentMapsAreEqual(map: Map<String, *>, comparable: Map<String, *>): Boolean {
+    val requiresDeepComparison = (map.size == comparable.size && map.keys == comparable.keys)
+    if (requiresDeepComparison) {
+        for (entry in map.iterator()) {
+            val areEqual = when (entry.value) {
+                is Function<*> -> continue
+                is Map<*, *> -> componentMapsAreEqual(
+                    entry.value as Map<String, *>,
+                    comparable[entry.key] as Map<String, *>
+                )
+                is Array<*> -> (entry.value as Array<*>).contentEquals(comparable[entry.key] as Array<*>)
+                is List<*> -> componentListsAreEqual(entry.value as List<*>, comparable as List<*>)
+                else -> (entry.value == comparable[entry.key])
+            }
+            if (!areEqual) return false
+        }
+        return true
+    }
+    return false
+}
+
+private fun componentsAreEquals(node: ServerDrivenNode, comparable: ServerDrivenNode): Boolean {
+    return !(
+        (node.id != comparable.id) ||
+        (node.component != comparable.component) ||
+        !(
+            (node.properties == comparable.properties) ||
+            (node.properties?.let { otherProperties ->
+                comparable.properties?.let { currentProperties ->
+                    componentMapsAreEqual(otherProperties, currentProperties)
+                }
+            } == true)
+        )
+    )
+}
+
 @Immutable
+@Stable
 class ComponentData(
     val node: ServerDrivenNode,
     val parent: ServerDrivenNode?,
@@ -17,6 +59,9 @@ class ComponentData(
     }
 
     override fun equals(other: Any?): Boolean {
-        return if (other is ComponentData) other.hashCode() == hash else false
+        return when (other) {
+            is ComponentData -> componentsAreEquals(other.node, node)
+            else -> false
+        }
     }
 }
diff --git a/compose/src/main/java/br/zup/com/nimbus/compose/core/ui/internal/NimbusDisposableEffect.kt b/compose/src/main/java/br/zup/com/nimbus/compose/core/ui/internal/NimbusDisposableEffect.kt
@@ -21,8 +21,8 @@ internal fun NimbusDisposableEffect(
                 Lifecycle.Event.ON_START -> onStart()
                 Lifecycle.Event.ON_CREATE -> onCreate()
                 Lifecycle.Event.ON_DESTROY -> onDestroy()
+                else -> {}
             }
-
         }
 
         // Add the observer to the lifecycle

diff --git a/processor/src/main/java/com/zup/nimbus/processor/ServerDrivenProcessor.kt b/processor/src/main/java/com/zup/nimbus/processor/ServerDrivenProcessor.kt
@@ -24,11 +24,9 @@ import com.squareup.kotlinpoet.asTypeName
 import kotlin.reflect.KClass
 
 class ServerDrivenProcessor(private val environment: SymbolProcessorEnvironment): SymbolProcessor {
-    private fun Resolver.findAnnotations(
-        kClass: KClass<*>,
-    ) = getSymbolsWithAnnotation(
-        kClass.qualifiedName.toString())
-        .filterIsInstance<KSFunctionDeclaration>()
+    private fun Resolver.findAnnotations(kClass: KClass<*>) =
+        getSymbolsWithAnnotation(kClass.qualifiedName.toString())
+            .filterIsInstance<KSFunctionDeclaration>()
 
     private fun createNimbusComposable(
         builder: FileSpec.Builder,
@@ -40,16 +38,15 @@ class ServerDrivenProcessor(private val environment: SymbolProcessorEnvironment)
             .addAnnotation(ClassNames.Composable)
             .addParameter("data", ClassNames.ComponentData)
             .addStatement("var nimbus = NimbusTheme.nimbus")
-            .addStatement("val properties = remember { " +
-                    "ComponentDeserializer(logger = nimbus.logger, node = data.node) }")
+            .addStatement("val properties = remember { ComponentDeserializer(logger = nimbus.logger, node = data.node) }")
             .addStatement("properties.start()")
 
         component.parameters.forEach {
             if (it.isParentName) {
                 if (!it.nullable) throw RequiredParentException(it.name, fn)
                 fnBuilder.addStatement("val %L = data.parent?.component", it.name)
             } else if (it.deserializer != null) {
-                if(it.deserializer.packageName != fn.packageName.asString()) {
+                if (it.deserializer.packageName != fn.packageName.asString()) {
                     builder.addClassImport(it.deserializer)
                 }
                 fnBuilder.addStatement(
@@ -80,17 +77,17 @@ class ServerDrivenProcessor(private val environment: SymbolProcessorEnvironment)
                     TypeCategory.ServerDrivenAction -> {
                         if (it.arity == 0) {
                             fnBuilder.addStatement(
-                                "val %LAction = properties.asAction%L(%S)",
+                                "val %LAction = remember(properties) { properties.asAction%L(%S) }",
                                 it.name,
                                 if (it.nullable) "OrNull" else "",
                                 it.name,
                             )
-                            val template = if (it.nullable) "val %L = %LAction?.let{ { it(null) } }"
-                            else "val %L = { %LAction(null) }"
+                            val template = if (it.nullable) "val %L = %LAction?.let{ remember(properties) { { it(null) } } }"
+                            else "val %L = remember(properties) { { %LAction(null) } }"
                             fnBuilder.addStatement(template, it.name, it.name)
                         } else {
                             fnBuilder.addStatement(
-                                "val %L = properties.asAction%L(%S)",
+                                "val %L = remember(properties) { properties.asAction%L(%S) }",
                                 it.name,
                                 if (it.nullable) "OrNull" else "",
                                 it.name,
@@ -139,11 +136,8 @@ class ServerDrivenProcessor(private val environment: SymbolProcessorEnvironment)
     ): Set<KSClassDeclaration> {
         val mustDeserialize = mutableSetOf<KSClassDeclaration>()
         val sourceFiles = functions.mapNotNull { it.containingFile }
-
-        val componentsFile = FileSpec.builder(
-            packageName,
-            "generatedComponents",
-        ).addClassImport(ClassNames.NimbusTheme)
+        val componentsFile = FileSpec.builder(packageName,"generatedComponents")
+            .addClassImport(ClassNames.NimbusTheme)
             .addClassImport(ClassNames.ComponentDeserializer)
             .addClassImport(ClassNames.NimbusMode)
             .addClassImport(ClassNames.Text)
@@ -157,10 +151,7 @@ class ServerDrivenProcessor(private val environment: SymbolProcessorEnvironment)
         if (mustDeserialize.isNotEmpty()) componentsFile.addClassImport(entityDeserializerRef)
 
         val file = environment.codeGenerator.createNewFile(
-            Dependencies(
-                false,
-                *sourceFiles.toList().toTypedArray(),
-            ),
+            Dependencies(false, *sourceFiles.toList().toTypedArray()),
             packageName,
             "generatedComponents"
         )
@@ -170,7 +161,8 @@ class ServerDrivenProcessor(private val environment: SymbolProcessorEnvironment)
     }
 
     fun createClassDeserializer(builder: FileSpec.Builder, clazz: KSClassDeclaration): FunSpec {
-        val name = "${clazz.packageName.asString()}.${clazz.simpleName.asString()}".replace(".", "_")
+        val name = "${clazz.packageName.asString()}.${clazz.simpleName.asString()}"
+            .replace(".", "_")
         val fnBuilder = FunSpec.builder(name)
             .addParameter("properties", ClassNames.ComponentDeserializer)
             .addModifiers(KModifier.PRIVATE)
@@ -180,7 +172,7 @@ class ServerDrivenProcessor(private val environment: SymbolProcessorEnvironment)
         )
         constructorInfo.parameters.forEach {
             if (it.deserializer != null) {
-                if(it.deserializer.packageName != clazz.packageName.asString()) {
+                if (it.deserializer.packageName != clazz.packageName.asString()) {
                     builder.addClassImport(it.deserializer)
                 }
                 fnBuilder.addStatement(
@@ -259,45 +251,51 @@ class ServerDrivenProcessor(private val environment: SymbolProcessorEnvironment)
         )
 
         val objectBuilder = TypeSpec.objectBuilder("NimbusEntityDeserializer")
-            .addProperty(PropertySpec.builder(
-                "deserializers",
-                ClassName("kotlin.collections", "MutableMap")
-                    .parameterizedBy(
-                        String::class.asTypeName(),
-                        LambdaTypeName.get(
-                            parameters = listOf(ParameterSpec.builder(
-                                "properties",
-                                ClassNames.ComponentDeserializer,
-                            ).build()),
-                            returnType = Any::class.asTypeName(),
-                        )
-                    ),
-                KModifier.PRIVATE,
-            ).initializer(
-                CodeBlock.of(
-                    "mutableMapOf(%L)",
-                    mustDeserialize.joinToString(", ") {
-                        val name = "${it.packageName.asString()}.${it.simpleName.asString()}"
-                        "\"$name\" to { ${name.replace(".", "_")}(it) }"
-                    },
+            .addProperty(
+                PropertySpec.builder(
+                    "deserializers",
+                    ClassName("kotlin.collections", "MutableMap")
+                        .parameterizedBy(
+                            String::class.asTypeName(),
+                            LambdaTypeName.get(
+                                parameters = listOf(
+                                    ParameterSpec.builder(
+                                        "properties",
+                                        ClassNames.ComponentDeserializer,
+                                    ).build()
+                                ),
+                                returnType = Any::class.asTypeName(),
+                            )
+                        ),
+                    KModifier.PRIVATE,
+                )
+                .initializer(
+                    CodeBlock.of(
+                        "mutableMapOf(%L)",
+                        mustDeserialize.joinToString(", ") {
+                            val name = "${it.packageName.asString()}.${it.simpleName.asString()}"
+                            "\"$name\" to { ${name.replace(".", "_")}(it) }"
+                        },
+                    )
                 )
-            ).build())
+                .build()
+            )
             .addFunction(
                 FunSpec.builder("deserialize")
-                    .addTypeVariables(listOf(
-                        TypeVariableName("T"),
-                        TypeVariableName(
-                            "U",
-                            listOf(KClass::class.asClassName().parameterizedBy(
-                                TypeVariableName("T")
-                            ))
+                    .addTypeVariables(
+                        listOf(
+                            TypeVariableName("T"),
+                            TypeVariableName(
+                                "U",
+                                listOf(
+                                    KClass::class.asClassName()
+                                        .parameterizedBy(TypeVariableName("T"))
+                                )
+                            )
                         )
-                    ))
-                    .addParameter("properties", ClassNames.ComponentDeserializer)
-                    .addParameter(
-                        "clazz",
-                        TypeVariableName("U"),
                     )
+                    .addParameter("properties", ClassNames.ComponentDeserializer)
+                    .addParameter("clazz", TypeVariableName("U"))
                     .returns(TypeVariableName("T"))
                     .addStatement(
                         "return deserializers.get(clazz.qualifiedName ?: \"\")?.let " +
@@ -327,7 +325,7 @@ class ServerDrivenProcessor(private val environment: SymbolProcessorEnvironment)
     override fun process(resolver: Resolver): List<KSAnnotated> {
         val functions: Sequence<KSFunctionDeclaration> =
             resolver.findAnnotations(ServerDrivenComponent::class)
-        if(!functions.iterator().hasNext()) return emptyList()
+        if (!functions.iterator().hasNext()) return emptyList()
 
         val mustDeserialize = mutableSetOf<KSClassDeclaration>()
         val byPackage = functions.groupBy { it.packageName.asString() }