Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[FR]: Minimize Unnecessary Recompositions #1353

Closed
2 tasks done
KDW03 opened this issue Apr 2, 2024 · 1 comment · Fixed by #1381
Closed
2 tasks done

[FR]: Minimize Unnecessary Recompositions #1353

KDW03 opened this issue Apr 2, 2024 · 1 comment · Fixed by #1381
Assignees
Labels
enhancement New feature or request

Comments

@KDW03
Copy link
Contributor

KDW03 commented Apr 2, 2024

Is there an existing issue for this?

  • I have searched the existing issues

Describe the problem

In current implementation of NiaApp, we've observed an issue where unrelated state changes, such as modifications to showSettingsDialog, trigger unnecessary recompositions across the entire app, including within NiaNavHost and its child screens

Upon investigation, we identified that lambda expressions, such as onTopicClick, are being recreated with each recomposition cycle. Compose treats these newly instantiated lambda expressions as different from their predecessors, thus triggering recompositions even when there are no pertinent state changes.

image

this behavior will persist until the Jetpack Compose Compiler is updated to version 1.7, which is expected to introduce an enhanced 'Strong skipping' mode for more efficient recomposition skipping. In the meantime, developers are advised to manage lambda instances manually using remember to mitigate unnecessary recompositions.

This is particularly evident in screens like ForYouScreen, where the onTopicClick lambda is passed down and utilized. Despite showSettingsDialog having no direct relation to these screens, they undergo recomposition, leading to performance inefficiencies.

Describe the solution

I propose to mitigate this issue by employing the remember construct to cache navigation-related lambda expressions within NiaNavHost. This approach will ensure that lambdas are only recreated when genuinely necessary, such as when the NavController instance changes

change

image

before

image

after

image

Additional context

No response

Code of Conduct

  • I agree to follow this project's Code of Conduct
@bentrengrove
Copy link
Member

This was a great investigation and your conclusion is correct! We will be enabling strong skipping mode in this project very soon so it isn't worth implementing the fix to manually remember lambdas. It would actually introduce a small amount of tech debt as the remember would become redundant once strong skipping is enabled.

I'll close this issue out but thank you for the report.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
2 participants