Leakcanary: Improve text rendering of leaks

[pyricau]

After testing this it's different people, it's clear that we could improve text LeakTraces.

Note: the comments reflects different things we've tried, but this issue description has been updated to reflect the latest.

Current state

In com.example.leakcanary:1.0:1.
* com.example.leakcanary.MainActivity has leaked:
* Toast$TN.mNextView
* ↳ LinearLayout.mContext
* ↳ MainActivity.!(httpRequestHelper)!
* ↳ HttpRequestHelper.!(button)!
* ↳ Button.mContext
* ↳ MainActivity

Goals

• Enable Android developers to fix leaks quickly by pointing them to the potential leak causes.
• Help developers understand how Leakcanary is identifying the potential leak causes.

Insights

• Developers tend to focus on instances in the leak chain, when they should instead focus on the references between instances. The cause of a leak is an incorrect reference.
• Fully qualified class names add noise that impacts legibility
• The first thing developers care about is which instead is leaking. Then which instance is causing that instance to leak. Etc. So we should put the gc roots at the bottom. While trees are traditionally represented with root at the top, this is a special case (linear, no sibling). Worth noting Java stacktraces are also represented with the root of the call stack at the bottom.
• It should be clear that the leak trace is a directed chain.
• The leak trace should be as self served as possible:
  
  
  
  • Use traditional graphical representations instead of words to improve legibility, if some representations can be interpreted without subtext.
  
  
  • Use minimum words necessary to provide context
  
  
• To help understand how LeakCanary is identifying the potential leak causes, we need to surface which instances it believes are leaking and which it knows are not.
  
  
  
  • We need to do this in a way that feels like metadata rather than the most important information. Developers tend to jump to "this instance is leaking, therefore the problem is there". The problem isn't there, the problem is in between the group of non leaking instances and the group of leaking instances.
  
  
• We looked at many ways to represent the known state of instances (when it is known).
  
  
  
  • The most correct would be: "This instance is reachable as we expected" and "This instance is reachable but should be unreachable"
  
  
  • We tried ✓, †, ✗ . However, the meaning isn't clear. Also, the ✗ tends to draw more attention (when in fact ✗ and ✓ are equally important, and they're just metadata)
  
  
  • "reachable" / "unreachable" is a little bit too technical. Especially since it's about expectations (they're actually all reachable and that's the issue)
  
  
  • "alive" isn't much better, not very clear what that would mean.
  
  
• When two instances of the same class exist in a chain, we need to disambiguate
• Some descriptions of "why we know this is leaking / non leaking" are actually just references to other elements in the chain. So we need to be able to point to other elements.

Latest proposed solution

