Skip to content

Commit

Permalink
Adding labels API and removing extras (#1292)
Browse files Browse the repository at this point in the history
Fixes #1027
  • Loading branch information
pyricau authored Apr 22, 2019
1 parent 4a2b158 commit d23b26a
Showing 20 changed files with 243 additions and 99 deletions.
Original file line number Diff line number Diff line change
@@ -18,14 +18,18 @@ data class LeakTraceElement(
*/
val classHierarchy: List<String>,

/** Additional information, may be null. */
val extra: String?,

/** If not null, there was no path that could exclude this element. */
val exclusion: Exclusion?,

/** List of all fields (member and static) for that object. */
val fieldReferences: List<LeakReference>
@Deprecated("This field will be replaced with the parser itself")
val fieldReferences: List<LeakReference>,

/**
* Ordered labels that were computed during analysis. A label provides
* extra information that helps understand the leak trace element.
*/
val labels: List<String>
) : Serializable {

val className: String
Original file line number Diff line number Diff line change
@@ -20,7 +20,9 @@ fun LeakTrace.renderToString(): String {
val currentReachability = expectedReachability[index]
leakInfo += """
#├─ ${leakTraceElement.className}
#│ Leaking: ${currentReachability.renderToString()}
#│ Leaking: ${currentReachability.renderToString()}${if (leakTraceElement.labels.isNotEmpty()) leakTraceElement.labels.joinToString(
"\n", prefix = "\n"
) else ""}
#│ ↓ ${getNextElementString(this, leakTraceElement, index)}
#""".trimMargin("#")
}
@@ -54,7 +56,6 @@ private fun getNextElementString(
} else ""
val simpleClassName = element.getSimpleClassName()
val referenceName = if (element.reference != null) ".${element.reference.displayName}" else ""
val extraString = if (element.extra != null) " ${element.extra}" else ""
val exclusionString =
if (element.exclusion != null) " , matching exclusion ${element.exclusion.matching}" else ""
val requiredSpaces =
@@ -67,7 +68,7 @@ private fun getNextElementString(
""
}

return staticString + holderString + simpleClassName + referenceName + extraString + exclusionString + leakString
return staticString + holderString + simpleClassName + referenceName + exclusionString + leakString
}

private const val ZERO_WIDTH_SPACE = '\u200b'
Original file line number Diff line number Diff line change
@@ -585,9 +585,9 @@ class PerflibHeapAnalyzer @TestOnly internal constructor(
holderType = OBJECT
}
}
val labels = if (extra == null) emptyList<String>() else mutableListOf(extra)
return LeakTraceElement(
node.leakReference, holderType, classHierarchy, extra,
node.exclusion, leakReferences
node.leakReference, holderType, classHierarchy, node.exclusion, leakReferences, labels
)
}

Original file line number Diff line number Diff line change
@@ -2,11 +2,11 @@ package leakcanary.internal

import leakcanary.ExcludedRefs
import leakcanary.ExcludedRefs.BuilderWithParams
import leakcanary.LeakTraceElement.Holder.THREAD
import leakcanary.LeakTraceElement.Type.STATIC_FIELD
import leakcanary.internal.HeapDumpFile.ASYNC_TASK_M
import leakcanary.internal.HeapDumpFile.ASYNC_TASK_O
import leakcanary.internal.HeapDumpFile.ASYNC_TASK_PRE_M
import leakcanary.LeakTraceElement.Holder.THREAD
import leakcanary.LeakTraceElement.Type.STATIC_FIELD
import org.assertj.core.api.Assertions.assertThat
import org.junit.Before
import org.junit.Test
@@ -40,9 +40,6 @@ internal class AsyncTaskLeakTest(private val heapDumpFile: HeapDumpFile) {
val gcRoot = result.leakTrace!!.elements[0]
assertThat(Thread::class.java.name).isEqualTo(gcRoot.className)
assertThat(THREAD).isEqualTo(gcRoot.holder)
assertThat(gcRoot.extra).contains(
ASYNC_TASK_THREAD
)
}

