Skip to content

Commit

Permalink
Avoid memoizing lambdas with captured delegates
Browse files Browse the repository at this point in the history
Prevents memoization of internal delegate variables with strong skipping enabled.

Fixes: [342557697](https://issuetracker.google.com/342557697)
Relnote: Stop memoizing lambdas with captured property delegates.
  • Loading branch information
ShikaSD authored and Space Cloud committed Jun 11, 2024
1 parent 329717c commit d6ac8a5
Show file tree
Hide file tree
Showing 23 changed files with 865 additions and 75 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,10 @@

package androidx.compose.compiler.plugins.kotlin

import kotlin.test.assertFalse
import kotlin.test.assertTrue
import org.junit.Assume.assumeFalse
import org.junit.Test
import kotlin.test.assertFalse
import kotlin.test.assertTrue

/* ktlint-disable max-line-length */
class ComposeBytecodeCodegenTest(useFir: Boolean) : AbstractCodegenTest(useFir) {
Expand Down Expand Up @@ -655,4 +655,35 @@ class ComposeBytecodeCodegenTest(useFir: Boolean) : AbstractCodegenTest(useFir)
)
}
)

@Test
fun testMemoizingFromDelegate() = testCompile(
"""
import androidx.compose.runtime.*
class ClassWithData(
val action: Int = 0,
)
fun getData(): ClassWithData = TODO()
@Composable
fun StrongSkippingIssue(
data: ClassWithData
) {
val state by remember { mutableStateOf("") }
val action by data::action
val action1 by getData()::action
{
action
}
{
action1
}
{
state
}
}
"""
)
}
Original file line number Diff line number Diff line change
Expand Up @@ -743,4 +743,36 @@ class LambdaMemoizationTransformTests(useFir: Boolean) : AbstractIrTransformTest
}
"""
)

@Test
fun testMemoizingFromDelegate() = verifyGoldenComposeIrTransform(
extra = """
class ClassWithData(
val action: Int = 0,
)
fun getData(): ClassWithData = TODO()
""",
source = """
import androidx.compose.runtime.*
@Composable
fun StrongSkippingIssue(
data: ClassWithData
) {
val state by remember { mutableStateOf("") }
val action by data::action
val action1 by getData()::action
{
action
}
{
action1
}
{
state
}
}
"""
)
}
Original file line number Diff line number Diff line change
Expand Up @@ -368,6 +368,49 @@ class StrongSkippingModeTransformTests(
"""
)

@Test
fun testMemoizingFromReferenceDelegate() = verifyMemoization(
"""
class ClassWithData(
var action: Int = 0,
)
fun getData(): ClassWithData = TODO()
""",
"""
@Composable
fun StrongSkippingIssue(
data: ClassWithData
) {
val action by data::action
val action1 by getData()::action
{
action
}
{
action1
}
}
""",
)

@Test
fun testMemoizingFromStateDelegate() = verifyMemoization(
"""
""",
"""
import androidx.compose.runtime.mutableStateOf
import androidx.compose.runtime.remember
import androidx.compose.runtime.getValue
@Composable
fun StrongSkippingState() {
val state by remember { mutableStateOf("") }; // <-- this is a load bearing ;
{ state }
}
""",
)

