zixpo / candybar

Dashboard for Android Icon Packs. Supported by the community.
Apache License 2.0
288 stars 55 forks source link

Using icon search can lead to OutOfMemoryError #113

Closed moertel closed 1 year ago

moertel commented 1 year ago

When searching icons in fast succession (i.e. deleting, adding, deleting and adding characters), I can watch the memory footprint grow in AndroidStudio. If done fast enough, the device goes out of memory. If I do it more slowly, I can see the garbage collector catching up and freeing memory.

In the wild, every week or two I see a user hitting the OutOfMemoryError while searching for icons. (I have analytics implemented in my app, so I can see the actions that lead up to a crash.)

Right now I'm unsure how to solve this. Is there a way to give the garbage collector a hint that a previous search result may be completely discarded if a new search is typed in?

Stacktrace example:

Fatal Exception: java.lang.OutOfMemoryError: Failed to allocate a 24 byte allocation with 2383632 free bytes and 2327KB until OOM, target footprint 268435456, growth limit 268435456; giving up on allocation because <1% of heap free after GC.
       at androidx.appcompat.widget.AppCompatTextHelper.updateTypefaceAndStyle(AppCompatTextHelper.java:365)
       at androidx.appcompat.widget.AppCompatTextHelper.loadFromAttributes(AppCompatTextHelper.java:150)
       at androidx.appcompat.widget.AppCompatTextView.<init>(AppCompatTextView.java:116)
       at com.google.android.material.textview.MaterialTextView.<init>(MaterialTextView.java:93)
       at com.google.android.material.textview.MaterialTextView.<init>(MaterialTextView.java:88)
       at com.google.android.material.textview.MaterialTextView.<init>(MaterialTextView.java:83)
       at com.google.android.material.theme.MaterialComponentsViewInflater.createTextView(MaterialComponentsViewInflater.java:61)
       at androidx.appcompat.app.AppCompatViewInflater.createView(AppCompatViewInflater.java:121)
       at androidx.appcompat.app.AppCompatDelegateImpl.createView(AppCompatDelegateImpl.java:1569)
       at androidx.appcompat.app.AppCompatDelegateImpl.onCreateView(AppCompatDelegateImpl.java:1620)
       at android.view.LayoutInflater.tryCreateView(LayoutInflater.java:1065)
       at android.view.LayoutInflater.createViewFromTag(LayoutInflater.java:1001)
       at android.view.LayoutInflater.createViewFromTag(LayoutInflater.java:965)
       at android.view.LayoutInflater.rInflate(LayoutInflater.java:1127)
       at android.view.LayoutInflater.rInflateChildren(LayoutInflater.java:1088)
       at android.view.LayoutInflater.rInflate(LayoutInflater.java:1130)
       at android.view.LayoutInflater.rInflateChildren(LayoutInflater.java:1088)
       at android.view.LayoutInflater.inflate(LayoutInflater.java:686)
       at android.view.LayoutInflater.inflate(LayoutInflater.java:538)
       at candybar.lib.adapters.IconsAdapter.onCreateViewHolder(IconsAdapter.java:185)
       at candybar.lib.adapters.IconsAdapter.onCreateViewHolder(IconsAdapter.java:64)
       at androidx.recyclerview.widget.RecyclerView$Adapter.createViewHolder(RecyclerView.java:7295)
       at androidx.recyclerview.widget.RecyclerView$Recycler.tryGetViewHolderForPositionByDeadline(RecyclerView.java:6416)
       at androidx.recyclerview.widget.GapWorker.prefetchPositionWithDeadline(GapWorker.java:288)
       at androidx.recyclerview.widget.GapWorker.flushTaskWithDeadline(GapWorker.java:345)
       at androidx.recyclerview.widget.GapWorker.flushTasksWithDeadline(GapWorker.java:361)
       at androidx.recyclerview.widget.GapWorker.prefetch(GapWorker.java:368)
       at androidx.recyclerview.widget.GapWorker.run(GapWorker.java:399)
       at android.os.Handler.handleCallback(Handler.java:938)
       at android.os.Handler.dispatchMessage(Handler.java:99)
       at android.os.Looper.loopOnce(Looper.java:346)
       at android.os.Looper.loop(Looper.java:475)
       at android.app.ActivityThread.main(ActivityThread.java:7889)
       at java.lang.reflect.Method.invoke(Method.java)
       at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:548)
       at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:1009)
sarsamurmu commented 1 year ago

Maybe assigning mIcons to null before assigning it to the array can solve it? - https://github.com/zixpo/candybar/blob/main/library/src/main/java/candybar/lib/adapters/IconsAdapter.java#L355

I am not sure if it will make a difference, this is the last thing I can think of

moertel commented 1 year ago

Keeping you posted: I tried a lot by now. Setting mIcons to null, rewriting everything so it would be reusing the same ArrayList (clearing and refilling, instead of assigning brand new lists), explicitly calling the Garbage Collector... None worked.

The interesting bit is that memory increases both when a search term is entered and when navigating back to the full list.

I'm puzzled by this. :)

sarsamurmu commented 1 year ago

It was not caused by the icons' array. It was caused by this List of ViewHolder.

https://github.com/zixpo/candybar/blob/55cabe98832304e677633c085b38c5b4189d951b/library/src/main/java/candybar/lib/adapters/IconsAdapter.java#L68 https://github.com/zixpo/candybar/blob/55cabe98832304e677633c085b38c5b4189d951b/library/src/main/java/candybar/lib/adapters/IconsAdapter.java#L192

Here, https://github.com/zixpo/candybar/blob/55cabe98832304e677633c085b38c5b4189d951b/library/src/main/java/candybar/lib/adapters/IconsAdapter.java#L207-L213 I thought that removing holders on onViewRecycled would remove all unused holders, but that was not the case. Those holders kept some reference to other bigger objects and caused memory leaks.

The new implementation doesn't hold any reference to objects that would cause memory leaks.

moertel commented 1 year ago

You're a wizard, @sarsamurmu! Thanks for tracking down and solving this issue.