Closed Bug 1645986 Opened 4 months ago Closed 3 months ago

Inefficiency when there are multiple GeneratedFile in a directory

Categories

(Firefox Build System :: General, task)

task

Tracking

(firefox79 fixed)

RESOLVED FIXED
mozilla79
Tracking Status
firefox79 --- fixed

People

(Reporter: glandium, Assigned: glandium)

References

(Blocks 1 open bug)

Details

Attachments

(4 files)

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.

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

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.

Assignee: nobody → mh+mozilla
Depends on: 1646653
Blocks: 1646939

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.

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).

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.

Pushed by mh@glandium.org:
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
Pushed by mh@glandium.org:
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
Flags: needinfo?(mh+mozilla)
Pushed by mh@glandium.org:
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
Regressions: 1648043
Regressions: 1648575
Flags: needinfo?(mh+mozilla)
You need to log in before you can comment on or make changes to this bug.