1190622 - [Telemetry] Report USS memory at time of performance marks

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Developer Tools, defect)

Description

[Russ Nicoletti [:russn]]

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.

Comment 1

[Russ Nicoletti [:russn]]

Attachment: 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.

Comment 2

[Russ Nicoletti [:russn]]

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 3

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

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.

Comment 4

[Russ Nicoletti [:russn]]

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

Comment 5

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

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.

Comment 6

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

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

Comment 7

[Russ Nicoletti [:russn]]

Attachment: 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.

Comment 8

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

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.

Comment 9

[Russ Nicoletti [:russn]]

Try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=796c54b754b3

Comment 10

[Pulsebot]

https://hg.mozilla.org/integration/b2g-inbound/rev/5d6a69634894

Comment 11

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/integration/b2g-inbound/rev/5d6a69634894

Comment 12

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/5d6a69634894