[Telemetry] Report USS memory at time of performance marks

RESOLVED FIXED in Firefox 43

Status

Firefox OS
Developer Tools
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: russn, Assigned: russn)

Tracking

(Blocks: 2 bugs)

unspecified
FxOS-S5 (21Aug)
ARM
Gonk (Firefox OS)
Dependency tree / graph

Firefox Tracking Flags

(feature-b2g:2.5+, firefox43 fixed)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

2 years ago
For NGA telemetry purposes, we would like to record a snapshot of USS memory when performance marks are created so that developers can correlate memory consumed with performance marks.
(Assignee)

Comment 1

2 years ago
Created attachment 8642686 [details] [diff] [review]
use MemoryFront to capture uss mem in performanceEntryWatcher

Hi Ryan, I'm trying to capture uss memory in the new performanceEntryWatcher by using the MemoryFront. I believe I'm using the MemoryFront the same way the memoryWatcher is using it except that I'm not getting a result from `residentUnique`. Can you have a look at the patch? Thanks.
Flags: needinfo?(jryans)
(Assignee)

Updated

2 years ago
Blocks: 1152000
(Assignee)

Comment 2

2 years ago
When I say I'm not getting a result from `residentUnique`, I mean I don't see "huddbg measureMem memory" or "huddbg measureMem error" in the log output. I only see "in measureMem, memFront: [Front for memory/server1..."
Comment on attachment 8642686 [details] [diff] [review]
use MemoryFront to capture uss mem in performanceEntryWatcher

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

My guess is you may be running into bug 1173376, it seems this actor may not be replying.

Can you check the simulator / device log to see if you're getting a similar error when calling residentUnique?

::: b2g/chrome/content/devtools/hud.js
@@ +620,5 @@
>  
>      let front = new PerformanceEntriesFront(this._client, target.actor);
>      this._fronts.set(target, front);
>  
> +    let memFront = MemoryFront(this._client, target.actor);

You should use `new` here, but not sure if that's the real issue.  Seems `hud.js` skips this elsewhere, but still good practice.
Flags: needinfo?(jryans)
(Assignee)

Comment 4

2 years ago
I'm seeing errors in the device log [1] "Unexpected packet"; I'm guessing they are related to residentUnique as they involve the memory actor.

[1] http://pastebin.com/rs3FDMec
Flags: needinfo?(jryans)
Okay, I think the issue you're seeing is related to the other MemoryFront that another part of hud.js makes.

DevTools code currently makes the following assumption:

* Each DebuggerClient instance expects at most 1 front listening to a given actor

So, your options are:

A. Reorganize code to share the MemoryFronts created in hud.js so there is only 1 floating around per target
B. Create a new DebuggerClient for your watcher, which could then have it's own MemoryFront safely

Option B is more wasteful and would duplicate a lot of RDP packets, so option A seems like the right path to me.
Flags: needinfo?(jryans)
Actually, one approach we've used in past is to track a map of client -> front in the actor for cases like this, as in the device actor[1] for example.

We should eventually do this generically in the DevTools and stop duplicating... but for now, you could add a similar getter to the memory actor, and then use that getter in both parts of hud.js.

Up to you whether you prefer to add something to the actor like this, or change hud.js and how it works.  Either seems fine to me.

[1]: https://dxr.allizom.org/mozilla-central/source/toolkit/devtools/server/actors/device.js?offset=0#101
(Assignee)

Comment 7

2 years ago
Created attachment 8646466 [details] [diff] [review]
use getter for memory watcher front

Ryan, thanks for your feedback. I've added a MemoryFront getter to memoryWatcher that allows multiple watchers to share a MemoryFront.
Assignee: nobody → rnicoletti
Attachment #8642686 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #8646466 - Flags: review?(jryans)
Comment on attachment 8646466 [details] [diff] [review]
use getter for memory watcher front

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

Seems fine, assuming `memoryWatcher` has always created the front before you need it.
Attachment #8646466 - Flags: review?(jryans) → review+
(Assignee)

Comment 9

2 years ago
Try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=796c54b754b3
Keywords: checkin-needed

Comment 10

2 years ago
https://hg.mozilla.org/integration/b2g-inbound/rev/5d6a69634894
Keywords: checkin-needed
https://hg.mozilla.org/integration/b2g-inbound/rev/5d6a69634894
https://hg.mozilla.org/mozilla-central/rev/5d6a69634894
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
status-firefox43: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → FxOS-S5 (21Aug)
Blocks: 1180699
feature-b2g: --- → 2.5+
You need to log in before you can comment on or make changes to this bug.