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

Clearing resources onStop #942

Closed
TepesLucian opened this issue Jan 29, 2016 · 7 comments · Fixed by #5145
Closed

Clearing resources onStop #942

TepesLucian opened this issue Jan 29, 2016 · 7 comments · Fixed by #5145
Labels

Comments

@TepesLucian
Copy link

This issue is based on #526 .
In case of an application that has a deep stack of activities (and some other use cases like device having a low heap memory) displaying images using Glide, I think it would be a great if the app could indicate Glide to clear/recycle resources on onStop() and reload the requests on onStart(). Since the activity/fragment is no longer visible to the user the memory can be reused by future activities.

I've managed to implement by manually loading the images onStart and clearing them onStop using Glide.clear(view) to avoid OOM in the case user opened a deep stack of activities. I think is fairly acceptable if the user opens multiple screens and navigating back to see at some point placeholders instead previous loaded images.

@TWiStErRob
Copy link
Collaborator

@sjudd do you think this is feasible, especially the reloading on return part?

Some thoughts:

  • How would this affect rotation? Is the RMFragment stopped on rotation?
  • What kind of API could convey this best? I guess Glide.with(...).autoManage() or something.
  • Probably need a way to reset normal behavior
  • GlideModule also need to be able to set default to autoManage in case the whole app would use it.

@sjudd
Copy link
Collaborator

sjudd commented Feb 12, 2016

I like this idea, though you'd want to be a bit careful with your cache size and Activity flow. I've worked on an app where they always cleared resources in onStop and re-loaded them in onStart, and we regularly had issues where a user would open an Activity then immediately hit the back button and see a bunch of grey squares where they had just seen images.

  • Rotation is ok, Glide will clear/restart when the Activity is destroyed and re-created anyway.
  • API would be a method on the RequestManager, I wouldn't be opposed to adding something in the GlideModule as well to set a default.

It's less neat than autoManage, but a bit more descriptive, maybe: Glide.with(...).setClearOnStop(boolean)

@TepesLucian
Copy link
Author

@sjudd I never seen this problem so far on my testing (maybe has to do with using a Nexus 5 or emulators). I'm curious what version you were using or doesn't have anything to do with it.

Wanted to point out that I recently refactored to a fragment approach because the client wanted stack like navigation with common ui so the app replaces fragments in container. Even though fragments aren't always a great thing, in this case the memory consumption should be lower due to fragment views being destroyed when replacing them (onDestroyView() being called). In this particular case, the resources loaded by the fragment must be cleared onDestroyView() or else at some point the memory will be filled if user opens an enormous stack (I know Glide clears resources onDestroy()).

@sjudd
Copy link
Collaborator

sjudd commented Mar 1, 2016

No objections to implementing this, a pull request would definitely be welcome if anyone wants to try this out.

@osamaaftab
Copy link
Contributor

@sjudd Can we please closed this issue as this has already been merged to master branch?

@TWiStErRob
Copy link
Collaborator

Thanks for the implementation @osamaaftab!

I wouldn't be opposed to adding something in the GlideModule as well to set a default.

This part is still up for grabs if someone wants to contribute.

@TWiStErRob
Copy link
Collaborator

(Tip: @osamaaftab if you put these two words "fixes #942" when you open the pull request it'll automatically close the issue on merge.)

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

Successfully merging a pull request may close this issue.

4 participants