private fun verifyMemoization(
@Language("kotlin")
unchecked: String,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,91 @@
//
// Source
// ------------------------------------------

import androidx.compose.runtime.*

@Composable
fun StrongSkippingIssue(
data: ClassWithData
) {
val state by remember { mutableStateOf("") }
val action by data::action
val action1 by getData()::action
{
action
}
{
action1
}
{
state
}
}

//
// Transformed IR
// ------------------------------------------

@Composable
fun StrongSkippingIssue(data: ClassWithData, %composer: Composer?, %changed: Int) {
%composer = %composer.startRestartGroup(<>)
sourceInformation(%composer, "C(StrongSkippingIssue)<rememb...>,<{>:Test.kt")
val %dirty = %changed
if (%changed and 0b1110 == 0) {
%dirty = %dirty or if (%composer.changed(data)) 0b0100 else 0b0010
}
if (%dirty and 0b1011 != 0b0010 || !%composer.skipping) {
if (isTraceInProgress()) {
traceEventStart(<>, %dirty, -1, <>)
}
val state by {
val state%delegate = <block>{
sourceInformationMarkerStart(%composer, <>, "CC(remember):Test.kt#9igjgp")
val tmp0_group = %composer.cache(false) {
mutableStateOf(
value = ""
)
}
sourceInformationMarkerEnd(%composer)
tmp0_group
}
get() {
return state%delegate.getValue(null, ::state%delegate)
}
}
val action by {
val action%delegate = data::action
get() {
return action%delegate.getValue(null, ::action%delegate)
}
}
val action1 by {
val action1%delegate = getData()::action
get() {
return action1%delegate.getValue(null, ::action1%delegate)
}
}
{
<get-action>()
}
{
<get-action1>()
}
sourceInformationMarkerStart(%composer, <>, "CC(remember):Test.kt#9igjgp")
val tmp1_group = %composer.cache(false) {
{
<get-state>()
}
}
sourceInformationMarkerEnd(%composer)
tmp1_group
if (isTraceInProgress()) {
traceEventEnd()
}
} else {
%composer.skipToGroupEnd()
}
%composer.endRestartGroup()?.updateScope { %composer: Composer?, %force: Int ->
StrongSkippingIssue(data, %composer, updateChangedFlags(%changed or 0b0001))
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,91 @@
//
// Source
// ------------------------------------------

import androidx.compose.runtime.*

@Composable
fun StrongSkippingIssue(
data: ClassWithData
) {
val state by remember { mutableStateOf("") }
val action by data::action
val action1 by getData()::action
{
action
}
{
action1
}
{
state
}
}

//
// Transformed IR
// ------------------------------------------

@Composable
fun StrongSkippingIssue(data: ClassWithData, %composer: Composer?, %changed: Int) {
%composer = %composer.startRestartGroup(<>)
sourceInformation(%composer, "C(StrongSkippingIssue)<rememb...>,<{>:Test.kt")
val %dirty = %changed
if (%changed and 0b1110 == 0) {
%dirty = %dirty or if (%composer.changed(data)) 0b0100 else 0b0010
}
if (%dirty and 0b1011 != 0b0010 || !%composer.skipping) {
if (isTraceInProgress()) {
traceEventStart(<>, %dirty, -1, <>)
}
val state by {
val state%delegate = <block>{
sourceInformationMarkerStart(%composer, <>, "CC(remember):Test.kt#9igjgp")
val tmp0_group = %composer.cache(false) {
mutableStateOf(
value = ""
)
}
sourceInformationMarkerEnd(%composer)
tmp0_group
}
get() {
return state%delegate.getValue(null, ::state%delegate)
}
}
val action by {
val action%delegate = data::action
get() {
return action%delegate.getValue(null, ::action%delegate)
}
}
val action1 by {
val action1%delegate = getData()::action
get() {
return action1%delegate.getValue(null, ::action1%delegate)
}
}
{
<get-action>()
}
{
<get-action1>()
}
sourceInformationMarkerStart(%composer, <>, "CC(remember):Test.kt#9igjgp")
val tmp1_group = %composer.cache(false) {
{
<get-state>()
}
}
sourceInformationMarkerEnd(%composer)
tmp1_group
if (isTraceInProgress()) {
traceEventEnd()
}
} else {
%composer.skipToGroupEnd()
}
%composer.endRestartGroup()?.updateScope { %composer: Composer?, %force: Int ->
StrongSkippingIssue(data, %composer, updateChangedFlags(%changed or 0b0001))
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -52,9 +52,11 @@ fun Test(%composer: Composer?, %changed: Int) {
TestMemoizedFun(TestFunInterface { it: Int ->
use(it)
}, %composer, 0b0110)
TestMemoizedFun(%composer.cache(false) {
TestFunInterface { it: Int ->
use(capture)
TestMemoizedFun(<block>{
%composer.cache(false) {
TestFunInterface { it: Int ->
use(capture)
}
}
}, %composer, 0)
if (isTraceInProgress()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,14 +49,18 @@ fun Test(%composer: Composer?, %changed: Int) {
traceEventStart(<>, %changed, -1, <>)
}
val capture = 0
TestMemoizedFun(%composer.cache(false) {
TestFunInterface { it: Int ->
use(it)
TestMemoizedFun(<block>{
%composer.cache(false) {
TestFunInterface { it: Int ->
use(it)
}
}
}, %composer, 0)
TestMemoizedFun(%composer.cache(false) {
TestFunInterface { it: Int ->
use(capture)
TestMemoizedFun(<block>{
%composer.cache(false) {
TestFunInterface { it: Int ->
use(capture)
}
}
}, %composer, 0)
if (isTraceInProgress()) {
Expand Down
Loading

0 comments on commit d6ac8a5

Please sign in to comment.