Closed Bug 1238066 Opened 4 years ago Closed 4 years ago
Canary to local builds
58 bytes, text/x-review-board-request
LeakCanary is a memory leak detection library for Android and Java: https://github.com/square/leakcanary We just want to use this in local builds (maybe in Nightly?). Square offers two libraries: The actual lib and a no-op lib that can be used to compile release versions without the need to change the code using LeakCanary: * com.squareup.leakcanary:leakcanary-android:1.3.1 * com.squareup.leakcanary:leakcanary-android-no-op:1.3.1 By default LeakCanary will detect Activity leaks. For everything else we'll need to manually add references to RefWatcher if they are interesting to monitor.
I recently did this locally (just in the gradle build because that's easy) and it worked without issues (I didn't find any leaks though).
(In reply to Sebastian Kaspari (:sebastian) from comment #1) > I recently did this locally (just in the gradle build because that's easy) > and it worked without issues (I didn't find any leaks though). Somebody (mcomella? mhaigh?) did this before. Post the patch and let's get it into Gradle, at least. We can try to get the no-op thing working in moz.build if it's useful.
(In reply to Nick Alexander :nalexander from comment #2) > (In reply to Sebastian Kaspari (:sebastian) from comment #1) > > I recently did this locally (just in the gradle build because that's easy) > > and it worked without issues (I didn't find any leaks though). > > Somebody (mcomella? mhaigh?) did this before. Post the patch and let's get > it into Gradle, at least. We can try to get the no-op thing working in > moz.build if it's useful. I had a gist that did this long ago: https://gist.github.com/leibovic/38c21068e4ac58e5c751
This patch adds LeakCanary to gradle builds and the no-op library to mach-based builds. Review commit: https://reviewboard.mozilla.org/r/31731/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/31731/
Attachment #8710373 - Flags: review?(nalexander)
I tested this patch with a manually created leak. I mean eventually we could add LeakCanary to mach based builds too (There are a bunch of classes though..) and hide it behind a build flag. By default LeakCanary only detects Activity leaks. If we want to use it for more then we should expose the RefWatcher instance like described here: https://github.com/square/leakcanary#how-do-i-use-it
Assignee: nobody → s.kaspari
Status: NEW → ASSIGNED
Comment on attachment 8710373 [details] MozReview Request: Bug 1238066 - Add LeakCanary to local (gradle) builds. r?nalexander https://reviewboard.mozilla.org/r/31731/#review28719 I'd like to see a green try build, but other than that, rock on! I will have to rebase a bunch of Gradle work on top of this, but whatever :) ::: mobile/android/app/base/build.gradle:121 (Diff revision 1) > + // Gradle based debug builds include LeakCanary. Mach based builds only include the no-op It's not quite true that it's debug only. As written, it's *all* Gradle builds. (Unless LeakCanary itself differentiates, which it can and might.) Let's do not be clever about debug/release in our build.gradle files for now, since it's easier. ::: mobile/android/config/proguard/leakcanary-keeps.cfg:6 (Diff revision 1) > +# For Android 6.0 compatibility nit: full sentences with periods, please. ::: mobile/android/thirdparty/com/squareup/leakcanary/LeakCanary.java:1 (Diff revision 1) > +package com.squareup.leakcanary; nit: Include license. I think this should be MPL, since it's our version.
Attachment #8710373 - Flags: review?(nalexander) → review+
https://hg.mozilla.org/integration/fx-team/rev/2ff2a6523bc06d93a5d55d848c8638296f0f574f Bug 1238066 - Add LeakCanary to local (gradle) builds. r=nalexander
Product: Firefox for Android → Firefox Build System
Target Milestone: Firefox 47 → mozilla47
You need to log in before you can comment on or make changes to this bug.