I recently removed the processing of WebIDL and IPDL files when --disable-compile-environment is in effect. This makes artifact builds a few seconds faster. We should do the same for XPIDL. Unlike WebIDL and IPDL (where I just had to disable building via "ifdef COMPILE_ENVIRONMENT"), XPIDL is slightly more involved because the xpt and manifest artifacts are distributed in omni.ja (IIRC) (they aren't all compiled into the shared libraries). This violates the boundary between compiled code in chrome, but that's the world we live in. In order to disable XPIDL processing with --disable-compile-environment, we'll need to teach the artifact build mode to install the xpt and manifest files. I reckon this will shave 4-8s off artifact build times due to skipping a lot of python build processes in the export tier.

I've kinda got this working, but only on Linux so far. We have to process omni.ja files to extract components/components.interfaces and components/interfaces.xpt files. Tentative wall time improvement for a clobber build with the .jar artifacts already cached: Before: real 0m25.721s user 0m52.472s sys 0m6.324s After: real 0m22.852s user 0m30.092s sys 0m4.444s Not the biggest wall time win (~3s). But CPU time drops significantly (~59s to ~35s) since we're no longer processing >1000 XPIDL files (in reality ~200 process invocations to produce ~200 xpt files). This will likely also translate into a more significant wall time win on Windows since we won't create >1000 files (installing .h files and creating dependency files). In the ~23s clobber build world: * ~3.5s configure * ~6.1s moz.build * ~4.0s Python package installation (surely this can be optimized) * ~0.8s artifact archive install * ~3.5s pre-export (mainly install manifests - tests are long pole and chmanchester is on that) * ~0.7s export * ~2.1s misc (feels like a lot of XPI generation) * ~1.5s libs * ~0.2s tools In that ~23s build, CPU usage only averaged ~26% (over 4+4 cores). There are a few seconds to shave here and there. <20s is achievable. I think we'll start to plateau around ~15s with in-the-pipeline improvements unless we figure out a way to saturate more cores or find some CPU abuse. Not too shabby.

This will be needed to teach artifact builds to extract files from omni.ja files whose content is loaded into memory (from a tar archive). Review commit: https://reviewboard.mozilla.org/r/36191/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/36191/

We don't need to build XPIDL files during artifact builds because the build artifacts have the interfaces.xpt and components.manifest files in them. Unlike executables and other files currently handled by artifact builds, these files are in omni.ja files within the main archive. This commit teaches the artifact processing code to extract XPIDL related files from omni.ja files. By installing these files from the artifacts, we'll be able to stop processing XPIDL source files as part of artifact builds. This commit does introduce some overhead to extract and read omni.ja files. However, this still takes less CPU than processing 1000+ XPIDL source files. TODO OS X support Review commit: https://reviewboard.mozilla.org/r/36193/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/36193/

Review commit: https://reviewboard.mozilla.org/r/36195/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/36195/

Comment on attachment 8722716 [details] MozReview Request: Bug 1240134 - Allow JARReader to read data already in memory; r?glandium https://reviewboard.mozilla.org/r/36191/#review32859

I'm going to take a stab at landing this.

Review commit: https://reviewboard.mozilla.org/r/69144/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/69144/

Comment on attachment 8777617 [details] Bug 1240134 - Incorporate the interfaces.xpt from downloaded artifacts instead of building XPIDL during an artifact build. https://reviewboard.mozilla.org/r/69144/#review67534 ::: browser/moz.build:41 (Diff revision 1) > +if CONFIG['MOZ_ARTIFACT_BUILDS']: > + # Include "pre-built interfaces" for dist/bin/browser > + # This is a dummy manifest that ensures an interfaces.xpt > + # installed to the objdir by an artifact build is incorporated > + # into the build. > + include('/build/include-interfaces.mozbuild') interesting that the build system wouldn't barf on you including this file twice: once here and once from build/moz.build. You should only need the latter. ::: python/mozbuild/mozbuild/artifacts.py:213 (Diff revision 1) > + if basename == 'omni.ja': > + # Our incoming file will be in assets/, but we need to install > + # to bin/, so ignore the base directory. > + self._process_omni_ja(f.read(), writer) This code shouldn't assume anything about omni.ja. It would be better to use an UnpackFinder, although it doesn't support to do its job from a Jar, but it should be straightforward to make it support the case through a JarFinder. It probably makes sense to change UnpackFinder to take an instance of a BaseFinder as argument and to dispatch there instead of dispatching to self as a FileFinder. ::: python/mozbuild/mozbuild/artifacts.py:252 (Diff revision 1) > with tarfile.open(filename) as reader: > for f in reader: > if not f.isfile(): > continue > > + if mozpath.basename(f.name) == 'omni.ja': same here, but we'd need a TarFinder.

Comment on attachment 8777617 [details] Bug 1240134 - Incorporate the interfaces.xpt from downloaded artifacts instead of building XPIDL during an artifact build. https://reviewboard.mozilla.org/r/69144/#review67534 > interesting that the build system wouldn't barf on you including this file twice: once here and once from build/moz.build. You should only need the latter. I found this necessary in browser/moz.build to end up with a manifest for dist/bin/browser/components/interfaces.xpt ... maybe including another mozbuild file isn't helping this, I guess it's clearer just adding to EXTRA_COMPONENTS in build/moz.build and browser/moz.build separately.

Comment on attachment 8780746 [details] Bug 1240134 - Re-factor UnpackFinder to contain a Finder instance it delegates to rather than inheriting from FileFinder. https://reviewboard.mozilla.org/r/71382/#review69250 ::: python/mozbuild/mozpack/packager/l10n.py:228 (Diff revision 1) > - app_finder = UnpackFinder(source) > - l10n_finder = UnpackFinder(l10n) > + app_finder = UnpackFinder(FileFinder(source)) > + l10n_finder = UnpackFinder(FileFinder(l10n)) This is somehow heavyweight. Why not make UnpackFinder take a path or a BaseFinder instance, and have it create the FileFinder on its own when given a path?

Comment on attachment 8780747 [details] Bug 1240134 - Implement a TarFinder to facilitate extracting files from compressed Firefox archives. https://reviewboard.mozilla.org/r/71384/#review69252 ::: python/mozbuild/mozpack/files.py:524 (Diff revision 1) > + GeneratedFile.__init__(self, tar.extractfile(info).read()) > + self._mode = info.mode > + > + @property > + def mode(self): > + return self._mode mmmmm it might be desirable to use the same mode normalization as File.mode (so factor it out from there). ::: python/mozbuild/mozpack/files.py:527 (Diff revision 1) > + @property > + def mode(self): > + return self._mode > + > + def read(self): > + return BytesIO(self.content).read() you can return self.content here. ::: python/mozbuild/mozpack/files.py:971 (Diff revision 1) > Actual implementation of JarFinder.find(), dispatching to specialized > member functions depending on what kind of pattern was given. > ''' > return self._find_helper(pattern, self._files, > lambda x: DeflatedFile(self._files[x])) > 2 empty lines between classes. Here and after the class definition.

Comment on attachment 8780746 [details] Bug 1240134 - Re-factor UnpackFinder to contain a Finder instance it delegates to rather than inheriting from FileFinder. https://reviewboard.mozilla.org/r/71382/#review69254

Comment on attachment 8777617 [details] Bug 1240134 - Incorporate the interfaces.xpt from downloaded artifacts instead of building XPIDL during an artifact build. https://reviewboard.mozilla.org/r/69144/#review69258 I think it would be better to split this patch in two, where you first convert to use UnpackFinder, and then add the interfaces.xpt.

Comment on attachment 8780746 [details] Bug 1240134 - Re-factor UnpackFinder to contain a Finder instance it delegates to rather than inheriting from FileFinder. Not sure whether these changes warrant re-review. Erring on the side of re-review.

Comment on attachment 8780746 [details] Bug 1240134 - Re-factor UnpackFinder to contain a Finder instance it delegates to rather than inheriting from FileFinder. https://reviewboard.mozilla.org/r/71382/#review69700 ::: python/mozbuild/mozpack/packager/unpack.py:42 (Diff revision 2) > > This means that for example, paths like chrome/browser/content/... match > files under jar:chrome/browser.jar!/content/... in case of jar chrome > format. > - ''' > - def __init__(self, *args, **kargs): > + > + The only argument to the constructor is a Finder instance or path. "or a path"

(In reply to Mike Hommey [:glandium] from comment #17) > I think it would be better to split this patch in two, where you first > convert to use UnpackFinder, and then add the interfaces.xpt. Let me insist on this, which was not addressed.

(In reply to Mike Hommey [:glandium] from comment #23) > (In reply to Mike Hommey [:glandium] from comment #17) > > I think it would be better to split this patch in two, where you first > > convert to use UnpackFinder, and then add the interfaces.xpt. > > Let me insist on this, which was not addressed. I'll fix this before landing.

Pushed by cmanchester@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/099a5d697513 Re-factor UnpackFinder to contain a Finder instance it delegates to rather than inheriting from FileFinder. r=glandium https://hg.mozilla.org/integration/mozilla-inbound/rev/0c2c15236f5b Implement a TarFinder to facilitate extracting files from compressed Firefox archives. r=glandium https://hg.mozilla.org/integration/mozilla-inbound/rev/91dfed0af1dc Use the UnpackFinder when extracting from archives for an artifact build. r=glandium https://hg.mozilla.org/integration/mozilla-inbound/rev/98bc41e5c777 Incorporate the interfaces.xpt from downloaded artifacts instead of building XPIDL during an artifact build. r=glandium

Pushed by cmanchester@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/0f4e45fb0783 Fixup TarFinder test on Windows by closing the TarFile after the test. r=me

https://hg.mozilla.org/mozilla-central/rev/099a5d697513 https://hg.mozilla.org/mozilla-central/rev/0c2c15236f5b https://hg.mozilla.org/mozilla-central/rev/91dfed0af1dc https://hg.mozilla.org/mozilla-central/rev/98bc41e5c777 https://hg.mozilla.org/mozilla-central/rev/0f4e45fb0783