Closed
Bug 1185061
Opened 9 years ago
Closed 9 years ago
Report dom.ipc.processCount to Telemetry
Categories
(Toolkit :: Telemetry, defect)
Toolkit
Telemetry
Tracking
()
RESOLVED
FIXED
mozilla43
People
(Reporter: vladan, Assigned: robertthyberg, Mentored)
References
Details
(Whiteboard: [good first bug][lang=js])
Attachments
(1 file, 1 obsolete file)
1.73 KB,
patch
|
vladan
:
review+
|
Details | Diff | Splinter Review |
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
Reporter | ||
Comment 1•9 years ago
|
||
Overview of Telemetry data format: https://ci.mozilla.org/job/mozilla-central-docs/Tree_Documentation/toolkit/components/telemetry/telemetry/pings.html
Comment 2•9 years ago
|
||
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.
Reporter | ||
Comment 3•9 years ago
|
||
(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
Updated•9 years ago
|
Blocks: TelemetryAdditions
Reporter | ||
Comment 5•9 years ago
|
||
(In reply to rthyberg from comment #4) > hi can i work on this bug? Certainly
Flags: needinfo?(vdjeric)
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•9 years ago
|
||
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
Attachment #8643440 -
Flags: review?(vdjeric)
Reporter | ||
Comment 9•9 years ago
|
||
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?
Flags: needinfo?(vdjeric)
Flags: needinfo?(gfritzsche)
Flags: needinfo?(benjamin)
Reporter | ||
Comment 10•9 years ago
|
||
Comment on attachment 8643440 [details] [diff] [review] name.patch Review of attachment 8643440 [details] [diff] [review]: ----------------------------------------------------------------- Looks good
Attachment #8643440 -
Flags: review?(vdjeric) → review+
Comment 11•9 years ago
|
||
(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
Flags: needinfo?(gfritzsche)
Reporter | ||
Comment 12•9 years ago
|
||
(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?
Reporter | ||
Comment 13•9 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/1190bc7b862d
Reporter | ||
Updated•9 years ago
|
Flags: needinfo?(benjamin)
Comment 14•9 years ago
|
||
Backout: https://hg.mozilla.org/integration/mozilla-inbound/rev/59ad2812d3c7
Reporter | ||
Comment 15•9 years ago
|
||
TelemetryEnviroment -> TelemetryEnviro*N*ment
Assignee | ||
Comment 16•9 years ago
|
||
fixed the spelling this time
Attachment #8643440 -
Attachment is obsolete: true
Flags: needinfo?(vdjeric)
Attachment #8645441 -
Flags: review?(vdjeric)
Updated•9 years ago
|
Assignee: nobody → robertthyberg
Status: NEW → ASSIGNED
Comment 17•9 years ago
|
||
(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.
Reporter | ||
Updated•9 years ago
|
Attachment #8645441 -
Flags: review?(vdjeric) → review+
Reporter | ||
Comment 18•9 years ago
|
||
Try push is green: https://treeherder.mozilla.org/#/jobs?repo=try&revision=15c43f35b764 But the tree is closed
Flags: needinfo?(vdjeric)
Keywords: checkin-needed
Comment 19•9 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/65972984187d
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/65972984187d
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
status-firefox43:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla43
You need to log in
before you can comment on or make changes to this bug.
Description
•