1185061 - Report dom.ipc.processCount to Telemetry

Status: RESOLVED FIXED

(Toolkit :: Telemetry, defect)

Description

[Vladan Djeric (:vladan)]

The dom.ipc.processCount pref controls whether multi-process Firefox (also known as e10s) uses a single web content process or whether it spawns a new process for every new tab.

The default configuration is dom.ipc.processCount=1 (i.e. only one content process), but some advanced users are testing out Firefox in process-per-tab mode with processCount > 1. We'd like to be able to distinguish between these configurations when studying Firefox performance using Telemetry.


What is E10S? https://wiki.mozilla.org/Electrolysis
What is Telemetry? https://wiki.mozilla.org/Telemetry

Code to modify: http://mxr.mozilla.org/mozilla-central/source/toolkit/components/telemetry/TelemetrySession.jsm#1105

Comment 1

[Vladan Djeric (:vladan)]

Overview of Telemetry data format: 
https://ci.mozilla.org/job/mozilla-central-docs/Tree_Documentation/toolkit/components/telemetry/telemetry/pings.html

Comment 2

[Benjamin Smedberg]

I suggest that it would be better to return this in the environment block by modifying this list:

http://mxr.mozilla.org/mozilla-central/source/toolkit/components/telemetry/TelemetryEnvironment.jsm#82

It's weird that the e10s setting is separate from everything else in TelemetrySession.

Comment 3

[Vladan Djeric (:vladan)]

(In reply to Benjamin Smedberg  [:bsmedberg] (away until 27-July) from comment #2)
> I suggest that it would be better to return this in the environment block

ok

Comment 4

[rthyberg]

hi can i work on this bug?

Comment 5

[Vladan Djeric (:vladan)]

(In reply to rthyberg from comment #4)
> hi can i work on this bug?

Certainly

Comment 6

[rthyberg]

So should this be added to the telemetryEnviroment.jsm or telemetrySession.jsm? I added it to default enviroment vairables. How should i test it? This is my first bug

Comment 7

[Georg Fritzsche [:gfritzsche]]

The location shown in comment 2 would be best.

You can test your changes manually by going to about:telemetry and checking its present in the Environment -> settings section.
It should reflect the current value in about:config.

Automated tests can be run using: mach xpcshell-test toolkit/components/telemetry/tests/unit

Comment 8

[rthyberg]

Attachment: name.patch

Comment 9

[Vladan Djeric (:vladan)]

I remembered why I wanted to report this pref outside TelemetryEnvironment. I didn't want to start a new Telemetry sub-session as soon as the pref was changed because the pref only takes effect on restart.

Georg or Benjamin, what do you think?

Comment 10

[Vladan Djeric (:vladan)]

Comment on attachment 8643440 [details] [diff] [review]
name.patch

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

Looks good

Comment 11

[Georg Fritzsche [:gfritzsche]]

(In reply to Vladan Djeric (:vladan) -- please needinfo! from comment #9)
> I remembered why I wanted to report this pref outside TelemetryEnvironment.
> I didn't want to start a new Telemetry sub-session as soon as the pref was
> changed because the pref only takes effect on restart.
> 
> Georg or Benjamin, what do you think?

I don't think that would be a problem with that pref for now, we wouldn't expect it to change often.

For the more general issue, we can just add e.g. an optional flag to the pref definitions [0] that means "don't start a new subsession when this changes".
Sounds good?

0: https://dxr.mozilla.org/mozilla-central/rev/07befc6f54e743b8be189cd2c14d74cf1bcef1c2/toolkit/components/telemetry/TelemetryEnvironment.jsm#83

Comment 12

[Vladan Djeric (:vladan)]

(In reply to Georg Fritzsche [:gfritzsche] from comment #11)
> For the more general issue, we can just add e.g. an optional flag to the
> pref definitions [0] that means "don't start a new subsession when this
> changes".
> Sounds good?

Yes, can you file another bug?

Comment 13

[Vladan Djeric (:vladan)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/1190bc7b862d

Comment 14

[Pulsebot]

Backout:
https://hg.mozilla.org/integration/mozilla-inbound/rev/59ad2812d3c7

Comment 15

[Vladan Djeric (:vladan)]

TelemetryEnviroment -> TelemetryEnviro*N*ment

Comment 16

[rthyberg]

Attachment: added dom.ipc.processCount to TelemetryEnvironment.jsm

fixed the spelling this time

Comment 17

[Georg Fritzsche [:gfritzsche]]

(In reply to Vladan Djeric (:vladan) -- please needinfo! from comment #12)
> (In reply to Georg Fritzsche [:gfritzsche] from comment #11)
> > For the more general issue, we can just add e.g. an optional flag to the
> > pref definitions [0] that means "don't start a new subsession when this
> > changes".
> > Sounds good?
> 
> Yes, can you file another bug?

Filed bug 1192811.

Comment 18

[Vladan Djeric (:vladan)]

Try push is green: https://treeherder.mozilla.org/#/jobs?repo=try&revision=15c43f35b764

But the tree is closed

Comment 19

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/65972984187d

Comment 20

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

https://hg.mozilla.org/mozilla-central/rev/65972984187d