1551992 - Compress profile data before sending to perf-html

Status: RESOLVED FIXED

(Core :: Gecko Profiler, enhancement, P3)

Description

[Jim Porter (:squib)]

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...

Comment 1

[Jim Porter (:squib)]

Attachment: Proof of concept

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.

Comment 2

[Julien Wajsberg [:julienw]]

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.

Comment 3

[Denis Palmeiro [:denispal]]

(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

Comment 4

[Julien Wajsberg [:julienw]]

Thanks, the improvements are indeed quite dramatic :)

Comment 5

[Jim Porter (:squib)]

: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. :)

Comment 6

[Denis Palmeiro [:denispal]]

(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.

Comment 7

[Jim Porter (:squib)]

Filed https://github.com/firefox-devtools/profiler/issues/2044 and https://github.com/firefox-devtools/Gecko-Profiler-Addon/issues/165 for the perf-html and add-on parts of this, respectively.

Comment 8

[Jim Porter (:squib)]

Attachment: Bug 1551992 - Compress profile data before sending to perf-html; r?mstange

Comment 9

[Jim Porter (:squib)]

Clearing needinfo, since we discussed this over IRC.

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/0ec0ee9c1e40
Compress profile data before sending to perf-html; r=mstange,julienw

Comment 11

[Arthur Iakab [arthur_iakab]]

https://hg.mozilla.org/mozilla-central/rev/0ec0ee9c1e40