Closed buttercookie42 closed 2 years ago
This is a big one! Will have to test for a bit longer. Appreciate all the work put into this, quite a hefty change.
@MrAlex94 Yes, understandable.
One big wish: Could you please not squash this pull request together into one commit when merging? That would be really helpful if some other future changes to the build system (no, I haven't got anything planned in that area right now except possibly experimenting whether I can get PGO builds going with Github's build automation, too, but who knows…) require some code archaeology in the future.
(Edit: Also I'd dread to think what the commit message on a squashed down version of this would look like.)
One big wish: Could you please not squash this pull request together into one commit when merging?
Yes of course! I really didn't mean to squash the others, habit from Current when trying to keep the commits clean there since we rebase a lot.
Tested across Windows and Mac and seems to be good 👍
Tested across Windows and Mac and seems to be good
With PGO+LTO Enabled?
Apologies for the ginormous pull request, but
Taken together, this series of patches allows turning on both profile guided and link time optimisation (on Windows at least, that is), which should give a little bit of a performance boost, while at the same time also shrinking the size of the XUL library by a few MB. (On the flip side, it also noticeably increases build times and due to LTO also memory requirements during linking.)
I've only tested and successfully enabled it when building Waterfox on Windows, though. While I've tried backporting the relevant patches for Linux and Mac builds, too, as far as I've been able to identify them from Bugzilla, I haven't been able to test them, so no ideas whether those platforms would work, too, or whether they still require some additional work. The best I can say is that I don't seem to have broken the non-PGO/LTO-builds, as those still seem to build successfully in the Github automation.
For Macintosh there's also the fact that at least Mozilla's implementation depended on bug 1507330, which I've stopped short of during backporting. On the other hand Waterfox's builds aren't cross-compiled, so it might work even so…
Note that this pull request doesn't yet turn on PGO for Waterfox's build automation – both to put this up a little for discussion (I've run my local PGO build for a few days now without problems, but there's still a chance that enabling PGO could trigger some obscure bugs somewhere), and also because I haven't had a chance to actually try if and how this could be enabled for Waterfox's build automation. (I've also ran into some issue where sccache seems to cause "IDs have conflicting values" errors during linking, a bit like in bug 1484897, which is a bit strange because that bug was supposed to have been fixed in sccache somewhere around 0.2.8 and we've reached 0.2.15 by now…)
To give a rough summary (don't look at the commits within Github's interface because it seems it's displaying them in some sort of confusing order that only has a somewhat tenuous relationship to the actual commit order) about the contents:
--disable-tests
again, because during initial attempts, building with PGO had some sort of dependency on tests not being disabled. Of course I have absolutely no idea in how far tests would actually pass if you tried to run them, but at least they no longer break the build (at least on Windows, that is). Also it seems that bug 1412932 possibly removed the dependency between PGO and having tests enabled, but at least theoretically being able to run some tests might still be nice.MOZ_PGO=1
still didn't do much and I have no idea what exactly was broken, so it was a bit of a lucky coincidence that this bug refactored the PGO infrastructure in the build system and got things into a known-good state again. After these patches, the basic steps of PGO builds (build a build with instrumentation, do a profiling run, then re-build an optimised build based on the profiling data) are executed, except of course they don't yet do anything in practice because the build system isn't yet set up for building PGO builds with Clang.export LINKER=lld-link
line in the default mozconfig didn't actually seem to take effect and my build was still using Microsoft's linker. It's only after that bug that my build actually starts using lld-link for linking.