Recomposition & State issues - nothing gets `remember { ... }`'ed

[theIDinside]

Steps to reproduce

Preface: I don't know much Android-development at all and I have minimal Kotlin experience.

Irrelevant apropo

I ran into this issue, because I'm starting to implement a rudimentary syntax highlighting engine, similar to the one we can see on the normal web app.

The syntax highlighting stuff is trivial (and I do think the android app should mimick the browser/desktop version), whether one should handroll per language, or do some regex stuff - either way, reducing 3rd party dependencies is always a good thing. Always.

The thing is though, running syntax highlighting on a message should only be done once. So how would I go about doing this? Using remember { ... } right? Which brings me to:

Outcome

Actual issue

Every time recomposition happens, the remember { } stuff never actually remembers anything. This got me thinking; I have very little experience in the JVM/Android platform, so I might not be doing it right, at all.

But then I set some breakpoints, and it seems, this functionality is not working as expected anywhere.

For instance, in the TimelineItemTextView composable, there's a remember(content.body) - and it gets called every time recomposition happens, making it completely unnecessary to have it in a remember block, no?

In other words, the data never changes, yet all these styles need to be applied every time, doing costly operations, over, and over, and over again.

Now, I'm not well-versed in Compose (or Kotlin for that matter) but this seems to be a design flaw, right? The client has the data, yet it has to go through all the hoops of parsing it into a document etc applying styles, etc.

The culprit seems to be TimelineView which refires every recomposition and thus forces everything below it to update as well. Other remember calls actually remembers their state, but everything "below" this point, remember seem to not work at all.

Now, it makes sense that recomposition should happen, when scrolling, but every subsequent child in that "Timeline " has their operations applied fully, not just their positions or layout in the UI, but as previously show, the linkify stuff as well (and in my case - the reparsing of the syntax + applying colors on "code blocks"), the traversing of already parsed html, etc. The data of all the subcomponents should only be calculated once, it seems to me, only the UI should care.

Is this the intended design? What can be done about it? If anyone has ideas on how to solve it?

Your phone model

Pixel 7 (Emulator)

Operating system version

Doesn't matter.

Application version and app store

develop branch

Homeserver

matrix.org

Will you send logs?

No

Are you willing to provide a PR?

Yes

[theIDinside]

I think we can close this as just me being stupid.

Obviously, the solution would be to extend TimelineItem interface to actually hold any annotated / processed data in an optional (nullable) value and check this each time during recomposition instead. I think.

[Edit]: Well, technically, the composing stuff needs a pretty big overhaul. Right now, it does all non-rendering related logic, every single time it recomposes. Building AnnotatedString from Element if they are a HTML List item etc. This obviously needs to be refactored entirely as this should only ever be done once then have that result passed around to the different compositing functions; so they can do what their name implies; compose (only). (In the case of lists in a markdown document, or headers, or whatever, the "result" here means the final AnnotatedString not the Element or HTML-Document as that means, the logic would have to to run on each recomposition, to convert it finally to an AnnotatedString, which can be several hundred times per frame, according to the docs on Jetpack Compose)

Some feedback would be nice on this, as I'd be glad to give it a go. It would require some major overhaul though.

[theIDinside]

Here are examples of linkify, being triggered over and over again when the user scroll. And as you can see, it's being called on different string objects each time.

So there's some pretty major performance issues currently and I don't quite understand why it's behaving like this. The TimelineItem are immutable, etc, the Document that gets parsed, doesn't change identity (i.e. it's the same object, with the same address in memory), yet and still, HtmlBlock and all it's cousins can not remember anything on scroll.

linkify_triggered.webm

[theIDinside]

I think I might have a solution to this. In TimelineItemContentMessageFactory instead of just creating TimelineItemEventContent with it's string contents, process each type there, so that the respective HtmlBody and cousins, etc, all take AnnotatedString (or whatever other processed data it desires) which is immutable inside the now extended TimelineItemEventContent, instead of processing it each time during recomposition.

Then, adding functionality, becomes a question of extending TimelineItemContentMessageFactory so that it creates and processes objects.

It's the only solution I can think of, seeing as how saving state across the application like the post-processing of TimelineItem's string content (in the case of linkify, for instance) etc, becomes extremely hard to reason about otherwise.

[theIDinside]

In discussions on the matrix channel (element-android) - I might have found out what the issue is.

Element-X manually keeps calling code like TimelineItemRow in itemsIndexed in the TimelineView file. This does not trigger recomposition. This (possibly? It's hard to understand the Jetpack Compose docs really) triggers a full invalidation of the entire tree, since it actually adds a new composition with it's own life time. This is (probably) even worse than I expected? Here's from the docs about life cycles:

And it's this code that specifically is the problem.

[ganfra]

Thanks for the insights. You're right using remember inside a LazyColumn doesn't really "work" as it composes from scratch each time it leaves and reenter the visible aera. But the Timeline is still in his PoC state as we didn't spent time yet to rework it. So it'll change in the future, no worry!

[theIDinside]

No problem! I would love to contribute to this project one way or the other and so I figured I'd create this issue.