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

Gif memory leak #203

Closed
1 task done
terrysahaidak opened this issue Sep 8, 2023 · 8 comments
Closed
1 task done

Gif memory leak #203

terrysahaidak opened this issue Sep 8, 2023 · 8 comments

Comments

@terrysahaidak
Copy link

terrysahaidak commented Sep 8, 2023

New Issue Checklist

  • I have searched for a similar issue in the project and found none

Issue Info

Info Value
Device Info Pixel 7 pro
System Version 13.0
APNG4Android Library Version 2.24.0
Repro rate all the time (100%)
Repro with our demo project not sure
Demo project link https://github.com/terrysahaidak/expo-image-gifs-memory-leak

Issue Description and Steps

I was building an app with React Native and was using expo-image package, that uses Glide and this library to animate gifs. After some investigation, I realized that the only thing causing my memory leaks was this library. Commenting out this library implementation in Gradle solves all the memory leaks.

The expo-image package uses pretty much the same code as another library called react-native-fast-image that also uses glide, but the only difference between both of them is the usage of this library - the fast-image one animates gifs using glide, but the performance of it is not great by any means.

Taking a heap dump I saw that byte[] are never released from the memory, and it happens only with this library enabled.
They do clear the view the normal way you would do with glide: https://github.com/expo/expo/blob/main/packages/expo-image/android/src/main/java/expo/modules/image/ExpoImageViewWrapper.kt#L423-L429

After a quick investigation of the code, should this method be implemented somehow?
https://github.com/penfeizhou/APNG4Android/blob/master/plugin_glide/src/main/java/com/github/penfeizhou/animation/glide/FrameDrawableTranscoder.java#L101

Here is the original issue: expo/expo#24259
Here is the gradle file: https://github.com/expo/expo/blob/main/packages/expo-image/android/build.gradle#L103

@penfeizhou
Copy link
Owner

The memory will be released while the animation is being stopped.
In glide the animation's stop api should be called automatically.So we didn't do things inside recycle.
However,due to this issue,we will check this to see if something is wrong.

@terrysahaidak
Copy link
Author

I just tried to use APNG and it seems that there is no memory leak anymore. Previously i tried WEBP and had the same problem.
APNG is the only one that has implemented recycle method which stops the animation.
Not sure why glide doesn't stop the animation in my case.
https://github.com/penfeizhou/APNG4Android/blob/master/plugin_glide/src/main/java/com/github/penfeizhou/animation/glide/FrameDrawableTranscoder.java#L51

@terrysahaidak
Copy link
Author

2023-09-09.12.18.09.mov

@jingpeng
Copy link
Collaborator

Ok, we tried to detect this leak with your project and found that as below:
屏幕截图 2023-09-11 112216
Expo only start the animation and do not invoked the animation's stop method, so we implements the recylce method in glide as follows:
c3ec09f

@penfeizhou
Copy link
Owner

Update to v2.28.0 will resolve this problem.

@jingpeng
Copy link
Collaborator

expo/expo#24358 await the review and merge

@terrysahaidak
Copy link
Author

terrysahaidak commented Sep 11, 2023

That was quick! Thanks a lot! Appreciate it!

Somehow I missed the start call in the sources with the Animatable "if" block, was thinking about calling stop when they clear the view, but I had to do the if block there so the type would be Animatable to stop the animation too.

But I guess the recycle method is a more bullet proof way here.

@terrysahaidak
Copy link
Author

By the way, can confirm 2.28.0 fixes all the issues I had.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants