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

Possible memory leak in EpoxyController when validateEpoxyModelUsage is set to true #597

Closed
SeptimusX75 opened this issue Oct 29, 2018 · 10 comments

Comments

@SeptimusX75
Copy link

This may very well be user error but I can't seem to track down what I could be doing wrong. I'm rendering a screen using an MVI style architecture so whenever the user changes something in the RecyclerView a new state is emitted which results in call to SimpleEpoxyController#setModels. This can be called multiple times in a second potentially. In my case I'm calling cancelPendingModelBuild as the quick and dirty way to debounce rendering until input is finished (not sure if this is related). Upon inspecting the heap dump it appears that modelInterceptorCallbacks is not releasing references to my models and as I continue to add more models I eventually run out of memory. I've set validateEpoxyModelUsage to false and it seems to remedy the problem but it feels like the wrong solution.

Any suggestions or guidance? Let me know If I can provide any additional information. Thanks.

@elihart
Copy link
Contributor

elihart commented Oct 29, 2018

thanks for the info, let's see if we can figure out what's going on.

modelInterceptorCallbacks is set to null after each model build, can you tell what is referencing it that is causing it to be kept around? I could explicitly clear it, but afaik clearing the reference to the list should be enough

fyi cancelPendingModelBuild in your case probably is doing nothing. Epoxy already debounced calls to build models, and in a much more efficient way, so you're just adding overhead.

that will still allow models to be built several times per second, it just debounces them per frame. if you want a longer debounce period you can use requestDelayedModelBuild

@SeptimusX75
Copy link
Author

I really appreciate the prompt reply.

Here is a screenshot of the heapdump. The controller appears to be the only thing holding the reference. Should models that are replaced in the controller be removed from modelInterceptorCallbacks#elementData after they've been diffed?

image

@elihart
Copy link
Contributor

elihart commented Oct 30, 2018

Are you forcing a garbage collection before you take the heap dump? The reference in the controller is cleared.

@elihart
Copy link
Contributor

elihart commented Oct 30, 2018

I've profiled our app and modelInterceptorCallbacks is set to null, the only time it should be nonnull is while models are actively being built

it looks like only a little over a hundred EpoxyModels are kept, that should be fine, the bigger question is where is the 215 MB of retained size coming from? how come each of your models is retaining 1.92 MB? that seems crazy high, it should be more like 1 KB

what are you storing in your models that is taking up so much space? that could be the issue

@SeptimusX75
Copy link
Author

As the first point, I did attempt garbage collection and it had no effect. I can't get those references to be released for some reason.

As for the second point, you're correct that the number is inordinately high. The reason each model is so large is because each one has a thumbnail bitmap it's holding a reference to. That is a problem in and of itself and needs to be addressed but from where I'm standing it appears that this fact has exposed the behavior that I'm observing.

@elihart
Copy link
Contributor

elihart commented Oct 31, 2018

How many models do you have on screen? The adapter has to hold a reference to all currently built models. I don't think anything is actually being leaked, unless you can show a reference chain - modelInterceptorCallbacks is cleared and doesn't hold on to anything. Your models are just too big

@SeptimusX75
Copy link
Author

SeptimusX75 commented Oct 31, 2018

We only have 9 models on screen at any given time. It makes me wonder why the size of modelInterceptorCallbacks is so large... 🤔

Also, I'm not quite sure I know how to correctly determine a reference chain. I don't profile memory too often so I'm a bit of a noob, I apologize.

@elihart
Copy link
Contributor

elihart commented Nov 1, 2018

Did you take that heap dump while models were being built? that's the only reason I can think that it would be non null.

Have you set breakpoints in the modelInterceptorCallbacks usage to see why there are so many models in it? there definitely seem to be more than 9.

I think the modelInterceptorCallbacks may be a red herring since it is nulled out after model build, I think the issue may be that you are building models are frequently and that your models contain so much data.

Are you sharing a bitmap between models, or does each new model create another bitmap? if so, that would be your problem.

have you trying the delayed model build approach I suggested earlier?

@romankivalin
Copy link
Contributor

modelInterceptorCallbacks is not nulled out if interceptors array is empty:

Since addAfterInterceptorCallback doesn't check interceptors and always add callback to modelInterceptorCallbacks, it grows up with each update and keep references to all models if we didn't add any interceptor.

@elihart
Copy link
Contributor

elihart commented Nov 20, 2018

@romankivalin Totally right, thanks a lot for that, I feel pretty silly for missing it.

@SeptimusX75 Hope this helps you, sorry for the issue and not catching it sooner. If you want a workaround before the next release you can simply add a no-op interceptor, which will trigger the right behavior.

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