Optimize crashreporter symbols zip generation

RESOLVED FIXED in Firefox 55

Status

defect
RESOLVED FIXED
3 years ago
Last year

People

(Reporter: gps, Assigned: chmanchester)

Tracking

(Blocks 1 bug)

unspecified
mozilla55
Dependency tree / graph

Firefox Tracking Flags

(firefox55 fixed)

Details

Attachments

(2 attachments)

Reporter

Description

3 years ago
Generating crashreporter-symbols-full.zip and crashreporter-symbols.zip currently take ~70s wall time on Linux TC tasks. There are a number of relatively easy optimizations we can perform to make this faster:

* Generate the archives concurrently
* Don't attempt to compress .dbg.gz and .pd_ files (they are already compressed and compressing them again just wastes CPU cycles)
* Consider throwing a thread pool at compression (Python releases the GIL when doing zlib compress operation)

Not compressing the .dbg.gz files will likely require rewriting the compression to use mozjar. At the point we are using mozjar, we might as well sort the order of entries in the zip file so the archive is deterministic and reproducible.
Reporter

Comment 1

3 years ago
Ted's been talking about moving symbol generation into the binaries rules in the build backend. If we do that, it's also tempting to generate a zlib compressed version of each file. That way, expensive zlib compression will be performed concurrently in the build backend and all archive generation would need to do is copy compressed bits from disk into a zip file.
Reporter

Comment 2

3 years ago
* Consider streaming objdump output directly into gzip

Currently, we invoke the symbol dumper to write files to disk. Then we gzip compress these symbol files *before* they go into the symbols zip archives. If we could pipe the output from the symbol dumper directly into gzip, that would avoid hundreds of megabytes of I/O. And if we know anything about I/O in automation it is to avoid I/O in automation :)
Reporter

Comment 3

3 years ago
* Use a lower zlib level

We're using level 6 (the default). History has taught us 4 or 5 is often where we want to be on the curve to save precious seconds.
Reporter

Updated

3 years ago
Depends on: 1307435
Reporter

Comment 4

3 years ago
* Use objcopy's --compress-debug-sections instead of gzipping .dbg files

There was discussion on IRC about this. gdb can natively read those files. So symbols files could be consumed either. We'd have to change symbol server ingestion code to accommodate new "format" of the zip file, since we would no longer have .dbg.gz files.
Reporter

Comment 5

3 years ago
As part of bug 1307482, I sorted file upload order by size. symbols zip files are the largest. So that may make them a long pole during upload as well. That upload should perform at 1Gbps speeds (at least in TaskCluster), so hopefully it is only adding <10s to build tasks. Still, yet more overhead related to symbols.
This will be a good follow up to bug 1337986
Depends on: 1337986
I don't know if I'll implement all the optimizations mentioned here but I intend to stop compressing compressed files as a follow up to bug 1337986.
Assignee: nobody → cmanchester
Comment hidden (mozreview-request)

Comment 10

2 years ago
mozreview-review
Comment on attachment 8863962 [details]
Bug 1307301 - Pack symbols with a python helper and mozjar instead of zip.

https://reviewboard.mozilla.org/r/135684/#review141720

::: python/mozbuild/mozbuild/action/symbols_archive.py:17
(Diff revision 1)
> +from mozpack.mozjar import JarWriter
> +
> +def make_archive(archive_name, base, exclude, include):
> +    finder = FileFinder(base, ignore=exclude)
> +    if not include:
> +        include = ['*']

The inability to have a non-empty default with `append` is a pretty crappy design choice in argparse. :-/

::: python/mozbuild/mozbuild/action/symbols_archive.py:30
(Diff revision 1)
> +                    writer.add(p.encode('utf-8'), f.read(), mode=f.mode, skip_duplicates=True)
> +
> +def main(argv):
> +    parser = argparse.ArgumentParser(description='Produce a symbols archive')
> +    parser.add_argument('--archive', help='Which archive to generate')
> +    parser.add_argument('--base', help='Base directory to package')

You might make these positional arguments so that the script will error if it's called incorrectly.

::: python/mozbuild/mozbuild/action/symbols_archive.py:39
(Diff revision 1)
> +    args = parser.parse_args(argv)
> +
> +    make_archive(args.archive, args.base, args.exclude, args.include)
> +
> +if __name__ == '__main__':
> +    sys.exit(main(sys.argv[1:]))

If you want sys.exit to return anything useful you'll have to make `main` and `make_archive` return something. :)
Attachment #8863962 - Flags: review?(ted) → review+

Comment 11

2 years ago
mozreview-review
Comment on attachment 8863963 [details]
Bug 1307301 - Don't attempt to compress compressed files when packing the symbols archive.

https://reviewboard.mozilla.org/r/135686/#review141740

::: python/mozbuild/mozbuild/action/symbols_archive.py:20
(Diff revision 1)
> -def make_archive(archive_name, base, exclude, include):
> +def make_archive(archive_name, base, exclude, include, compress):
>      finder = FileFinder(base, ignore=exclude)
>      if not include:
>          include = ['*']
> -
> +    if not compress:
> +        compress = ['*']

Maybe we should just default this to `**/*.sym`, just in case?
Attachment #8863963 - Flags: review?(ted) → review+
Why not reuse zip.py?
(In reply to Mike Hommey [:glandium] from comment #12)
> Why not reuse zip.py?

I just didn't realize how close these are, thank you for pointing that out.
(In reply to Chris Manchester (:chmanchester) from comment #13)
> (In reply to Mike Hommey [:glandium] from comment #12)
> > Why not reuse zip.py?
> 
> I just didn't realize how close these are, thank you for pointing that out.

Now that I'm testing these side-by-side I'm finding a lot of the speedup I was getting from my patches was from setting a lower compression level. I'd have to make several other modifications to zip.py as well.
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)

Comment 17

2 years ago
Pushed by cmanchester@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/fdd37b2a2451
Pack symbols with a python helper and mozjar instead of zip. r=ted
https://hg.mozilla.org/integration/autoland/rev/8f6ff09a8a82
Don't attempt to compress compressed files when packing the symbols archive. r=ted

Comment 18

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/fdd37b2a2451
https://hg.mozilla.org/mozilla-central/rev/8f6ff09a8a82
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla55

Updated

Last year
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.