Parsing and applying proguard mapping during heap analysis #1542
Conversation
|
|
mzgreen
changed the title
…mapping # Conflicts: # shark-graph/src/main/java/shark/HeapObject.kt # shark-graph/src/main/java/shark/HprofHeapGraph.kt # shark-graph/src/main/java/shark/ProguardMapping.kt # shark-graph/src/main/java/shark/internal/HprofInMemoryIndex.kt # shark-graph/src/test/java/shark/ProguardMappingTest.kt
| @@ -44,12 +71,14 @@ fun printHelp() { | |||
|
|
|||
| analyze-process: Dumps the heap for the provided process name, pulls the hprof file and analyzes it. | |||
| USAGE: analyze-process PROCESS_PACKAGE_NAME | |||
| (optional) -proguard_mapping PROGUARD_MAPPING_FILE_PATH | |||
There was a problem hiding this comment.
I'm curious about the distinct casing (dash for "analyze-process" vs underscore for "proguard_mapping"), is there a standard practice?
There was a problem hiding this comment.
I actually don't know, I just wanted it to look different than the "main options" because it's more like adding an additional parameter. But I started reading and it seems that it's pretty interesting topic!
This is an awesome response with links to good resources: https://softwareengineering.stackexchange.com/questions/307467/what-are-good-habits-for-designing-command-line-arguments
Maybe we should use something like: http://commons.apache.org/proper/commons-cli/ or http://jcommander.org/?
I think it should be done as a separate change, and for this one I'll change the name to match the dash convention you started. Will that be ok?
There was a problem hiding this comment.
I was thinking of checking out Clikt: #1467
There was a problem hiding this comment.
Yeah that seems cool too and it's in Kotlin :)
| val listener = OnAnalysisProgressListener { step -> | ||
| SharkLog.d { step.name } | ||
| } | ||
|
|
||
| val proguardMappingReader = proguardMappingFile?.let { ProguardMappingReader(it.inputStream()) } |
There was a problem hiding this comment.
Sorry I should have been more clear: I think we should push ProguardMappingReader to the outmost layer. I can't think of a good reason for HeapAnalyzer to know about where the mapping comes from. The general idea is to enable doing this even when we don't have a file
There was a problem hiding this comment.
Sorry for misunderstanding, I moved the reader our of the analyzer :)
| @@ -25,7 +52,7 @@ fun printHelp() { | |||
|
|
|||
| // ASCII art is a remix of a shark from -David "TAZ" Baltazar- and chick from jgs. | |||
| SharkLog.d { | |||
| """ | |||
| """ | |||
| @@ -96,7 +125,7 @@ private fun dumpHeap(packageName: String): File { | |||
| val heapDumpDevicePath = "/data/local/tmp/$heapDumpFileName" | |||
|
|
|||
| SharkLog.d { | |||
| "Dumping heap for process \"$processName\" with pid $processId to $heapDumpDevicePath" | |||
| @@ -81,7 +110,7 @@ private fun dumpHeap(packageName: String): File { | |||
| matchingExactly | |||
| } else { | |||
| SharkLog.d { | |||
| "More than one process matches \"$packageName\" but none matches exactly: ${matchingProcesses.map { it.first }}" | |||
I created a proguard mapping parser and hooked it into the heap analysis process.
Please review it and I'm happy to fix any issues you find.
Note that this is not ready to merge yet - I need to figure out the best way to expose an API for providing proguard mapping files.
Resolves #1499