1037465 - Add USS reporting to the Monitor actor

Status: RESOLVED FIXED

(DevTools Graveyard :: WebIDE, defect)

Description

[Jan Keromnes [:janx]]

In addition to forwarding Monitor updates, the Monitor actor should also be able to start/stop one or more reporting agents.

One type of reporting agent we're interested in is Unique Set Size for all processes.

Comment 1

[Jan Keromnes [:janx]]

Attachment: Add USS reporting to the Monitor actor.

Comment 2

[Jan Keromnes [:janx]]

Comment on attachment 8456914 [details] [diff] [review]
Add USS reporting to the Monitor actor.

Paul, care to take a look?

Comment 3

[Paul Rouget [:paul]]

(you need to remove the dbg-client changes)

Comment 4

[Jan Keromnes [:janx]]

Attachment: Add USS reporting to the Monitor actor.

Carry over Paul's r+ (if git-bz feels generous).

Comment 5

[Jan Keromnes [:janx]]

Comment on attachment 8457944 [details] [diff] [review]
Add USS reporting to the Monitor actor.

So git-bz didn't feel generous, carrying over by hand.

Indeed the dbg-client.jsm changes are already landed as bug 1039448, and the shell.js change wasn't necessary. Removed and rebased.

Comment 6

[Ryan VanderMeulen [:RyanVM]]

Sorry for the hassle, but is there a Try run for this?

Comment 7

[Jan Keromnes [:janx]]

Attachment: Add USS reporting to the Monitor actor.

Rebased, try run: https://tbpl.mozilla.org/?tree=Try&rev=11db123bbf42

Comment 8

[Jan Keromnes [:janx]]

Comment on attachment 8459478 [details] [diff] [review]
Add USS reporting to the Monitor actor.

(this whole carrying over thing feels prehistoric)

Comment 9

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

https://hg.mozilla.org/integration/fx-team/rev/236c33cffbe4

Comment 10

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

This apparently broke an xpcshell test: https://tbpl.mozilla.org/php/getParsedLog.php?id=44318344&tree=Fx-Team

Backed out in https://hg.mozilla.org/integration/fx-team/rev/76c587b06771

Comment 11

[Jan Keromnes [:janx]]

Attachment: Add USS reporting to the Monitor actor.

Fixed test.

Comment 12

[Jan Keromnes [:janx]]

Comment on attachment 8462571 [details] [diff] [review]
Add USS reporting to the Monitor actor.

Carry over Paul's r+.

Comment 13

[Paul Rouget [:paul]]

https://tbpl.mozilla.org/?tree=Try&rev=98b927c76b85

Comment 14

[Paul Rouget [:paul]]

test_monitor_actor.js | Test timed out

Comment 15

[Paul Rouget [:paul]]

Jan - did you try to call update as a start callback?

Comment 16

[Jan Keromnes [:janx]]

Yes I tried that, still times out on try https://tbpl.mozilla.org/?tree=Try&rev=0e3578240e90 (even though it works on my machine). Doesn't look intermittent.

Comment 17

[Paul Rouget [:paul]]

`residentUnique` is not available on Windows and Mac.

Only call `MonitorActor.prototype._addAgent(USSAgent)` if `residentUnique` is available (or on Linux).

And enable the test just for Linux,

Comment 18

[Jan Keromnes [:janx]]

Attachment: Add USS reporting to the Monitor actor.

Fixed for non-linux platforms by auto-removing update agents whose `start()` method throws.

Try: https://tbpl.mozilla.org/?tree=Try&rev=380f64d73706

Comment 19

[Jan Keromnes [:janx]]

Comment on attachment 8463346 [details] [diff] [review]
Add USS reporting to the Monitor actor.

Paul, asking you to re-review because I slightly changed the way update agents work (see previous comment).

Do you think this approach makes sense? If you do, and if the change really fixes non-linux xpcshell tests, please land the patch for me.

Comment 20

[Paul Rouget [:paul]]

https://hg.mozilla.org/integration/fx-team/rev/72db2ae08bc5

Comment 21

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/72db2ae08bc5