Closed Bug 1551992 Opened 6 months ago Closed 5 months ago

Compress profile data before sending to perf-html

Categories

(Core :: Gecko Profiler, enhancement, P3)

enhancement

Tracking

()

RESOLVED FIXED
mozilla69
Tracking Status
firefox69 --- fixed

People

(Reporter: squib, Assigned: squib)

References

Details

Attachments

(1 file, 1 obsolete file)

Profiles can get pretty big, especially when screenshots or the JS tracelogger are enabled. To make uploading large profiles faster, we could compress the file locally before sending it off.

Note: This is inspired by bug 1544544, but isn't currently meant to fix that, since bug 1550702 looks like a better fix...

Attached patch Proof of concept (obsolete) — Splinter Review

This is a slightly-messy proof of concept adapted from my discarded patch from bug 1544544. It adds a getProfileAsGzippedArrayBuffer function to the WebExtension API for the profile. This should probably be moved to nsIProfiler for this bug, which will hopefully resolve some of the messiness with nsIStringInputStream...

I'm mostly just posting this here for historical interest/discussion as I adapt the patch to nsIProfiler.

The gecko profile is already fairly compressed by the format itself, with very few things that repeat themselves.
Except maybe the markers part: it could be better optimized and use the string table more, in the payload.

AFAIK the screenshots are already PNG assets, so wouldn't compress well.

So before having this I'd be interested by actual numbers before/after the gz operation.

(In reply to Julien Wajsberg [:julienw] from comment #2)

The gecko profile is already fairly compressed by the format itself, with very few things that repeat themselves.
Except maybe the markers part: it could be better optimized and use the string table more, in the payload.

AFAIK the screenshots are already PNG assets, so wouldn't compress well.

So before having this I'd be interested by actual numbers before/after the gz operation.

Hi Julien,

Here is some data I collected using each feature:

https://www.reddit.com (6s load)    JSON (MB)       GZIP (MB)
----------------------------------------------------------
Default Features :                  9               2
with Screenshots :                  13              4
with JSTracer    :                  147             15


https://www.buzzfeed.com (5s load)  JSON (MB)       GZIP (MB)
----------------------------------------------------------
Default Features :                  14              2
with Screenshots :                  21              5
with JSTracer    :                  136             13


https://www.cnbc.com (3s load)      JSON (MB)       GZIP (MB)
----------------------------------------------------------
Default Features :                  9               2
with Screenshots :                  12              3
with JSTracer    :                  153             13

Thanks, the improvements are indeed quite dramatic :)

:denispal, I've been looking at this with the plan to add something like getProfileDataAsGzippedArrayBuffer to nsIProfiler by using nsIStreamConverter to convert the data, but trying to do this with stream listeners in C++ (along the lines of attachment 9065175 [details] [diff] [review]) seems way more complex than it should be. I think you mentioned looking into doing this before; do you have any thoughts on what a better implementation would be? I could just use zlib directly, though I'm not sure that would pass review...

Of course, feel free to forward this needinfo onto anyone else you think would be a better person to ask. :)

Flags: needinfo?(dpalmeiro)

(In reply to Jim Porter (:squib) from comment #5)

:denispal, I've been looking at this with the plan to add something like getProfileDataAsGzippedArrayBuffer to nsIProfiler by using nsIStreamConverter to convert the data, but trying to do this with stream listeners in C++ (along the lines of attachment 9065175 [details] [diff] [review]) seems way more complex than it should be. I think you mentioned looking into doing this before; do you have any thoughts on what a better implementation would be? I could just use zlib directly, though I'm not sure that would pass review...

Of course, feel free to forward this needinfo onto anyone else you think would be a better person to ask. :)

If we can use zlib (maybe from modules/zlib/src/zlib.h?), then I think adding a new interface like nsProfiler::GetProfileDataAsGzippedArrayBuffer that just gzips directly isn't a bad idea? Maybe Randell has some other thoughts since Markus is on PTO.

Flags: needinfo?(dpalmeiro) → needinfo?(rjesup)

Clearing needinfo, since we discussed this over IRC.

Flags: needinfo?(rjesup)
Attachment #9065175 - Attachment is obsolete: true
Pushed by jporter@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/0ec0ee9c1e40
Compress profile data before sending to perf-html; r=mstange,julienw
Status: ASSIGNED → RESOLVED
Closed: 5 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla69
You need to log in before you can comment on or make changes to this bug.