StoryMaker / storymaker

Android mobile app. Make your story great.
https://storymaker.org
GNU General Public License v2.0
69 stars 49 forks source link

Performance improvement suggestion #39

Open 00conan00 opened 10 years ago

00conan00 commented 10 years ago

Dear developers,

I am a big fan of storymaker, and recently I am writing a static code analysis tool to conduct performance analysis for Android apps. I found several violations of "resource leakage" patterns in storymaker's code. These violations could affect the performance of your app. Here is a representative one.

In info.guardianproject.mrapp.media.VideoCamera, the method onCreate() invokes "addCallback" to add a Callback interface for this holder. However, according to the suggestion of android developer, it is a good practice to use "removeCallback " to removes a previously added callback interface from this holder. Unfortunately, this principle is not correctly applied in this class.

So I am curious :) Looking forward to your response. Thanks.

References: resource leakage pattern: http://developer.android.com/reference/android/view/SurfaceHolder.html#addCallback(android.view.SurfaceHolder.Callback)

vitriolix commented 10 years ago

Thanks for opening the issue. We'll definitely fix this and any others you find. As the app is approaching stability we are going to be focusing some energy on performance so finds like this will surely help.

vitriolix commented 10 years ago

also, that link doesn't describe the resource leakage pattern, did you mean to paste a different link?

SteveWyshy commented 10 years ago

I've added the issue to our backlog to make sure we don't loose track.

https://trello.com/c/lcQIguPk/67-replace-addcallback-with-removecallback

Thanks for the suggestion.

00conan00 commented 10 years ago

Thanks for your prompt reply.

If you are interested in the resource leakage pattern, you can find more detailed discussion in "Characterizing and Detecting Resource Leaks in Android Applications" which can be found in the following link: http://ieeexplore.ieee.org/stamp/stamp.jsp?tp=&arnumber=6693097

In short, they summarize 65 resource operations which involve resources that need to be released manually, and "addCallback" "removeCallback" is one pair of them.

Hope this help.