@Test
90 changes: 28 additions & 62 deletions leakcanary-analyzer/src/main/java/leakcanary/HeapAnalyzer.kt
Original file line number Diff line number Diff line change
@@ -41,7 +41,7 @@ import leakcanary.HeapValue.LongValue
import leakcanary.HeapValue.ObjectReference
import leakcanary.HeapValue.ShortValue
import leakcanary.HprofParser.RecordCallbacks
import leakcanary.LeakTraceElement.Holder
import leakcanary.LeakNode.ChildNode
import leakcanary.LeakTraceElement.Holder.ARRAY
import leakcanary.LeakTraceElement.Holder.CLASS
import leakcanary.LeakTraceElement.Holder.OBJECT
@@ -60,8 +60,6 @@ import leakcanary.Record.HeapDumpRecord.ObjectRecord.ObjectArrayDumpRecord
import leakcanary.Record.LoadClassRecord
import leakcanary.Record.StringRecord
import leakcanary.internal.KeyedWeakReferenceMirror
import leakcanary.internal.LeakNode
import leakcanary.internal.LeakNode.ChildNode
import leakcanary.internal.ShortestPathFinder
import leakcanary.internal.ShortestPathFinder.Result
import java.util.ArrayList
@@ -79,7 +77,8 @@ class HeapAnalyzer constructor(
* and then computes the shortest strong reference path from that instance to the GC roots.
*/
fun checkForLeaks(
heapDump: HeapDump
heapDump: HeapDump,
labelers: List<Labeler>
): HeapAnalysis {
val analysisStartNanoTime = System.nanoTime()

@@ -121,7 +120,9 @@ class HeapAnalyzer constructor(

val pathResults = findShortestPaths(heapDump, parser, leakingWeakRefs, gcRootIds)

buildLeakTraces(heapDump, pathResults, parser, leakingWeakRefs, analysisResults)
buildLeakTraces(
heapDump, labelers, pathResults, parser, leakingWeakRefs, analysisResults
)

addRemainingInstancesWithNoPath(parser, leakingWeakRefs, analysisResults)

@@ -247,7 +248,7 @@ class HeapAnalyzer constructor(
parser: HprofParser,
leakingWeakRefs: List<KeyedWeakReferenceMirror>,
gcRootIds: MutableList<Long>
): List<ShortestPathFinder.Result> {
): List<Result> {
listener.onProgressUpdate(FINDING_SHORTEST_PATHS)

val pathFinder = ShortestPathFinder(heapDump.excludedRefs)
@@ -256,6 +257,7 @@ class HeapAnalyzer constructor(

private fun buildLeakTraces(
heapDump: HeapDump,
labelers: List<Labeler>,
pathResults: List<Result>,
parser: HprofParser,
leakingWeakRefs: MutableList<KeyedWeakReferenceMirror>,
@@ -278,7 +280,7 @@ class HeapAnalyzer constructor(
)
}

val leakTrace = buildLeakTrace(parser, heapDump, pathResult.leakingNode)
val leakTrace = buildLeakTrace(parser, heapDump, pathResult.leakingNode, labelers)

// TODO Compute retained heap size
val retainedSize = null
@@ -310,18 +312,20 @@ class HeapAnalyzer constructor(
private fun buildLeakTrace(
parser: HprofParser,
heapDump: HeapDump,
leakingNode: LeakNode
leakingNode: LeakNode,
labelers: List<Labeler>
): LeakTrace {
val elements = ArrayList<LeakTraceElement>()
// We iterate from the leak to the GC root
val ignored = leakingNode.instance
var node: LeakNode? =
ChildNode(ignored, null, leakingNode, null)
while (node is ChildNode) {
val element = buildLeakElement(parser, node)
if (element != null) {
elements.add(0, element)
val labels = mutableListOf<String>()
for (labeler in labelers) {
labels.addAll(labeler.computeLabels(parser, node))
}
elements.add(0, buildLeakElement(parser, node, labels))
node = node.parent
}

@@ -368,7 +372,7 @@ class HeapAnalyzer constructor(
expectedReachability[0] = Reachability.reachable("it's a GC root")
}

when(expectedReachability[lastElementIndex].status) {
when (expectedReachability[lastElementIndex].status) {
UNKNOWN, REACHABLE -> {
expectedReachability[lastElementIndex] =
Reachability.unreachable("RefWatcher was watching this")
@@ -415,13 +419,11 @@ class HeapAnalyzer constructor(

private fun buildLeakElement(
parser: HprofParser,
node: ChildNode
): LeakTraceElement? {
node: ChildNode,
labels: List<String>
): LeakTraceElement {
val objectId = node.parent.instance

val holderType: Holder
var extra: String? = null

val record = parser.retrieveRecordById(objectId)

val leakReferences = describeFields(parser, record)
@@ -436,55 +438,19 @@ class HeapAnalyzer constructor(
else -> throw IllegalStateException("Unexpected record type for $record")
}

val className = classHierarchy[0]

if (record is ClassDumpRecord) {
holderType = CLASS
val holderType = if (record is ClassDumpRecord) {
CLASS
} else if (record is ObjectArrayDumpRecord) {
holderType = ARRAY
ARRAY
} else {

val instance = parser.hydrateInstance(record as InstanceDumpRecord)

if (instance.classHierarchy.any { it.className == Thread::class.java.name }) {
holderType = THREAD
val nameField = instance.fieldValueOrNull<ObjectReference>("name")
// Sometimes we can't find the String at the expected memory address in the heap dump.
// See https://github.com/square/leakcanary/issues/417
val threadName =
if (nameField != null) parser.retrieveString(nameField) else "Thread name not available"
extra = "(named '$threadName')"
} else if (className.matches(ANONYMOUS_CLASS_NAME_PATTERN.toRegex())) {

val parentClassName = instance.classHierarchy[1].className
if (parentClassName == "java.lang.Object") {
holderType = OBJECT
try {
// This is an anonymous class implementing an interface. The API does not give access
// to the interfaces implemented by the class. We check if it's in the class path and
// use that instead.
val actualClass = Class.forName(instance.classHierarchy[0].className)
val interfaces = actualClass.interfaces
extra = if (interfaces.isNotEmpty()) {
val implementedInterface = interfaces[0]
"(anonymous implementation of " + implementedInterface.name + ")"
} else {
"(anonymous subclass of java.lang.Object)"
}
} catch (ignored: ClassNotFoundException) {
}
} else {
holderType = OBJECT
// Makes it easier to figure out which anonymous class we're looking at.
extra = "(anonymous subclass of $parentClassName)"
}
if (classHierarchy.any { it == Thread::class.java.name }) {
THREAD
} else {
holderType = OBJECT
OBJECT
}
}
return LeakTraceElement(
node.leakReference, holderType, classHierarchy, extra,
node.exclusion, leakReferences
node.leakReference, holderType, classHierarchy, node.exclusion, leakReferences, labels
)
}

@@ -556,7 +522,7 @@ class HeapAnalyzer constructor(
}

companion object {

private const val ANONYMOUS_CLASS_NAME_PATTERN = "^.+\\$\\d+$"
internal const val ANONYMOUS_CLASS_NAME_PATTERN = "^.+\\$\\d+$"
internal val ANONYMOUS_CLASS_NAME_PATTERN_REGEX = ANONYMOUS_CLASS_NAME_PATTERN.toRegex()
}
}
68 changes: 68 additions & 0 deletions leakcanary-analyzer/src/main/java/leakcanary/Labeler.kt
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
package leakcanary

import leakcanary.HeapValue.ObjectReference
import leakcanary.LeakNode.ChildNode
import leakcanary.Record.HeapDumpRecord.ObjectRecord.InstanceDumpRecord

interface Labeler {

/**
* Note: this is a bit confusing but for a given node you should really be printing attributes
* of node.parent.instance
* TODO Make this less confusing.
*/
fun computeLabels(
parser: HprofParser,
node: ChildNode
): List<String>

object InstanceDefaultLabeler : Labeler {
override fun computeLabels(
parser: HprofParser,
node: ChildNode
): List<String> {
val objectId = node.parent.instance
val record = parser.retrieveRecordById(objectId)
if (record is InstanceDumpRecord) {
val labels = mutableListOf<String>()
val instance = parser.hydrateInstance(record)
val className = instance.classHierarchy[0].className

if (instance.classHierarchy.any { it.className == Thread::class.java.name }) {
val nameField = instance.fieldValueOrNull<ObjectReference>("name")
// Sometimes we can't find the String at the expected memory address in the heap dump.
// See https://github.com/square/leakcanary/issues/417
val threadName =
if (nameField != null) parser.retrieveString(nameField) else "not available"
labels.add("Thread name: '$threadName'")
} else if (className.matches(HeapAnalyzer.ANONYMOUS_CLASS_NAME_PATTERN_REGEX)) {
val parentClassName = instance.classHierarchy[1].className
if (parentClassName == "java.lang.Object") {
try {
// This is an anonymous class implementing an interface. The API does not give access
// to the interfaces implemented by the class. We check if it's in the class path and
// use that instead.
val actualClass = Class.forName(instance.classHierarchy[0].className)
val interfaces = actualClass.interfaces
labels.add(
if (interfaces.isNotEmpty()) {
val implementedInterface = interfaces[0]
"Anonymous class implementing ${implementedInterface.name}"
} else {
"Anonymous subclass of java.lang.Object"
}
)
} catch (ignored: ClassNotFoundException) {
}
} else {
// Makes it easier to figure out which anonymous class we're looking at.
labels.add("Anonymous subclass of $parentClassName")
}
}
return labels
} else {
return emptyList()
}
}
}
}
Original file line number Diff line number Diff line change
@@ -1,9 +1,6 @@
package leakcanary.internal
package leakcanary

import leakcanary.Exclusion
import leakcanary.LeakReference

internal sealed class LeakNode {
sealed class LeakNode {
abstract val instance: Long

class RootNode(
Original file line number Diff line number Diff line change
@@ -20,6 +20,7 @@ import leakcanary.Exclusion
import leakcanary.HeapValue
import leakcanary.HeapValue.ObjectReference
import leakcanary.HprofParser
import leakcanary.LeakNode
import leakcanary.LeakReference
import leakcanary.LeakTraceElement.Type.ARRAY_ENTRY
import leakcanary.LeakTraceElement.Type.INSTANCE_FIELD
@@ -31,8 +32,8 @@ import leakcanary.ObjectIdMetadata.STRING
import leakcanary.Record.HeapDumpRecord.ObjectRecord.ClassDumpRecord
import leakcanary.Record.HeapDumpRecord.ObjectRecord.InstanceDumpRecord
import leakcanary.Record.HeapDumpRecord.ObjectRecord.ObjectArrayDumpRecord
import leakcanary.internal.LeakNode.ChildNode
import leakcanary.internal.LeakNode.RootNode
import leakcanary.LeakNode.ChildNode
import leakcanary.LeakNode.RootNode
import java.util.ArrayDeque
import java.util.Deque
import java.util.LinkedHashMap
Original file line number Diff line number Diff line change
@@ -46,7 +46,7 @@ class HeapAnalyzerTest {
val heapDump = HeapDump.builder(file)
.excludedRefs(defaultExcludedRefs.build())
.build()
val leaks = heapAnalyzer.checkForLeaks(heapDump)
val leaks = heapAnalyzer.checkForLeaks(heapDump, emptyList())

val now = System.nanoTime()
val elapsed = (now - time) / 1000000
Loading

0 comments on commit d23b26a

Please sign in to comment.