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

Load leaks in DisplayActivity as they're loaded #1182

Closed
jrodbx opened this issue Feb 12, 2019 · 3 comments
Closed

Load leaks in DisplayActivity as they're loaded #1182

jrodbx opened this issue Feb 12, 2019 · 3 comments

Comments

@jrodbx
Copy link
Collaborator

jrodbx commented Feb 12, 2019

Thanks to #1081, I get a bunch of false alarms which I ignore. However, when I face a leak that I now care about and go to the DisplayActivity, I have to wait for all of them to load (I currently have 91), before appearing in the Activity list. An impatient user may think they have no leaks to review and exit the Activity prematurely.

A quick workaround might be to post a notification, but I'd prefer for each leak to appear in the list as soon as it's been loaded in a least-recent to most-recent order.

@pyricau
Copy link
Member

pyricau commented Mar 6, 2019

Great idea!

  • Seems fairly doable to load leaks one at a time, since we have one leak per file
  • We can first list the total number of file (to get an accurate list size) and then replace template items with real items
  • We should probably reverse the order as well to have latest at the top? Seems like that's what you want to see most of the time.

@pyricau pyricau added this to the 2.0 milestone Mar 30, 2019
@pyricau
Copy link
Member

pyricau commented Mar 30, 2019

Note: this should happen after #1202 (which changes the UX by adding groups)

@pyricau
Copy link
Member

pyricau commented May 5, 2019

LeakCanary now stores the data as rows in a Sqlite db, which should be much faster. Also, we now display groups so there should be way less rows. The db is read from a background IO thread and the full list displayed. Changing this to a dynamic list that requeries as we scroll but in a background thread (picasso like) seems like much more work.

If this becomes an issue again, we could look into this, or optimizing the SQL query, or cleaning up old data.

@pyricau pyricau closed this as completed May 5, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants