Inefficiency when there are multiple GeneratedFile in a directory
Categories
(Firefox Build System :: General, task)
Tracking
(firefox79 fixed)
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.
Assignee | ||
Comment 1•4 years ago
|
||
Note this might be Make version dependent, because ISTR it wasn't this way.
Assignee | ||
Comment 2•4 years ago
|
||
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 | ||
Comment 3•4 years ago
|
||
Assignee | ||
Comment 4•4 years ago
|
||
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.
Assignee | ||
Comment 5•4 years ago
|
||
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).
Assignee | ||
Comment 6•4 years ago
|
||
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 8•4 years ago
|
||
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
Assignee | ||
Updated•4 years ago
|
Comment 10•4 years ago
|
||
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•4 years ago
|
||
Comment 12•4 years ago
|
||
bugherder |
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
Assignee | ||
Updated•4 years ago
|
Description
•