JoanZapata / android-iconify

Android integration of multiple icon providers such as FontAwesome, Entypo, Typicons,...
http://joanzapata.com/android-iconify
Other
3.93k stars 525 forks source link

Possible memory inefficiency #63

Closed sam-ghosh closed 9 years ago

sam-ghosh commented 9 years ago

I am wondering if assigning a new Drawable resource on each OnCreateOptionsMenu() in activity would lead to a new instance of the same Drawable being created (and the previous one floating around somewhere and causing memory leak). Is it better to create a Drawable myDrawable = new IconDrawable() in the activity constructor and re-use it?

    menu.findItem(R.id.action_notification).setIcon(
            new IconDrawable(this, Iconify.IconValue.fa_bell)
                    .colorRes(R.color.em_active_green)
                    .actionBarSize());
mikepenz commented 9 years ago

The systems gc should be clever enough to remove the old drawable, but yeah you are right the above sample would create a new drawable without any caching or keeping the single instance in memory.

Someone would have to do a sample to test and look how bad (or if at all) this is. On a different side the activity constructor is also called -> onScreenOrientationChange so perhaps this isn't clever too.

JoanZapata commented 9 years ago

WIth the standard Android way of putting icons in ActionBar (though resource references in menu.xml), the Activity would load a BitmapDrawable for each item each time the Activity is created, which is much slower than creating an IconDrawable. Moreover, IconDrawable doesn't rely on any bitmap, it's based on a Typeface which is cached.