Skip to content

Commit

Permalink
Add retained size to leak reports
Browse files Browse the repository at this point in the history 
Fixes square#162
  • Loading branch information
@pyricau
pyricau committed Jan 7, 2016
1 parent 5612e1e commit 7289587
Show file tree
Hide file tree
Showing 6 changed files with 175 additions and 43 deletions.
18 changes: 13 additions & 5 deletions src/main/java/com/squareup/leakcanary/AnalysisResult.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,16 +20,17 @@
public final class AnalysisResult implements Serializable {

public static AnalysisResult noLeak(long analysisDurationMs) {
return new AnalysisResult(false, false, null, null, null, analysisDurationMs);
return new AnalysisResult(false, false, null, null, null, 0, analysisDurationMs);
}

public static AnalysisResult leakDetected(boolean excludedLeak, String className,
LeakTrace leakTrace, long analysisDurationMs) {
return new AnalysisResult(true, excludedLeak, className, leakTrace, null, analysisDurationMs);
LeakTrace leakTrace, long retainedHeapSize, long analysisDurationMs) {
return new AnalysisResult(true, excludedLeak, className, leakTrace, null, retainedHeapSize,
analysisDurationMs);
}

public static AnalysisResult failure(Throwable failure, long analysisDurationMs) {
return new AnalysisResult(false, false, null, null, failure, analysisDurationMs);
return new AnalysisResult(false, false, null, null, failure, 0, analysisDurationMs);
}

/** True if a leak was found in the heap dump. */
Expand All @@ -56,16 +57,23 @@ public static AnalysisResult failure(Throwable failure, long analysisDurationMs)
/** Null unless the analysis failed. */
public final Throwable failure;

/**
* The number of bytes which would be freed if all references to the leaking object were
* released. 0 if {@link #leakFound} is false.
*/
public final long retainedHeapSize;

/** Total time spent analyzing the heap. */
public final long analysisDurationMs;

private AnalysisResult(boolean leakFound, boolean excludedLeak, String className,
LeakTrace leakTrace, Throwable failure, long analysisDurationMs) {
LeakTrace leakTrace, Throwable failure, long retainedHeapSize, long analysisDurationMs) {
this.leakFound = leakFound;
this.excludedLeak = excludedLeak;
this.className = className;
this.leakTrace = leakTrace;
this.failure = failure;
this.retainedHeapSize = retainedHeapSize;
this.analysisDurationMs = analysisDurationMs;
}
}
66 changes: 65 additions & 1 deletion src/main/java/com/squareup/leakcanary/HeapAnalyzer.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
import com.squareup.haha.perflib.HprofParser;
import com.squareup.haha.perflib.Instance;
import com.squareup.haha.perflib.RootObj;
import com.squareup.haha.perflib.RootType;
import com.squareup.haha.perflib.Snapshot;
import com.squareup.haha.perflib.Type;
import com.squareup.haha.perflib.io.HprofBuffer;
Expand Down Expand Up @@ -119,10 +120,73 @@ private AnalysisResult findLeakTrace(long analysisStartNanoTime, Snapshot snapsh

String className = leakingRef.getClassObj().getClassName();

return leakDetected(result.excludingKnownLeaks, className, leakTrace,
// Side effect: computes retained size.
snapshot.computeDominators();

Instance leakingInstance = result.leakingNode.instance;

long retainedSize = leakingInstance.getTotalRetainedSize();

retainedSize += computeIgnoredBitmapRetainedSize(snapshot, leakingInstance);

return leakDetected(result.excludingKnownLeaks, className, leakTrace, retainedSize,
since(analysisStartNanoTime));
}

/**
* Bitmaps and bitmap byte arrays are sometimes held by native gc roots, so they aren't included
* in the retained size because their root dominator is a native gc root.
* To fix this, we check if the leaking instance is a dominator for each bitmap instance and then
* add the bitmap size.
*
* From experience, we've found that bitmap created in code (Bitmap.createBitmap()) are correctly
* accounted for, however bitmaps set in layouts are not.
*/
private int computeIgnoredBitmapRetainedSize(Snapshot snapshot, Instance leakingInstance) {
int bitmapRetainedSize = 0;
ClassObj bitmapClass = snapshot.findClass("android.graphics.Bitmap");

for (Instance bitmapInstance : bitmapClass.getInstancesList()) {
if (isIgnoredDominator(leakingInstance, bitmapInstance)) {
ArrayInstance mBufferInstance = fieldValue(classInstanceValues(bitmapInstance), "mBuffer");
// Native bitmaps have mBuffer set to null. We sadly can't account for them.
if (mBufferInstance == null) {
continue;
}
long bufferSize = mBufferInstance.getTotalRetainedSize();
long bitmapSize = bitmapInstance.getTotalRetainedSize();
// Sometimes the size of the buffer isn't accounted for in the bitmap retained size. Since
// the buffer is large, it's easy to detect by checking for bitmap size < buffer size.
if (bitmapSize < bufferSize) {
bitmapSize += bufferSize;
}
bitmapRetainedSize += bitmapSize;
}
}
return bitmapRetainedSize;
}

private boolean isIgnoredDominator(Instance dominator, Instance instance) {
boolean foundNativeRoot = false;
while (true) {
Instance immediateDominator = instance.getImmediateDominator();
if (immediateDominator instanceof RootObj
&& ((RootObj) immediateDominator).getRootType() == RootType.UNKNOWN) {
// Ignore native roots
instance = instance.getNextInstanceToGcRoot();
foundNativeRoot = true;
} else {
instance = immediateDominator;
}
if (instance == null) {
return false;
}
if (instance == dominator) {
return foundNativeRoot;
}
}
}

private LeakTrace buildLeakTrace(LeakNode leakingNode) {
List<LeakTraceElement> elements = new ArrayList<>();
// We iterate from the leak to the GC root
Expand Down
27 changes: 12 additions & 15 deletions src/test/java/com/squareup/leakcanary/AsyncTaskLeakTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
*/
package com.squareup.leakcanary;

import java.io.File;
import java.lang.ref.WeakReference;
import java.util.Arrays;
import java.util.Collection;
Expand All @@ -24,10 +23,12 @@
import org.junit.runner.RunWith;
import org.junit.runners.Parameterized;

import static com.squareup.leakcanary.TestUtil.fileFromName;
import static com.squareup.leakcanary.TestUtil.analyze;
import static com.squareup.leakcanary.LeakTraceElement.Holder.THREAD;
import static com.squareup.leakcanary.LeakTraceElement.Type.STATIC_FIELD;
import static com.squareup.leakcanary.TestUtil.HeapDumpFile.ASYNC_TASK;
import static com.squareup.leakcanary.TestUtil.HeapDumpFile.ASYNC_TASK_MPREVIEW2;
import static com.squareup.leakcanary.TestUtil.HeapDumpFile.ASYNC_TASK_M_POSTPREVIEW2;
import static com.squareup.leakcanary.TestUtil.analyze;
import static org.hamcrest.core.StringContains.containsString;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
Expand All @@ -44,20 +45,17 @@ public class AsyncTaskLeakTest {

@Parameterized.Parameters public static Collection<Object[]> data() {
return Arrays.asList(new Object[][] {
{ fileFromName("leak_asynctask.hprof"), "dc983a12-d029-4003-8890-7dd644c664c5" },
{ fileFromName("leak_asynctask_mpreview2.hprof"), "1114018e-e154-435f-9a3d-da63ae9b47fa" },
{ fileFromName("leak_asynctask_m_postpreview2.hprof"), "25ae1778-7c1d-4ec7-ac50-5cce55424069" }
{ ASYNC_TASK }, //
{ ASYNC_TASK_MPREVIEW2 }, //
{ ASYNC_TASK_M_POSTPREVIEW2 } //
});
}

final File heapDumpFile;
final String referenceKey;

private final TestUtil.HeapDumpFile heapDumpFile;
ExcludedRefs.Builder excludedRefs;

public AsyncTaskLeakTest(File heapDumpFile, String referenceKey) {
public AsyncTaskLeakTest(TestUtil.HeapDumpFile heapDumpFile) {
this.heapDumpFile = heapDumpFile;
this.referenceKey = referenceKey;
}

@Before public void setUp() {
Expand All @@ -66,7 +64,7 @@ public AsyncTaskLeakTest(File heapDumpFile, String referenceKey) {
}

@Test public void leakFound() {
AnalysisResult result = analyze(heapDumpFile, referenceKey, excludedRefs);
AnalysisResult result = analyze(heapDumpFile, excludedRefs);
assertTrue(result.leakFound);
assertFalse(result.excludedLeak);
LeakTraceElement gcRoot = result.leakTrace.elements.get(0);
Expand All @@ -77,7 +75,7 @@ public AsyncTaskLeakTest(File heapDumpFile, String referenceKey) {

@Test public void excludeThread() {
excludedRefs.thread(ASYNC_TASK_THREAD);
AnalysisResult result = analyze(heapDumpFile, referenceKey, excludedRefs);
AnalysisResult result = analyze(heapDumpFile, excludedRefs);
assertTrue(result.leakFound);
assertFalse(result.excludedLeak);
LeakTraceElement gcRoot = result.leakTrace.elements.get(0);
Expand All @@ -91,9 +89,8 @@ public AsyncTaskLeakTest(File heapDumpFile, String referenceKey) {
excludedRefs.thread(ASYNC_TASK_THREAD);
excludedRefs.staticField(ASYNC_TASK_CLASS, EXECUTOR_FIELD_1);
excludedRefs.staticField(ASYNC_TASK_CLASS, EXECUTOR_FIELD_2);
AnalysisResult result = analyze(heapDumpFile, referenceKey, excludedRefs);
AnalysisResult result = analyze(heapDumpFile, excludedRefs);
assertTrue(result.leakFound);
assertTrue(result.excludedLeak);
}

}
53 changes: 53 additions & 0 deletions src/test/java/com/squareup/leakcanary/RetainedSizeTest.java
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
package com.squareup.leakcanary;

import java.lang.ref.WeakReference;
import java.util.Arrays;
import java.util.Collection;
import org.junit.Before;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.junit.runners.Parameterized;

import static com.squareup.leakcanary.TestUtil.HeapDumpFile.ASYNC_TASK;
import static com.squareup.leakcanary.TestUtil.HeapDumpFile.ASYNC_TASK_MPREVIEW2;
import static com.squareup.leakcanary.TestUtil.HeapDumpFile.ASYNC_TASK_M_POSTPREVIEW2;
import static com.squareup.leakcanary.TestUtil.HeapDumpFile.SERVICE_BINDER;
import static com.squareup.leakcanary.TestUtil.HeapDumpFile.SERVICE_BINDER_IGNORED;
import static com.squareup.leakcanary.TestUtil.analyze;
import static org.junit.Assert.assertEquals;

/**
* This test makes sure there is no regression on the retained size calculation.
*/
@RunWith(Parameterized.class) //
public class RetainedSizeTest {

@Parameterized.Parameters public static Collection<Object[]> data() {
return Arrays.asList(new Object[][] {
{ ASYNC_TASK, 207_407 }, //
{ ASYNC_TASK_MPREVIEW2, 1_604 }, //
{ ASYNC_TASK_M_POSTPREVIEW2, 1_870 }, //
{ SERVICE_BINDER, 378 }, //
{ SERVICE_BINDER_IGNORED, 378 }, //
});
}

private final TestUtil.HeapDumpFile heapDumpFile;
private final long expectedRetainedHeapSize;
ExcludedRefs.Builder excludedRefs;

public RetainedSizeTest(TestUtil.HeapDumpFile heapDumpFile, long expectedRetainedHeapSize) {
this.heapDumpFile = heapDumpFile;
this.expectedRetainedHeapSize = expectedRetainedHeapSize;
}

@Before public void setUp() {
excludedRefs = new ExcludedRefs.Builder().clazz(WeakReference.class.getName(), true)
.clazz("java.lang.ref.FinalizerReference", true);
}

@Test public void leakFound() {
AnalysisResult result = analyze(heapDumpFile, excludedRefs);
assertEquals(expectedRetainedHeapSize, result.retainedHeapSize);
}
}
27 changes: 7 additions & 20 deletions src/test/java/com/squareup/leakcanary/ServiceBinderLeakTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,15 +15,16 @@
*/
package com.squareup.leakcanary;

import java.lang.ref.WeakReference;
import org.junit.Before;
import org.junit.Test;

import java.lang.ref.WeakReference;

import static com.squareup.leakcanary.LeakTraceElement.Holder.CLASS;
import static com.squareup.leakcanary.LeakTraceElement.Holder.OBJECT;
import static com.squareup.leakcanary.LeakTraceElement.Type.INSTANCE_FIELD;
import static com.squareup.leakcanary.LeakTraceElement.Type.STATIC_FIELD;
import static com.squareup.leakcanary.TestUtil.HeapDumpFile.SERVICE_BINDER;
import static com.squareup.leakcanary.TestUtil.HeapDumpFile.SERVICE_BINDER_IGNORED;
import static com.squareup.leakcanary.TestUtil.analyze;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
Expand All @@ -38,19 +39,14 @@ public class ServiceBinderLeakTest {
ExcludedRefs.Builder excludedRefs;

@Before public void setUp() {
excludedRefs = new ExcludedRefs.Builder()
.clazz(WeakReference.class.getName(), true)
excludedRefs = new ExcludedRefs.Builder().clazz(WeakReference.class.getName(), true)
.clazz("java.lang.ref.FinalizerReference", true);
}

@Test public void realBinderLeak() {
excludedRefs.rootSuperClass("android.os.Binder", true);

AnalysisResult result = analyze(
TestUtil.fileFromName("leak_service_binder.hprof"),
"b3abfae6-2c53-42e1-b8c1-96b0558dbeae",
excludedRefs
);
AnalysisResult result = analyze(SERVICE_BINDER, excludedRefs);

assertTrue(result.leakFound);
assertFalse(result.excludedLeak);
Expand All @@ -63,11 +59,7 @@ public class ServiceBinderLeakTest {
@Test public void ignorableBinderLeak() {
excludedRefs.rootSuperClass("android.os.Binder", false);

AnalysisResult result = analyze(
TestUtil.fileFromName("leak_service_binder_ignored.hprof"),
"6e524414-9581-4ce7-8690-e8ddf8b82454",
excludedRefs
);
AnalysisResult result = analyze(SERVICE_BINDER_IGNORED, excludedRefs);

assertTrue(result.leakFound);
assertTrue(result.excludedLeak);
Expand All @@ -80,13 +72,8 @@ public class ServiceBinderLeakTest {
@Test public void alwaysIgnorableBinderLeak() {
excludedRefs.rootSuperClass("android.os.Binder", true);

AnalysisResult result = analyze(
TestUtil.fileFromName("leak_service_binder_ignored.hprof"),
"6e524414-9581-4ce7-8690-e8ddf8b82454",
excludedRefs
);
AnalysisResult result = analyze(SERVICE_BINDER_IGNORED, excludedRefs);

assertFalse(result.leakFound);
}

}
27 changes: 25 additions & 2 deletions src/test/java/com/squareup/leakcanary/TestUtil.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,15 +19,38 @@
import java.net.URL;

final class TestUtil {

enum HeapDumpFile {
ASYNC_TASK("leak_asynctask.hprof", "dc983a12-d029-4003-8890-7dd644c664c5"),
ASYNC_TASK_MPREVIEW2("leak_asynctask_mpreview2.hprof", "1114018e-e154-435f-9a3d-da63ae9b47fa"),
ASYNC_TASK_M_POSTPREVIEW2("leak_asynctask_m_postpreview2.hprof",
"25ae1778-7c1d-4ec7-ac50-5cce55424069"),

SERVICE_BINDER("leak_service_binder.hprof", "b3abfae6-2c53-42e1-b8c1-96b0558dbeae"),
SERVICE_BINDER_IGNORED("leak_service_binder_ignored.hprof",
"6e524414-9581-4ce7-8690-e8ddf8b82454"),;

private final String filename;
private final String referenceKey;

HeapDumpFile(String filename, String referenceKey) {
this.filename = filename;
this.referenceKey = referenceKey;
}

}

static File fileFromName(String filename) {
ClassLoader classLoader = Thread.currentThread().getContextClassLoader();
URL url = classLoader.getResource(filename);
return new File(url.getPath());
}

static AnalysisResult analyze(File heapDumpFile, String referenceKey, ExcludedRefs.Builder excludedRefs) {
static AnalysisResult analyze(HeapDumpFile heapDumpFile, ExcludedRefs.Builder excludedRefs) {
File file = fileFromName(heapDumpFile.filename);
String referenceKey = heapDumpFile.referenceKey;
HeapAnalyzer heapAnalyzer = new HeapAnalyzer(excludedRefs.build());
AnalysisResult result = heapAnalyzer.checkForLeak(heapDumpFile, referenceKey);
AnalysisResult result = heapAnalyzer.checkForLeak(file, referenceKey);
if (result.failure != null) {
result.failure.printStackTrace();
}
Expand Down

0 comments on commit 7289587

Please sign in to comment.