MainActivity leaked due to one of the fields highlighted with ~~~
┬
├─ android.widget.Toast$TN
│  Leaking: NO (LinearLayout↓ is not leaking)
│  Leaking: NO (it's a GC root)
│  ↓ Toast$TN.mNextView 
│
├─ android.widget.LinearLayout
│  Leaking: NO (View#mAttachInfo is not null)
│  Leaking: NO (MainActivity↓ is leaking)
│  View is attached (View#mAttachInfo is not null)
│  View id is R.id.container_layout
│  ↓ LinearLayout.mContext
│
├─ com.example.MainActivity
│  Leaking: NO (Activity#mDestroyed is false)
│  ↓ MainActivity.httpRequestHelper
│                 ~~~~~~~~~~~~~~~~~
│
│
├─ com.example.HttpRequestHelper
│  Leaking: UNKNOWN
│  ↓ HttpRequestHelper.button
│                      ~~~~~~
│
├─ android.widget.Button
│  Leaking: YES (View#mAttachInfo is null)
│  View id is R.id.charge_button
│  ↓ Button.mContext
│
╰→ com.example.MainActivity
   Leaking: YES (Activity#mDestroyed is true)
   Leaking: YES (RefWatcher was watching this)

[pyricau]

In com.example.leakcanary:1.0:1.
MainActivity@718013552 is leaking, see ╭?─╯ for the potential causes:
┬
╰→ Toast$TN (Alive: it's a GC root)
   Toast$TN.mNextView ─╮
╭──────────────────────╯
╰→ LinearLayout@1935364696 (Alive: View#mAttachInfo is not null)
   LinearLayout.mContext─╮
╭────────────────────────╯
╰→ MainActivity@718262384 (Alive: Activity#mDestroyed is not true)
   MainActivity.httpRequestHelper ─╮
╭?─────────────────────────────────╯
╰→ HttpRequestHelper@1867963480
   HttpRequestHelper.button ─╮
╭?───────────────────────────╯
╰→ Button@718299264 (Not alive: View#mAttachInfo is null)
   Button.mContext ─╮
╭───────────────────╯
╰→ MainActivity@718013552 (Not alive: Activity#mDestroyed is true)

[pyricau]

Some more iteration: backward and inlining:

In com.example.leakcanary:1.0:1.
MainActivity@718013552 is leaking, see ╰?─╮ for the potential causes:
╭→ MainActivity@718013552 not alive: Activity#mDestroyed is true
╰───────────────────╮
╭→ Button.mContext ─╯ Button@718299264 not alive: View#mAttachInfo is null
╰?───────────────────────────╮
╭→ HttpRequestHelper.button ─╯ HttpRequestHelper@1867963480
╰?─────────────────────────────────╮
╭→ MainActivity.httpRequestHelper ─╯ MainActivity@718262384 alive: Activity#mDestroyed is not true
╰────────────────────────╮
╭→ LinearLayout.mContext─╯ LinearLayout@1935364696 alive: View#mAttachInfo is not null
╰─────────────────────╮
╭→Toast$TN.mNextView ─╯ Toast$TN alive: it's a GC root
┴

[pyricau]

In com.example.leakcanary:1.0:1.
MainActivity@718013552 is leaking, see ╰?─╯ for the potential causes:
                                  ╭────────────────────────────────────────────────────────────────╮
╭→ MainActivity                   │ MainActivity@718013552, not alive: Activity#mDestroyed is true │
│                                 │                                                                │
├─ Button.mContext                │ Button@718013552, not alive: View#mAttachInfo is null          │
┊                                 │                                                                │
├─ HttpRequestHelper.button       │ HttpRequestHelper@1867963480                                   │
┊                    ╰?───╯       │                                                                │
├─ MainActivity.httpRequestHelper │ MainActivity@718262384, alive: Activity#mDestroyed is not true │
│               ╰?──────────────╯ │                                                                │
├─ LinearLayout.mContext          │ LinearLayout@1935364696, alive: View#mAttachInfo is not null   │
│                                 │                                                                │
├─ Toast$TN.mNextView             │ Toast$TN, alive: LinearLayout@1935364696 is alive              │
┴                                 ╰────────────────────────────────────────────────────────────────╯

[pyricau]

In com.example.leakcanary:1.0:1.
@6 MainActivity is leaking, see ╭?─╮ for the potential causes:

╭→ @6 MainActivity                              
│                                 
├─ @5 Button.mContext                
┊                       ╭?───╮    
├─ @4 HttpRequestHelper.button                                              
┊                  ╭?──────────────╮                                       
├─ @3 MainActivity.httpRequestHelper
│
├─ @2 LinearLayout.mContext    
│                                           
├─ @1 Toast$TN.mNextView          
┴                                 

╭→ @6 MainActivity leaking: Activity#mDestroyed is true
├─ @5 Button leaking: View#mAttachInfo is null
├─ @4 HttpRequestHelper
├─ @3 MainActivity alive: Activity#mDestroyed is not true
├─ @2 LinearLayout alive: View#mAttachInfo is not null
├─ @1 Toast$TN alive: @2 LinearLayout is alive
┴  

[pyricau]

In com.example.leakcanary:1.0:1.
MainActivity¹ is leaking, see ╭?─╮ for the potential causes:
╭→ MainActivity¹
│    MainActivity leaking: Activity#mDestroyed is true
├─ Button.mContext
┊    Button leaking: View#mAttachInfo is null
┊                    ╭?───╮
├─ HttpRequestHelper.button
┊               ╭?──────────────╮
├─ MainActivity².httpRequestHelper
│    MainActivity not leaking: Activity#mDestroyed is not true
├─ LinearLayout¹.mContext
│    LinearLayout not leaking: View#mAttachInfo is not null
├─ Toast$TN.mNextView
│    Toast$TN not leaking: LinearLayout¹ is not leaking
┴

[pyricau]

In com.example.leakcanary:1.0:1.
MainActivity¹ is leaking, see ? for the potential causes:
╭→ MainActivity¹
│    MainActivity leaking: Activity#mDestroyed is true
├─ Button.mContext
┊    Button leaking: View#mAttachInfo is null
┊?                   ╭────╮
├─ HttpRequestHelper.button
┊?               ╭───────────────╮
├─ MainActivity².httpRequestHelper
│    MainActivity not leaking: Activity#mDestroyed is not true
├─ LinearLayout¹.mContext
│    LinearLayout not leaking: View#mAttachInfo is not null
├─ Toast$TN.mNextView
│    Toast$TN not leaking: LinearLayout¹ is not leaking
┴

[Sayary]

@pyricau thanks for the change! This is super helpful for troubleshooting memory leaks, do you have a plan for a new release with this feature?

[pyricau]

Yes. The plan is that I'll work on this when I have time, and I'll make a release when I'm done working on it :) .

[pyricau]

This shipped in 2.0 alpha 1