Closed Bug 1307301 Opened 8 years ago Closed 7 years ago

Optimize crashreporter symbols zip generation

Categories

(Firefox Build System :: General, defect)

defect
Not set
normal

Tracking

(firefox55 fixed)

RESOLVED FIXED
mozilla55
Tracking Status
firefox55 --- fixed

People

(Reporter: gps, Assigned: chmanchester)

References

(Blocks 1 open bug)

Details

Attachments

(2 files)

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.
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.
* 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 :)
* 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.
Depends on: 1307435
* 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.
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 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 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.
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
https://hg.mozilla.org/mozilla-central/rev/fdd37b2a2451
https://hg.mozilla.org/mozilla-central/rev/8f6ff09a8a82
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.