Add retained size to reports #166
Conversation
|
|
| @@ -107,6 +107,14 @@ public static void setEnabled(Context context, final Class<?> componentClass, | |||
| }); | |||
| } | |||
|
|
|||
| /** Based on http://stackoverflow.com/a/3758880. */ | |||
| public static String humanReadableByteCount(long bytes) { | |||
There was a problem hiding this comment.
There was a problem hiding this comment.
I think you want KiB, MiB, etc. What you have is incorrect.
http://en.wikipedia.org/wiki/Binary_prefix
There was a problem hiding this comment.
I agree but most people have no clue what KiB is. MiB... Men in Black?
There was a problem hiding this comment.
Maybe. But it will be used in context like "Activity has leaked 51.4 KiB", so reader might get it right away.
|
Super cool. LGTM |
| @@ -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); | |||
There was a problem hiding this comment.
Builder would be nice, unless you remember what this third null does.
There was a problem hiding this comment.
dang. Normally I'm the one writing this sort of comment on PRs. Oh well. I'd argue this is "ok ish" because AnalysisResult has a private constructor and a small surface, so it's all in one place.
I could also extract local variables.
There was a problem hiding this comment.
Ah, you're right. I didn't noticed it's private class. Then let's keep it hidden under the carpet :)
There was a problem hiding this comment.
I would argue that there is too many literals. Extracting local vars seems to be overkill for this though.
Maybe inline comments will make a deal? Like:
return new AnalysisResult(
false /* leakFound */,
false /* excludedLeak */,
null /* className */,
null /* leakTrace */,
null /* failure */,
0 /* retainedHeapSize */,
analysisDurationMs);There was a problem hiding this comment.
@pepyakin I'm strictly against any non-JavaDocs comments in code. They're not consistent, they're not enforced, they're not utilizing features of IDE. So, either Builder or at least JavaDoc for constructor is better.
|
This PR will never be merged because it relies on MAT. I'm keeping it around until #219 is merged and then I'll redo it with perflib. |
|
Newer version: #260 |
Fixes #162