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

Fix memory leak in UpdateEffect #325

Merged
merged 1 commit into from Nov 10, 2022
Merged

Conversation

igordmn
Copy link
Collaborator

@igordmn igordmn commented Nov 10, 2022

If there are animations on the screen, after some time we will encounter performance degradation, and increased memory consumption.

This fix is similar to the other fix in the past (https://android-review.googlesource.com/c/platform/frameworks/support/+/1398690). We have to remember the lambda instead of creating it each time.

Fixes JetBrains/compose-multiplatform#2455
Fixes JetBrains/compose-multiplatform#1969

The tests are not very deterministic, but it seems better to have them than not to have. The second test fails before the fix, and passes after the fix.
We will see if they will be stable on our CI or not. If they will be flaky, we will tune them, or remove them.

P.S. the code of the test's application (everything above AppWindow) is just copied from the run1 example

If there are animations on the screen, after some time we will encounter performance degradation, and an increased memory consumption.

This fix is similar to the other fix in the past (https://android-review.googlesource.com/c/platform/frameworks/support/+/1398690). We have to remember the lambda instead of creating it each time.

Fixes JetBrains/compose-multiplatform#2455
Fixes JetBrains/compose-multiplatform#1969

The tests are not very deterministic, but it seems better to have them than not to have. The second test fails before the fix, and passes after the fix.
We will see if they will be stable on our CI or not. If they will be flaky, we will tune them, or remove them.
@igordmn igordmn requested a review from eymar November 10, 2022 02:44
@igordmn igordmn merged commit bded090 into jb-main Nov 10, 2022
@igordmn igordmn deleted the fix-memory-leak-in-update-effect branch November 10, 2022 20:53
eymar pushed a commit that referenced this pull request Nov 16, 2022
If there are animations on the screen, after some time we will encounter performance degradation, and an increased memory consumption.

This fix is similar to the other fix in the past (https://android-review.googlesource.com/c/platform/frameworks/support/+/1398690). We have to remember the lambda instead of creating it each time.

Fixes JetBrains/compose-multiplatform#2455
Fixes JetBrains/compose-multiplatform#1969

The tests are not very deterministic, but it seems better to have them than not to have. The second test fails before the fix, and passes after the fix.
We will see if they will be stable on our CI or not. If they will be flaky, we will tune them, or remove them.
igordmn added a commit that referenced this pull request Dec 5, 2022
If there are animations on the screen, after some time we will encounter performance degradation, and an increased memory consumption.

This fix is similar to the other fix in the past (https://android-review.googlesource.com/c/platform/frameworks/support/+/1398690). We have to remember the lambda instead of creating it each time.

Fixes JetBrains/compose-multiplatform#2455
Fixes JetBrains/compose-multiplatform#1969

The tests are not very deterministic, but it seems better to have them than not to have. The second test fails before the fix, and passes after the fix.
We will see if they will be stable on our CI or not. If they will be flaky, we will tune them, or remove them.
eymar pushed a commit that referenced this pull request Jan 13, 2023
If there are animations on the screen, after some time we will encounter performance degradation, and an increased memory consumption.

This fix is similar to the other fix in the past (https://android-review.googlesource.com/c/platform/frameworks/support/+/1398690). We have to remember the lambda instead of creating it each time.

Fixes JetBrains/compose-multiplatform#2455
Fixes JetBrains/compose-multiplatform#1969

The tests are not very deterministic, but it seems better to have them than not to have. The second test fails before the fix, and passes after the fix.
We will see if they will be stable on our CI or not. If they will be flaky, we will tune them, or remove them.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants