Closed
Bug 1275430
Opened 8 years ago
Closed 8 years ago
Add telemetry and logging to record content process failures to start
Categories
(Core :: DOM: Content Processes, defect, P2)
Core
DOM: Content Processes
Tracking
()
RESOLVED
FIXED
mozilla49
Tracking | Status | |
---|---|---|
firefox49 | --- | fixed |
People
(Reporter: haik, Assigned: haik)
References
Details
(Whiteboard: btpp-active)
Attachments
(2 files, 2 obsolete files)
93.73 KB,
image/png
|
Details | |
3.45 KB,
patch
|
Details | Diff | Splinter Review |
+++ This bug was initially created as a clone of Bug #1165945 +++ Splitting the telemetry and logging off from 1165945 so that 1165945 can be used for improving the user experience when a content process fails to launch. When I commented out the code to start a content process to test the telemetry code, Firefox hung at launch and no UI ever was displayed. This bug will just be used for the added telemetry and logging.
Assignee | ||
Updated•8 years ago
|
Assignee | ||
Comment 1•8 years ago
|
||
bsmedberg provided data-collection review on 1165945.
Assignee: nobody → haftandilian
Assignee | ||
Updated•8 years ago
|
Attachment #8756130 -
Flags: review?(wmccloskey)
Comment on attachment 8756130 [details] [diff] [review] Adds a telemetry probe and console logging to record subprocess launch failures Review of attachment 8756130 [details] [diff] [review]: ----------------------------------------------------------------- ::: ipc/glue/GeckoChildProcessHost.cpp @@ +567,5 @@ > + return true; > + } else { > + CHROMIUM_LOG(ERROR) << "Failed to launch " << > + XRE_ChildProcessTypeToString(mProcessType) << " subprocess"; > + Telemetry::Accumulate(Telemetry::SUBPROCESS_LAUNCH_FAILURE, mProcessType); In other cases we've used keyed count histograms, which can be a little clearer. Here's an example: http://searchfox.org/mozilla-central/rev/c416322cd4079ab9ef6b30203fedae11cf39fdc5/dom/ipc/ContentParent.cpp#2092 Rather than using "content", you should be able to use the result of XRE_ChildProcessTypeToString. You probably will have to use nsDependentCString as follows: http://searchfox.org/mozilla-central/rev/c416322cd4079ab9ef6b30203fedae11cf39fdc5/ipc/glue/MessageChannel.cpp#761
Attachment #8756130 -
Flags: review?(wmccloskey)
Updated•8 years ago
|
Whiteboard: btpp-active
Assignee | ||
Comment 3•8 years ago
|
||
Changed the histogram type per billm's recommendation. Should values persist across restarts? In my testing, I found they don't. Will add about:telemetry screenshot showing the new histograms.
Attachment #8756130 -
Attachment is obsolete: true
Attachment #8758802 -
Flags: review?(wmccloskey)
Assignee | ||
Comment 4•8 years ago
|
||
Assignee | ||
Comment 5•8 years ago
|
||
(In reply to Haik Aftandilian [:haik] from comment #3) > Created attachment 8758802 [details] [diff] [review] > Should values persist across restarts? In my testing, I found they don't. I wasn't looking in the right place. The values are getting saved in the archived data sets.
Comment on attachment 8758802 [details] [diff] [review] Adds a telemetry probe and console logging to record subprocess launch failures Review of attachment 8758802 [details] [diff] [review]: ----------------------------------------------------------------- Thanks.
Attachment #8758802 -
Flags: review?(wmccloskey) → review+
Assignee | ||
Updated•8 years ago
|
Keywords: checkin-needed
Pushed by kcambridge@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/32a8d48c26ee Add telemetry and logging to record content process failures to start; r=billm
Keywords: checkin-needed
Comment 9•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/32a8d48c26ee
Status: NEW → RESOLVED
Closed: 8 years ago
status-firefox49:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla49
You need to log in
before you can comment on or make changes to this bug.
Description
•