1645986 - Inefficiency when there are multiple GeneratedFile in a directory

Status: RESOLVED FIXED

(Firefox Build System :: General, task)

Description

[Mike Hommey [:glandium]]

The problem is that the file_generate commands are emitted sequentially in that directory. That makes things like e.g. security/ct/tests/gtest slower than it could be.
This is happening because the recursivemake backend expands them as (simplified):

tier:: target_file
target_file: deps
        command

The problem is that when there are multiple such targets, Make treats them sequentially.
Example:

$ cat > test.mk <<'EOF'
foo:: a
foo:: b
foo:: c
a b c:
        sleep 1
        echo $@
$ time make -f test.mk -j
sleep 1
echo a
a
sleep 1
echo b
b
sleep 1
echo c
c

real	0m3.006s
user	0m0.003s
sys	0m0.003s

Now, edit test.mk to replace the 3 foo:: lines with foo:: a b c:

$ time make -f test.mk foo -j
sleep 1
sleep 1
sleep 1
echo a
echo b
echo c
a
b
c

real	0m1.004s
user	0m0.006s
sys	0m0.001s

This is tricky to handle in the current code base, where each individual GeneratedFile is handled in make.MakeBackend._format_statements_for_generated_file, including the tier:: target_file part.

Comment 1

[Mike Hommey [:glandium]]

Note this might be Make version dependent, because ISTR it wasn't this way.

Comment 2

[Mike Hommey [:glandium]]

I have a patch but it ironically makes things slower because it makes these directories steal jobserver slots from things that take longer and thus now end up starting later.

Comment 3

[Mike Hommey [:glandium]]

Attachment: Bug 1645986 - Avoid relative paths in GeneratedFile targets.

Comment 4

[Mike Hommey [:glandium]]

Attachment: Bug 1645986 - Properly mark generated files required during compile as such.

Ideally, this kind of manual work wouldn't be required, but the frontend
doesn't provide enough information and while backend could correlate the
information, they currently don't.

Comment 5

[Mike Hommey [:glandium]]

Attachment: Bug 1645986 - Use the basename of the output file for const name in gen_cert_header.py.

While currently the path given to the script is relative and doesn't
contain a directory, but will soon, and in that case, we don't want
the directory to be part of the const name (that wouldn't even be a
valid identifier).

Comment 6

[Mike Hommey [:glandium]]

Attachment: Bug 1645986 - Move most GeneratedFile generation to top-level backend.mk.

While ideally we'd just move all of them at the top-level, there are
other things that depend on them that wouldn't then get the right
dependencies. One of them is install steps in dist/something, but there
are others, and that's a rather long pole of things requiring many
changes along with chances to break things.

So instead, we go with a simpler and more limited approach, where we
still recurse into directories to run their export tier (to run whatever
else than GeneratedFiles they have), but ensure the GeneratedFiles we
moved at the top-level are built before recursing by making
directory/export depend on them.

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/ce213e6bbc1f
Avoid relative paths in GeneratedFile targets. r=firefox-build-system-reviewers,rstewart
https://hg.mozilla.org/integration/autoland/rev/fdd319bdbfa7
Properly mark generated files required during compile as such. r=firefox-build-system-reviewers,rstewart
https://hg.mozilla.org/integration/autoland/rev/07b14b6a0b91
Use the basename of the output file for const name in gen_cert_header.py. r=firefox-build-system-reviewers,rstewart
https://hg.mozilla.org/integration/autoland/rev/ac871b2f34dc
Move most GeneratedFile generation to top-level backend.mk. r=firefox-build-system-reviewers,rstewart

Comment 8

[Dorel Luca [:dluca]]

Backed out 4 changesets (bug 1645986) for Build bustage

Log:
https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=307321698&repo=autoland&lineNumber=62048

Push with failures:
https://treeherder.mozilla.org/#/jobs?repo=autoland&group_state=expanded&selectedTaskRun=GBwzcC0cSse0_CAobfE5JA.0&revision=ac871b2f34dc4cd0a0faaf7e1bbb41ff7056526c

Backout:
https://hg.mozilla.org/integration/autoland/rev/adcd4d9c819d68ee4bb51b401db7ead7ead9bef3

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/faf6de9409f9
Avoid relative paths in GeneratedFile targets. r=firefox-build-system-reviewers,rstewart
https://hg.mozilla.org/integration/autoland/rev/fe1142397e72
Properly mark generated files required during compile as such. r=firefox-build-system-reviewers,rstewart
https://hg.mozilla.org/integration/autoland/rev/c43f9360b0a5
Use the basename of the output file for const name in gen_cert_header.py. r=firefox-build-system-reviewers,rstewart
https://hg.mozilla.org/integration/autoland/rev/602ea804234f
Move most GeneratedFile generation to top-level backend.mk. r=firefox-build-system-reviewers,rstewart

Comment 10

[Dorel Luca [:dluca]]

Backed out 4 changesets (bug 1645986) for Valgrid failure. CLOSED TREE

Log:
https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=307332270&repo=autoland&lineNumber=52047

Push with failures:
https://treeherder.mozilla.org/#/jobs?repo=autoland&group_state=expanded&revision=602ea804234ffcaeeed085865b12b39ca2b066ab

Backout:
https://hg.mozilla.org/integration/autoland/rev/1e1e4cd842b6aaeca7ae1b5bea743ca231bbc50b

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/c889623771d8
Avoid relative paths in GeneratedFile targets. r=firefox-build-system-reviewers,rstewart
https://hg.mozilla.org/integration/autoland/rev/f0214b179f69
Properly mark generated files required during compile as such. r=firefox-build-system-reviewers,rstewart
https://hg.mozilla.org/integration/autoland/rev/437d5c4859ac
Use the basename of the output file for const name in gen_cert_header.py. r=firefox-build-system-reviewers,rstewart
https://hg.mozilla.org/integration/autoland/rev/e858eb7ffeba
Move most GeneratedFile generation to top-level backend.mk. r=firefox-build-system-reviewers,rstewart

Comment 12

[Alexandru Michis [:malexandru]]

https://hg.mozilla.org/mozilla-central/rev/c889623771d8
https://hg.mozilla.org/mozilla-central/rev/f0214b179f69
https://hg.mozilla.org/mozilla-central/rev/437d5c4859ac
https://hg.mozilla.org/mozilla-central/rev/e858eb7ffeba