Closed
Bug 1384175
Opened 7 years ago
Closed 7 years ago
assemblePayloadWithMeasurements can be slow
Categories
(Toolkit :: Telemetry, enhancement, P1)
Toolkit
Telemetry
Tracking
()
RESOLVED
FIXED
People
(Reporter: mstange, Unassigned)
References
Details
Here's a profile of a 66ms jank in the parent process due to telemetry payload assembly: https://perfht.ml/2eKT7U9
Comment 1•7 years ago
|
||
Which build? A refactor _just_ landed (bug 1366294) that could speed this up slightly. But only slightly. Probably still more to do. What comes to mind: bug 1376600 is all about removing the registeredHistograms filtering nonsense, so that deserves a "Depends on" We pack the histograms individually in JS. I suppose we could do that instead in C++ and make it more aware of our transport structure. As a bonus it reduces the size of the data we need to marshal from C++ to JS... but it may be complex as we're currently using it to deal with threadHangs (until bug 1380081 changes it completely)
Depends on: 1376600
Flags: needinfo?(mstange)
Reporter | ||
Comment 2•7 years ago
|
||
(In reply to Chris H-C :chutten from comment #1) > Which build? A refactor _just_ landed (bug 1366294) that could speed this up > slightly. mozilla-central revision 5928d905c0bc, so I think yesterday's nightly. If the way we are dealing with thread hangs is changing anyway, then we probably shouldn't worry about this bug until that's done. Thread hang data is probably the major part of the telemetry ping anyway.
Flags: needinfo?(mstange)
Comment 3•7 years ago
|
||
Yup, so that'd be right before the refactor landed. With that, bug 1376600, and the BHR ping, I think we can wait to see if this still shows up later in August. Do you agree?
Updated•7 years ago
|
Priority: -- → P3
Comment 4•7 years ago
|
||
It's looking like most of the slow parts have been commented out in the storage refactor. I'll conduct a profile of the slowest possible case tomorrow on my Linux box and see if it is now as fast as we hope.
Flags: needinfo?(chutten)
Priority: P3 → P1
Comment 5•7 years ago
|
||
https://perf-html.io/public/f7d9c731ccaf22c74d3999c8b53b04c3411f675c/calltree/?hiddenThreads=&implementation=js&invertCallstack&search=elemetry&thread=0&threadOrder=0-1 So... well, I guess things sped up somewhat as a worst-case getPayload('gather-payload') doesn't even show up in the profile when performed on a debug build of latest mozilla-central. Of course, this is on a fast Linux machine. However, I did make sure to accumulate to each and every histogram so it would have to filter, serialize, then pack the largest structure. Once the registeredHistograms patch lands I'll close this against all these lovely perf improvements we've made.
Flags: needinfo?(chutten)
Updated•7 years ago
|
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•