Closed Bug 1078870 Opened 5 years ago Closed 5 years ago

Developer HUD doesn't log memory changes in ADB

Categories

(Firefox OS Graveyard :: Developer Tools, defect)

All
Gonk (Firefox OS)
defect
Not set

Tracking

(blocking-b2g:2.1+, firefox33 wontfix, firefox34 wontfix, firefox35 fixed, b2g-v2.1 fixed, b2g-v2.2 fixed)

RESOLVED FIXED
2.1 S6 (10oct)
blocking-b2g 2.1+
Tracking Status
firefox33 --- wontfix
firefox34 --- wontfix
firefox35 --- fixed
b2g-v2.1 --- fixed
b2g-v2.2 --- fixed

People

(Reporter: janx, Assigned: janx)

Details

Attachments

(1 file, 2 obsolete files)

In the Developer HUD, checking "Log changes in ADB" doesn't work for memory changes ("Unique Set Size" or "App memory").

Instead, it should log memory changes, and give the detailed composition of the "App memory" metric.
Harald, this change causes the memory widgets in Developer HUD to also log to ADB. It adds a considerable amount of logs, and they look like this:

(USS)
> I/GeckoDump( 2436): DeveloperHUD: [app://system.gaiamobile.org/manifest.webapp] uss: 59.36 MB
> I/GeckoDump( 2436): DeveloperHUD: [app://findmydevice.gaiamobile.org/manifest.webapp] uss: 11.97 MB
> I/GeckoDump( 2436): DeveloperHUD: [app://verticalhome.gaiamobile.org/manifest.webapp] uss: 15.7 MB

(App memory, partial details)
> I/GeckoDump( 2436): DeveloperHUD: [app://system.gaiamobile.org/manifest.webapp] app memory: 1.6 MB (js objects: 1.29 MB, js strings: 315.06 KB)
> I/GeckoDump( 2436): DeveloperHUD: [app://keyboard.gaiamobile.org/manifest.webapp] app memory: 216.88 KB (js objects: 214.28 KB, js strings: 2.59 KB)
> I/GeckoDump( 2436): DeveloperHUD: [app://verticalhome.gaiamobile.org/manifest.webapp] app memory: 265.38 KB (js objects: 245.27 KB, js strings: 20.11 KB)

(App memory, full details)
> I/GeckoDump( 2436): DeveloperHUD: [app://system.gaiamobile.org/manifest.webapp] app memory: 6.93 MB (js objects: 1.1 MB, js strings: 211.53 KB, js other: 2.64 MB, dom: 526.68 KB, style: 1.5 MB, other: 994.99 KB)
> I/GeckoDump( 2436): DeveloperHUD: [app://callscreen.gaiamobile.org/manifest.webapp] app memory: 785.56 KB (js objects: 164.36 KB, js strings: 480 B, js other: 362.93 KB, dom: 78.85 KB, style: 178.95 KB, other: 4 B)
> I/GeckoDump( 2436): DeveloperHUD: [app://verticalhome.gaiamobile.org/manifest.webapp] app memory: 1.88 MB (js objects: 245.77 KB, js strings: 20.84 KB, js other: 789.43 KB, dom: 107.81 KB, style: 237.11 KB, other: 523.16 KB)
> I/GeckoDump( 2436): DeveloperHUD: [app://settings.gaiamobile.org/manifest.webapp] app memory: 4.12 MB (js objects: 675.39 KB, js strings: 65.19 KB, js other: 1.29 MB, dom: 457.24 KB, style: 697.12 KB, other: 1007.82 KB)

Does that log format work for you? Would you prefer values to be represented as un unformatted byte count?
Flags: needinfo?(hkirschner)
Thanks janx, that is super helpful! On the format it feels preferable to have it readable vs easily parsable so what you have works.
Flags: needinfo?(hkirschner)
Comment on attachment 8501406 [details] [diff] [review]
Make the Developer HUD log memory changes in ADB.

Cool then! Try looks green, Alex please have a look :)
Attachment #8501406 - Flags: review?(poirot.alex)
Comment on attachment 8501406 [details] [diff] [review]
Make the Developer HUD log memory changes in ADB.

Review of attachment 8501406 [details] [diff] [review]:
-----------------------------------------------------------------

::: b2g/chrome/content/devtools/hud.js
@@ -426,2 @@
>        return;
> -    }

The common code guideline for JS code on m-c is to use brackets {}, even for one line statement.
Attachment #8501406 - Flags: review?(poirot.alex) → review+
Attachment #8501406 - Attachment is obsolete: true
Attachment #8502632 - Flags: review+
De-nitted and rebased, r+ carried over, metric names are now capitalized in logs.

https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=9074f14fa78c
Attachment #8502632 - Attachment is obsolete: true
Attachment #8502689 - Flags: review+
Try is taking ages, previous run was green enough[1], checkin-needed please.

[1] https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=ecf2a7f89feb
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/629c28f4c2e5
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
[Blocking Requested - why for this release]: Very useful for debugging memory and we still target low-memory devices.
blocking-b2g: --- → 2.1?
Component: Gaia::System → Developer Tools
blocking-b2g: 2.1? → 2.1+
Please request b2g34 approval on this patch when you get a chance :)
Flags: needinfo?(janx)
Target Milestone: --- → 2.1 S6 (10oct)
Comment on attachment 8502689 [details] [diff] [review]
Make the Developer HUD log memory changes in ADB.

NOTE: Please see https://wiki.mozilla.org/Release_Management/B2G_Landing to better understand the B2G approval process and landings.

[Approval Request Comment]

Bug caused by (feature/regressing bug #): We're changing the decision made in bug 963498 and bug 976024 not to log for memory.

User impact if declined: Without this, a user who enables Developer HUD > Developer Tools, and checks both "Log changes in ADB" and either "Unique Set Size" or "App Memory", will not see memory changes logged in ADB. See comment 12.

Testing completed: Manually tested, Developer HUD now logs memory changes when "Log changes in ADB" is on and memory is being watched.

Risk to taking this patch (and alternatives if risky): Low risk because it just changes what's being logged.

String or UUID changes made by this patch: none
Flags: needinfo?(janx)
Attachment #8502689 - Flags: approval-mozilla-b2g34?
Attachment #8502689 - Flags: approval-mozilla-b2g34? → approval-mozilla-b2g34+
You need to log in before you can comment on or make changes to this bug.