[perf vs sanity] We're in the process of moving up common modifiers to re-use them, and docs also ad...

# compose-android

[ascii]

[perf vs sanity] We're in the process of moving up common modifiers to re-use them, and docs also advise this: https://developer.android.com/jetpack/compose/modifiers#reusing-modifiers.

For simple modifier chains like fillMaxSize, padding, etc. it's an easy decision. In our codebase there are multiple composables in a screen that are almost guaranteed to use the same chain, so moving them up is a no brainer. Same instance, fewer allocations, free tiny bit of performance.

But then while refactoring we came across a monstrosity (screenshot). More in thread.

[ascii]

This particular composable runs animations, so it is definitely a good idea to pass the same unchanged modifiers from a parent scope.

However, this is getting a little ridiculous, especially because we've become used to the idea that modifiers define the layout of the UI they're a part of. Moving them up introduces two code quality problems:
• earlier everything looked so neat, we just had composables & states, but now quite a bit of space is polluted with modifier definitions
• this takes away the identity of the composable (and all its subcomposables), because now when someone goes through the code, they no longer see definitive modifiers as part of the composable itself. Instead it's somewhere else, making them work a bit harder to find it and change it.
What I mean is, earlier they could scroll through and be like "oh yes, this text is offset to that icon and this overlay adds this conditional background etc".

But now? They just see raw composable calls and they're aloof about how its structured and how it ends up looking (sure, previews, but in a pinch code should be enough to mentally visualize).

[ascii]

Has anyone done such refactoring, and compared benchmark results? Is there a meaningful difference? We're debating if we should abandon this idea. This poor composable had 4 params earlier, all of which were crucial to its responsibility. Now there's 6 more params and they're all just modifiers.

[ascii]

Also, is it wise to move common modifiers out of Lazy* items? Would that have the same perf benefit as the animation example — i.e. are they getting re-allocated for each item or is Compose doing some magic where it's no longer a concern?

[Ben Trengrove [G]]

I don't think it's worth it but I would be keen to hear if you do measure it. Especially if all those modifiers are backed by Modifier.Node. If they are Modifier.Node modifiers, you are saving the allocation of the element class for each modifier which really is pretty small compared with the complexity you are adding to your code.

[Landry Norris]

Can you just store the modifier chain in a global variable in the same file as the composables? It sounds like you don't really need the flexibility of params, so a global modifier chain has the same flexibility as hardcoding it.

[Filip Wiesner]

Isn't passing multiple modifiers to a composable against the Compose API design guidelines? Also a lot of times you'd need to put some modifier in the middle of the chain because order matters.
Honestly, this just feels like you are making your life miserable for some small performance improvement, which it feels like you didn't even need from the beginning when you asked, "_Is there a meaningful difference?_" just now.

[ascii]

Correct, but rather than do it later when codebase may become more complicated, it's better to establish a standard early in our team. Anyway, as we can't find enough real world examples of people adopting this idea, we've decided to do it only for simple one- or two-liner chains.