1224450 - (fast-compiledb) Make the CompileDB backend work off moz.build data instead of recursing the make build system

Status: RESOLVED FIXED

(Firefox Build System :: General, defect)

Description

[Mike Hommey [:glandium]]

The way the CompileDB backend works at the moment is that it essentially goes through all directories and does make showbuild for all of them, and this is very slow.

There are various things to overcome to make this possible, so this is likely to become a forest of bug dependencies. The goal is also not limited to the CompileDB backend: the real goal is to feed the FasterMake backend with everything needed to build C++, but making CompileDB faster is a good proxy for that because it forces to do all the things required to make the end goal actually happen.

I have done preliminary work (in the form of a very awful hack) that is able to generate, for a linux opt build, about 70% of the same command lines the current CompileDB backend generates. I will file dependent bugs according to my findings from this preliminary work.

Comment 1

[Mike Hommey [:glandium]]

The current status is that I'm able to generate 99.51% (2023 of 2033) of all target compile commands (so, no host compile) on a linux64 non-debug build. The remaining ones are:

- build/unix/elfhack/dummy.c
- build/unix/elfhack/test-array.c
- build/unix/elfhack/test-ctors.c
- $objdir/build/unix/elfhack/inject/x86_64.c
Those are actually tricky for multiple reasons, and I will ignore them for now.

- toolkit/xre/nsAppRunner.cpp
- toolkit/xre/nsEmbedFunctions.cpp
- toolkit/xre/ProfileReset.cpp
- $objdir/toolkit/xre/Unified_cpp_toolkit_xre0.cpp
All those add something like `-DTOOLKIT_EM_VERSION="46.0a1" -DGRE_BUILDID=20151230084421` through their Makefile.

- webapprt/gtk/webapprt.cpp
This one adds something like `-DGRE_BUILDID=20151230084421` through its Makefile. But it's set to go away soon, since webapprt is going to be removed.

- browser/app/nsBrowserApp.cpp
This one adds `-DPBMODE_ICO="../../dist/branding/pbmode.ico" -DNEWTAB_ICO="../../dist/branding/newtab.ico" -DNEWWINDOW_ICO="../../dist/branding/newwindow.ico" '-DDOCUMENT_ICO="../../dist/branding/document.ico" -DFIREFOX_ICO="../../dist/branding/firefox.ico"`, but those defines are actually not used for the C++ file, only for the RC file in the same directory.

Comment 3

[Mike Hommey [:glandium]]

Time for a status update: above 98% of target C, C++, ObjC, ObjC++ on Android, Mac and Linux. (no rust, and no host compilations ; the former is not currently supported by CompileDB anyways, and the latter is supported, but plenty wrong)
The closest to 100% is Android, where it's down to 4 files, all exclusively because of -DGRE_BUILDID.

Comment 4

[Mike Hommey [:glandium]]

(In reply to Mike Hommey [:glandium] from comment #3)
> The closest to 100% is Android, where it's down to 4 files, all exclusively
> because of -DGRE_BUILDID.

(with a yet-to-be-filed patch that I'm thoroughly testing on try because it removes a Makefile containing a workaround that I think is not necessary anymore, but needs douple checking in different configurations)

Comment 5

[Mike Hommey [:glandium]]

Attachment: MozReview Request: Bug 1224450 - Ignore host compilations in the CompileDB backend. r=gps

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

Comment 6

[Mike Hommey [:glandium]]

Attachment: MozReview Request: Bug 1224450 - Skip difficult directories in the CompileDB. r=gps

There are a few difficult directories to handle, with limited usefulness
compared to having the CompileDB properly filled for everything else
in a timely manner, so skip them for now.

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

Comment 7

[Mike Hommey [:glandium]]

Attachment: MozReview Request: Bug 1224450 - Ignore COMPILE_PDB_FLAG for the CompileDB. r=gps

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

Comment 8

[Mike Hommey [:glandium]]

Attachment: MozReview Request: Bug 1224450 - Make the CompileDB derive its commands from the moz.build data. r=gps

The moz.build data is now sufficient to, with some convolution, generate
the same compilation database that recursing the tree with the showbuild
target does.

The resulting code is not the prettiest, and exposes the shortcomings of
the current moz.build data model. It is however a first step towards
fixing those shortcomings, because they are now more clearly identified.

This was validated on all platforms on try by checking the output of
  mach build-backend -b CompileDB -d -n
is empty when backing out the patch after running
  mach build-backend -b CompileDB
once.

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

Comment 9

[Gregory Szorc [:gps]]

Comment on attachment 8718092 [details]
MozReview Request: Bug 1224450 - Ignore host compilations in the CompileDB backend. r=gps

https://reviewboard.mozilla.org/r/34399/#review31131

Comment 10

[Gregory Szorc [:gps]]

Comment on attachment 8718093 [details]
MozReview Request: Bug 1224450 - Skip difficult directories in the CompileDB. r=gps

https://reviewboard.mozilla.org/r/34401/#review31133

Sure.

Comment 11

[Gregory Szorc [:gps]]

Comment on attachment 8718094 [details]
MozReview Request: Bug 1224450 - Ignore COMPILE_PDB_FLAG for the CompileDB. r=gps

https://reviewboard.mozilla.org/r/34403/#review31137

Comment 12

[Gregory Szorc [:gps]]

Comment on attachment 8718095 [details]
MozReview Request: Bug 1224450 - Make the CompileDB derive its commands from the moz.build data. r=gps

https://reviewboard.mozilla.org/r/34405/#review31143

Very, very nice.

It's really difficult to audit that all this behaves like all the logic in config.mk and rules.mk. However, verifying the output didn't change as a result of this patch is verification enough for me.

Comment 13

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/5160753dedb2
https://hg.mozilla.org/integration/mozilla-inbound/rev/bc6f6b34333e
https://hg.mozilla.org/integration/mozilla-inbound/rev/c7ff3274d147
https://hg.mozilla.org/integration/mozilla-inbound/rev/241e53cd1012

Comment 14

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/5160753dedb2
https://hg.mozilla.org/mozilla-central/rev/bc6f6b34333e
https://hg.mozilla.org/mozilla-central/rev/c7ff3274d147
https://hg.mozilla.org/mozilla-central/rev/241e53cd1012