Fixed
Status Update
No update yet.
Comments
lp...@google.com
I'm making a small change (and adding some detailed benchmarks) to move the cost of first-ripple-composition (creating the view* machinery we need to integrate with the system) to first-ripple-press - this should help reduce the performance overhead in cases of multiple compose views in an application, such as in a recyclerview, but won't change much for the pure-compose case, since this cost only applies once per root compose view.
In general though, we already delay a lot of things to only be created on a press (such as creating a RippleDrawable a view to host the ripple drawable so we can render it in Compose) - it might be helpful to consider the objects in the images you sent as the analog to a RippleDrawable - an item can't know that it can ripple and needs to do work on press if we don't tell it that it can ripple and to react to presses. There's probably some allocations within this we can trim, but for the general case I think this is pretty much to be expected - at some point in the code we will need something to say 'when I am pressed, create and start drawing a ripple' - and this is basically what AndroidRippleIndicationInstance does
ap...@google.com
Project: platform/frameworks/support
Branch: androidx-main
commit 4f9d066546cb052122987fde605ec4e8fa7d474e
Author: Louis Pullen-Freilich <lpf@google.com>
Date: Tue Jul 11 16:18:20 2023
Lazily creates RippleContainer on first ripple
This delays the creation of RippleContainer (and adding it to the ComposeView) until the first ripple within a ComposeView needs to be drawn. For pure-Compose apps this won't have that much of an effect because this only needs to happen once, but for applications with many ComposeViews (such as a RecyclerView with ComposeViews inside) this will reduce the cost of using a ripple in the hierarchy by a decent amount.
Also adds a RippleBenchmark to track performance.
Benchmark numbers on a Pixel 2 XL for this change:
Before:
1,039,865 ns 171 allocs Trace RippleBenchmark.initialEmitPressInteraction
483,073 ns 122 allocs Trace RippleBenchmark.additionalEmitPressInteraction
267,431 ns 144 allocs Trace RippleBenchmark.rememberRippleFirstComposition
369,070 ns 190 allocs Trace RippleBenchmark.additionalRippleRememberUpdatedInstanceFirstComposition
495,369 ns 220 allocs Trace RippleBenchmark.initialRippleRememberUpdatedInstanceFirstComposition
387,528 ns 147 allocs Trace RippleBenchmark.initialEmitHoverInteraction
218,741 ns 141 allocs Trace RippleBenchmark.additionalEmitHoverInteraction
After:
1,252,022 ns 201 allocs Trace RippleBenchmark.initialEmitPressInteraction
487,709 ns 122 allocs Trace RippleBenchmark.additionalEmitPressInteraction
275,847 ns 144 allocs Trace RippleBenchmark.rememberRippleFirstComposition
326,985 ns 190 allocs Trace RippleBenchmark.additionalRippleRememberUpdatedInstanceFirstComposition
367,563 ns 191 allocs Trace RippleBenchmark.initialRippleRememberUpdatedInstanceFirstComposition
372,578 ns 147 allocs Trace RippleBenchmark.initialEmitHoverInteraction
214,901 ns 141 allocs Trace RippleBenchmark.additionalEmitHoverInteraction
So essentially this change just moves some overhead from composing the first ripple instance to drawing the first ripple instance (RippleBenchmark.initialRippleRememberUpdatedInstanceFirstComposition to RippleBenchmark.initialEmitPressInteraction).
Fixes:
Bug:
Test: RippleBenchmark
Change-Id: I0c1074c39d2da867e04c23fe1dbe76a77e332dff
A compose/material/material-ripple/benchmark/build.gradle
A compose/material/material-ripple/benchmark/src/androidTest/AndroidManifest.xml
A compose/material/material-ripple/benchmark/src/androidTest/java/androidx/compose/material/ripple/benchmark/RippleBenchmark.kt
A compose/material/material-ripple/benchmark/src/main/AndroidManifest.xml
M compose/material/material-ripple/src/androidMain/kotlin/androidx/compose/material/ripple/Ripple.android.kt
M compose/ui/ui-inspection/src/androidTest/java/androidx/compose/ui/inspection/testdata/RippleTestActivity.kt
M settings.gradle
https://android-review.googlesource.com/2653922
Branch: androidx-main
commit 4f9d066546cb052122987fde605ec4e8fa7d474e
Author: Louis Pullen-Freilich <lpf@google.com>
Date: Tue Jul 11 16:18:20 2023
Lazily creates RippleContainer on first ripple
This delays the creation of RippleContainer (and adding it to the ComposeView) until the first ripple within a ComposeView needs to be drawn. For pure-Compose apps this won't have that much of an effect because this only needs to happen once, but for applications with many ComposeViews (such as a RecyclerView with ComposeViews inside) this will reduce the cost of using a ripple in the hierarchy by a decent amount.
Also adds a RippleBenchmark to track performance.
Benchmark numbers on a Pixel 2 XL for this change:
Before:
1,039,865 ns 171 allocs Trace RippleBenchmark.initialEmitPressInteraction
483,073 ns 122 allocs Trace RippleBenchmark.additionalEmitPressInteraction
267,431 ns 144 allocs Trace RippleBenchmark.rememberRippleFirstComposition
369,070 ns 190 allocs Trace RippleBenchmark.additionalRippleRememberUpdatedInstanceFirstComposition
495,369 ns 220 allocs Trace RippleBenchmark.initialRippleRememberUpdatedInstanceFirstComposition
387,528 ns 147 allocs Trace RippleBenchmark.initialEmitHoverInteraction
218,741 ns 141 allocs Trace RippleBenchmark.additionalEmitHoverInteraction
After:
1,252,022 ns 201 allocs Trace RippleBenchmark.initialEmitPressInteraction
487,709 ns 122 allocs Trace RippleBenchmark.additionalEmitPressInteraction
275,847 ns 144 allocs Trace RippleBenchmark.rememberRippleFirstComposition
326,985 ns 190 allocs Trace RippleBenchmark.additionalRippleRememberUpdatedInstanceFirstComposition
367,563 ns 191 allocs Trace RippleBenchmark.initialRippleRememberUpdatedInstanceFirstComposition
372,578 ns 147 allocs Trace RippleBenchmark.initialEmitHoverInteraction
214,901 ns 141 allocs Trace RippleBenchmark.additionalEmitHoverInteraction
So essentially this change just moves some overhead from composing the first ripple instance to drawing the first ripple instance (RippleBenchmark.initialRippleRememberUpdatedInstanceFirstComposition to RippleBenchmark.initialEmitPressInteraction).
Fixes:
Bug:
Test: RippleBenchmark
Change-Id: I0c1074c39d2da867e04c23fe1dbe76a77e332dff
A compose/material/material-ripple/benchmark/build.gradle
A compose/material/material-ripple/benchmark/src/androidTest/AndroidManifest.xml
A compose/material/material-ripple/benchmark/src/androidTest/java/androidx/compose/material/ripple/benchmark/RippleBenchmark.kt
A compose/material/material-ripple/benchmark/src/main/AndroidManifest.xml
M compose/material/material-ripple/src/androidMain/kotlin/androidx/compose/material/ripple/Ripple.android.kt
M compose/ui/ui-inspection/src/androidTest/java/androidx/compose/ui/inspection/testdata/RippleTestActivity.kt
M settings.gradle
ap...@google.com
Project: platform/frameworks/support
Branch: androidx-main
commit 1097aa74463157195258e7fb2c590e1e8dcf2edc
Author: Louis Pullen-Freilich <lpf@google.com>
Date: Tue Dec 12 18:49:36 2023
Supports lazily creating indication in clickable
This CL adds a new parameter to clickable and combinedClickable, lazilyCreateIndication, and changes the interactionSource parameter type to be nullable. When lazilyCreateIndication is `true` (by default if interactionSource is null, and indication is an IndicationNodeFactory), an internal MutableInteractionSource will be lazily created along with the indication instance, delaying work until there is an incoming Interaction. This reduces the amount of work that needs to be done during composition - if the clickable never produces an interaction (it is never focused / hovered / pressed), then the indication instance never needs to be created.
For components that observe the provided InteractionSource, but never emit events, they can explicitly pass `true` to lazilyCreateIndication to opt in to the same behavior.
To take advantage of this change, higher level components should change from remembering a default InteractionSource to defaulting to null in their API signature. Components that explicitly observe a hoisted InteractionSource should consider providing `true` to lazilyCreateIndication if the InteractionSource is a) created internally by the component b) only observed, and never emitted to.
Bug:
Fixes:
Test: ClickableTest
Relnote: "Clickable and combinedClickable's interactionSource parameter has been made nullable, and a new parameter has been added: lazilyCreateIndication. When lazilyCreateIndication is `true` (by default if interactionSource is null, and indication is an IndicationNodeFactory), an internal MutableInteractionSource will be lazily created along with the indication instance, delaying work until there is an incoming Interaction. To take advantage of this change, it is recommended that you:
1. Migrate away from the deprecated Indication#rememberUpdatedInstance API if you haven't already, as this optimization is only possible for IndicationNodeFactory implementations
2. Explicitly pass indication to clickable and pass null for interactionSource if you aren't observing it yourself. If you are observing the interactionSource, but you are never emitting an Interaction, you can explicitly set `true` for lazilyCreateIndication.
3. Prefer the clickable overload with a required MutableInteractionSource and Indication parameter, and explicitly pass LocalIndication.current if you are calling this modifier only within composition, as this is more performant than the other clickable overload which needs to use Modifier.composed to retrieve LocalIndication."
Change-Id: Ie0699791a5e3014ff551624cfd001958e6b43e84
M compose/foundation/foundation/api/current.txt
M compose/foundation/foundation/api/restricted_current.txt
M compose/foundation/foundation/src/androidInstrumentedTest/kotlin/androidx/compose/foundation/ClickableTest.kt
M compose/foundation/foundation/src/androidInstrumentedTest/kotlin/androidx/compose/foundation/CombinedClickableTest.kt
M compose/foundation/foundation/src/commonMain/kotlin/androidx/compose/foundation/Clickable.kt
M compose/foundation/foundation/src/commonMain/kotlin/androidx/compose/foundation/Hoverable.kt
M compose/ui/ui/benchmark/src/androidTest/java/androidx/compose/ui/benchmark/ModifiersBenchmark.kt
https://android-review.googlesource.com/2870890
Branch: androidx-main
commit 1097aa74463157195258e7fb2c590e1e8dcf2edc
Author: Louis Pullen-Freilich <lpf@google.com>
Date: Tue Dec 12 18:49:36 2023
Supports lazily creating indication in clickable
This CL adds a new parameter to clickable and combinedClickable, lazilyCreateIndication, and changes the interactionSource parameter type to be nullable. When lazilyCreateIndication is `true` (by default if interactionSource is null, and indication is an IndicationNodeFactory), an internal MutableInteractionSource will be lazily created along with the indication instance, delaying work until there is an incoming Interaction. This reduces the amount of work that needs to be done during composition - if the clickable never produces an interaction (it is never focused / hovered / pressed), then the indication instance never needs to be created.
For components that observe the provided InteractionSource, but never emit events, they can explicitly pass `true` to lazilyCreateIndication to opt in to the same behavior.
To take advantage of this change, higher level components should change from remembering a default InteractionSource to defaulting to null in their API signature. Components that explicitly observe a hoisted InteractionSource should consider providing `true` to lazilyCreateIndication if the InteractionSource is a) created internally by the component b) only observed, and never emitted to.
Bug:
Fixes:
Test: ClickableTest
Relnote: "Clickable and combinedClickable's interactionSource parameter has been made nullable, and a new parameter has been added: lazilyCreateIndication. When lazilyCreateIndication is `true` (by default if interactionSource is null, and indication is an IndicationNodeFactory), an internal MutableInteractionSource will be lazily created along with the indication instance, delaying work until there is an incoming Interaction. To take advantage of this change, it is recommended that you:
1. Migrate away from the deprecated Indication#rememberUpdatedInstance API if you haven't already, as this optimization is only possible for IndicationNodeFactory implementations
2. Explicitly pass indication to clickable and pass null for interactionSource if you aren't observing it yourself. If you are observing the interactionSource, but you are never emitting an Interaction, you can explicitly set `true` for lazilyCreateIndication.
3. Prefer the clickable overload with a required MutableInteractionSource and Indication parameter, and explicitly pass LocalIndication.current if you are calling this modifier only within composition, as this is more performant than the other clickable overload which needs to use Modifier.composed to retrieve LocalIndication."
Change-Id: Ie0699791a5e3014ff551624cfd001958e6b43e84
M compose/foundation/foundation/api/current.txt
M compose/foundation/foundation/api/restricted_current.txt
M compose/foundation/foundation/src/androidInstrumentedTest/kotlin/androidx/compose/foundation/ClickableTest.kt
M compose/foundation/foundation/src/androidInstrumentedTest/kotlin/androidx/compose/foundation/CombinedClickableTest.kt
M compose/foundation/foundation/src/commonMain/kotlin/androidx/compose/foundation/Clickable.kt
M compose/foundation/foundation/src/commonMain/kotlin/androidx/compose/foundation/Hoverable.kt
M compose/ui/ui/benchmark/src/androidTest/java/androidx/compose/ui/benchmark/ModifiersBenchmark.kt
lp...@google.com
In the optimized case (which most components like Button fall into) we now don't create the ripple at all, until we receive our first press / focus / hover event - which results in a ~40% improvement in the optimal case micro benchmark
Description
For example, imagine a typical app with a scrolling list of clickable items (or, worse, a list with items containing many clickable sub-items). LazyListActivity, in the Compose tests, for example. When the user scrolls or flings that list, every item creates several Ripple objects. You can see this in the first two attached screenshots, taken in the memory profiler, filtered for items with "ripple" in the name. One show the items created when a single item appears, the other shows the objects created when 6 items appear. Note that this only shows Ripple objects, not the indirect object creation also happening (since some of these ripple objects create other objects internally, such as Color, shown in the third screenshot).
But the user is not typically clicking on all of those items, but is rather scrolling over them to view them, after which they might click on one of them. So we've created tons of ripples, only to throw them away. We've saved time for that single click operation, but traded it off against the time and memory used for every single clickable item created, whether or not it will be clicked in the future.
It is worth investigating whether we can delay at least some of this work until later. For example, can we create ripples lazily, on the first click operation, instead of at creation time? This would save us time on every single clickable item, and would save the allocations for those objects (and the objects created indirectly). Avoiding allocations for all of these objects would be helpful for later GC collections (and memory pressure overall).