Performance refactors for Posts feed #1527
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
# Conflicts: # app/build.gradle.kts # app/src/main/java/com/jerboa/ui/components/home/HomeActivity.kt
dessalines
reviewed
May 31, 2024
There was a problem hiding this comment.
Check the comments below, but my biggest concern is the fact that there are now two lists to keep track of, rather than a single snapshotstatelist.
Also the remembers are potentially bug-worthy, since source data changes might not be reflected properly. If they're necessary that's okay, but maybe they should be keyed.
app/src/main/java/com/jerboa/ui/components/community/CommunityLink.kt
Outdated
Show resolved
Hide resolved
app/src/main/java/com/jerboa/ui/components/home/HomeActivity.kt
Outdated
Show resolved
Hide resolved
# Conflicts: # app/src/main/java/com/jerboa/model/PostsViewModel.kt
dessalines
reviewed
May 31, 2024
dessalines
reviewed
May 31, 2024
# Conflicts: # app/src/main/java/com/jerboa/ui/components/post/PostListing.kt # app/src/main/java/com/jerboa/ui/components/post/composables/PostOptionsDropdown.kt
# Conflicts: # app/src/main/java/com/jerboa/ui/components/community/CommunityActivity.kt # app/src/main/java/com/jerboa/ui/components/home/HomeActivity.kt # app/src/main/java/com/jerboa/ui/components/post/PostActivity.kt # app/src/main/java/com/jerboa/ui/components/post/PostListings.kt
dessalines
approved these changes
Jun 1, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Refactor to use
SnapshotStateListinstead ofListBefore what we did was create a new list each time we change something in that list. Since compose checks on equality the list is now different and fully recomposes the entire list. Instead use
SnapshotStateListwhich emits events for only the changes made to the list.Many changes have made to the postlisting so that when a vote changes it doesn't recompose the entire postlisting but just the votes. This was only successful for postlisting as the card modes use postview to deeply into the "onAction" lambdas which trigger recompose as the postview has different equality.
It no longer recomposes the feed when adding/updating postviews
The collapsible header also causes recompositions as it changes the innerpadding which is applied to the lazy column. Not sure if anything can be done for that. (But only when it moves, so when u scroll down once its up, no recompositions happen)(This has been solved)I am also going to refactor the comments. (I'll do that in a separate PR)