Closed Bug 1384175 Opened 7 years ago Closed 7 years ago

assemblePayloadWithMeasurements can be slow

Categories

(Toolkit :: Telemetry, enhancement, P1)

enhancement

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
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)
(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)
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?
Priority: -- → P3
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
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)
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.