Closed
Bug 1307301
Opened 8 years ago
Closed 7 years ago
Optimize crashreporter symbols zip generation
Categories
(Firefox Build System :: General, defect)
Firefox Build System
General
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.
Reporter | ||
Comment 1•8 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•8 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•8 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 | ||
Comment 4•8 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•8 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.
Assignee | ||
Comment 7•8 years ago
|
||
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 hidden (mozreview-request) |
Comment 10•8 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•8 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+
Comment 12•8 years ago
|
||
Why not reuse zip.py?
Assignee | ||
Comment 13•7 years ago
|
||
(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.
Assignee | ||
Comment 14•7 years ago
|
||
(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•7 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•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/fdd37b2a2451
https://hg.mozilla.org/mozilla-central/rev/8f6ff09a8a82
Status: NEW → RESOLVED
Closed: 7 years ago
status-firefox55:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
Updated•7 years ago
|
Product: Core → Firefox Build System
You need to log in
before you can comment on or make changes to this bug.
Description
•