1268938 - Most reply Messages don't have a name set

Status: RESOLVED FIXED

(Core :: IPC, defect, P1)

Description

[Andrew McCreight [:mccr8]]

Bug 1264820 adds telemetry for the size of reply messages. However, most, but not all, of them seem to not set a name, so the name is just "???". It seems like sync calls that don't return values, like PCompositorBridge::FlushRendering, set the name, but those that do, like PContent::GetXPCOMProcessAttributes do not. This also shows up in existing IPC logging.

Comment 1

[Andrew McCreight [:mccr8]]

I guess what happens is that when you send a message, it goes like IPDL -> MessageChannel -> Chromium IPC. IPDL sets the name of the message, and telemetry reports the name. When you get a reply, it goes in the opposite direction, so when you are doing telemetry in MessageChannel before the name is set. Actually, nothing sets the message name in IPDL for a reply, but it could.

I'm not 100% sure why some messages have their name set, but I think they are not coming directly from the other process. (They are coming in via ThreadLink::SendMessage() instead of ProcessLink::OnMessageReceived().)

Comment 2

[Andrew McCreight [:mccr8]]

Attachment: Set a name for reply messages.

This is a hacky solution, but it works.

https://treeherder.mozilla.org/#/jobs?repo=try&revision=c270defeeb71

Comment 3

[Andrew McCreight [:mccr8]]

This makes Send() and Call() take an extra argument that is the message name. Before the methods return the reply message or do telemetry, set the message. All callers of Send() and Call() are in codegenned IPDL code, so they are easy to fix, using prettyReplyName.

Also, we don't collect telemetry for the reply messages in Call(), but I didn't see any when loading cnn.com so hopefully they aren't too important. I could also just add telemetry for that. I'm not sure what it is for.

Comment 4

[Jed Davis [:jld] ⟨⏰|UTC-8⟩ ⟦he/him⟧]

(In reply to Andrew McCreight [:mccr8] from comment #3)
> Also, we don't collect telemetry for the reply messages in Call(), but I
> didn't see any when loading cnn.com so hopefully they aren't too important.

It looks like Call is only for `intr` messages, and it looks like those are used only by plugins (heavily by NPAPI and a few things in GMP).

Comment 5

[Andrew McCreight [:mccr8]]

Comment on attachment 8747312 [details] [diff] [review]
Set a name for reply messages.

Bill pointed out that we could just take the name from the sending message and use that, rather than pass a new thing around.

Comment 6

[Andrew McCreight [:mccr8]]

Attachment: Use the name of the original message in Send for reply telemetry.

This is much nicer than my original patch.

As I mentioned to Bill, the root of this is that there was a bug in my fix for the bug in Bill's original patch in bug 1264820.

Comment 7

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/53d3b7cdb82f

Comment 8

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/53d3b7cdb82f

Comment 9

[Andrew McCreight [:mccr8]]

Comment on attachment 8748001 [details] [diff] [review]
Use the name of the original message in Send for reply telemetry.

Approval Request Comment
[Feature/regressing bug #]: e10s
[User impact if declined]: This improves some telemetry that might help investigating e10s OOM crashes.
[Describe test coverage new/current, TreeHerder]: this code runs very frequently.
[Risks and why]: Low. The patch is simple. I verified that the expected telemetry has started showing up in Nightly.
[String/UUID change made/needed]: none

Comment 10

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Comment on attachment 8748001 [details] [diff] [review]
Use the name of the original message in Send for reply telemetry.

Needed to better diagnose e10s OOM crashes, telemetry data was verified on Nightly, Aurora48+, Beta47+

Comment 11

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

https://hg.mozilla.org/releases/mozilla-aurora/rev/cb9344d9c9ad

Comment 12

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

https://hg.mozilla.org/releases/mozilla-beta/rev/3a218b10e323