Interface IIcon is unstable.
Nek-12 opened this issue · 12 comments
About this issue
The IIcon parameter of the Image composable is unstable, which leads to unnecessary recompositions.
The IIcon interface is immutable, as all icons that inherit from it are immutable. However, Compose cannot derive the stability of the interface itself, leading to the composable being non-restartable / non-skippable.
IIcon is widely used in our project because of the api of the Image itself, and in our efforts to optimize performance, we discovered that the composables are recursively unstable because of us passing the IIcon everywhere.
How can we solve this problem?
I'm thinking about something like a wrapper value class that is going to be marked as @Immutable.
@JvmInline
@Immutable
value class StableIcon(val icon: IIcon) /* : IIcon by icon */
val IIcon.stable get() = StableIcon(this)Details
- Used library version 5.1.0
- Compose 1.6.0-alpha01
Thank you for the suggestion. This seems like a great proposal. I did not actually test such a usage of marking a type Immutable for compose. Did you check this improving optimisation in your project?
Actually have a similar ticket open for AboutLibraries: mikepenz/AboutLibraries#868
I wished the annotations were available in isolation, to annotate in the core lib without adding any additional overhead.
According to compose metrics, value classes work well and make the composable stable indeed. We're using the wrapper strategy for a lot of stuff right now and are migrating our code to StableIcon. The only thing that is questionable is delegating the interface, which is why I commented it out as an optional feature.
About the Image itself, since the composable is very thin, it doesn't need to be restartable/skippable it seems. For example, even the Painter interface from compose is unstable. The Image functions are annotated with @NonRestartableComposable which is made for thin wrappers around overload. We could use that annotation too. Another solution is making the Image function inline like Row/Column/Box, making Compose ignore it altogether.
The bigger problem is how we didn't notice such a small detail and ended up using the interface in a lot of places. That's what the original issue is about I guess. Preventing people from passing the interface up the hierarchy, where it would recursively make parent composables (which in our case can be very heavy on performance, like list items) nonskippable.
I maybe have something to try for you later today, which will not require the wrapping. Let's see
Ok released v5.5.0-compose01, which most likely will stay a one off, given I have to include the composeRuntime in the core or typeface-api module which is not the best idea.
Agree, I don't think we should include compose in the core module...
That being said given how stable the core and typeface modules have been. This may work for you, and won't require wrapping.
Especially as long term it looks compose got significantly better support for icons derived from SVGs
I'm sorry, I don't understand your last comment. Are you suggesting we move on from using the library? The main and probably the only reason we're using Iconics is to be able to load icons by their name. This is useful to us as icon IDs are stored in the database and can be changed by the user. I don't know any ways to accomplish this using androidx icons.
Not really, I was mostly thinking that a lot of use-cases which used to be the reason for people using this library got replaced by direct svg usage.
However your usecase still sounds quite valid, accessing wider part of the fonts.
With that, I really hope the 5.5.0-compose01 version helps to fix the stability problem you had noted (based on the checks I run it reported the class now as stable)
Purely for reference on the svg to compose converter I meant for example: https://www.composables.com/svgtocompose
Yeah this gives me an idea for a next version of this library, a script or a plugin that would convert the SVG/font to an enum, but would use built-in compose functionality (shape, path) for icons instead of falling back to an Iconics implementation. I don't know if you have resources for that though, so for now all we need is for the library to stay updated.