build: Workaround make jobserver bug#735
Conversation
This works around the following bug, where the invocation below fails with the pipe style and works fine with the new fifo jobserver. Since there is no sign of cargo going to support fifo jobservers, and single threaded compilation is horribly slow, we don't pass makeflags on, when using make 4.4. We use a crude version check, which checks the presense of the flag - since new make versions are rare, this should be fine for a while Minimum reproducer: `gmake -j2 --jobserver-style=pipe -f Makefile` Makefile: ```make define RECURSE +$(MAKE) -f sub.mk endef all: $(RECURSE) ``` sub.mk: ``` all: @: ``` Output: `gmake[1]: warning: jobserver unavailable: using -j1. Add '+' to parent make rule.` Signed-off-by: Jonathan Schwender <schwenderjonathan@gmail.com>
|
The downside is of course increased system load, since now you have two make jobservers that don't know about each other. It mostly shouldn't affect servo, since most builds will be pre-compiled. We could expose an environment variable to limit parallelism again for SM if people do run into memory / load issues. |
Using cargo clean and `htop` for crude verification of concurrency. Signed-off-by: Jonathan Schwender <schwenderjonathan@gmail.com>
jschwe
force-pushed
the
workaround_buggy_make
branch
from
7e9dd07 to
7689059
Compare
Signed-off-by: Jonathan Schwender <schwenderjonathan@gmail.com>
I think I'd prefer opening a new one, since the problem changes from "No parallism" to "potentially too much parallelism" |
sagudev
left a comment
sagudev
left a comment
There was a problem hiding this comment.
LGTM, but needs updated PR description to reflect last changes (because these gets into commit message on land)
|
Uf, could you also redo try run in servo (in github hosted runners - personal fork) to make sure we do not fail there (due too more jobs). |
https://github.com/jschwe/servo/actions/runs/24789316855/job/72542152621 - The windows MSRV build is failing, but thats expected / an existing issue because it isn't able to build mozjs from source (missing bootstrap). All other jobs are done, or past mozjs-sys, so looking good. |
Ugh I am stupid, CI uses old make so it will not really test this so this is not the problem at all (at least for now). |
|
Ah, I didn't think of that either. But perhaps by the time we update to ubuntu 26.04 in CI, cargo might have already added the new jobserver support. (we just recently updated to 24.04 after all). |
Companion PR to servo/mozjs#735. This bumps mozjs to the latest version 0.15.9. The changes were reviewed in the linked PR. Testing: This changes behavior when using `make` 4.4 and compiling mozjs from source in CI. This path is not exercised in CI, since Ubuntu 24.04 still ships make 4.3. Signed-off-by: Jonathan Schwender <schwenderjonathan@gmail.com>
3 participants
This works around the following bug, where the invocation below fails with the pipe style and works fine with the new fifo jobserver. Since there is no sign of cargo going to support fifo jobservers, and single threaded compilation is horribly slow, we don't pass makeflags on, when using make 4.4. We use a crude version check, which checks the presence of the flag - since new make versions are rare, this should be fine for a while.
If we detect the flag (i.e. make 4.4 or newer), we pass
-j <NUM_JOBS>to make instead of inheriting MAKEFLAGS and the jobserver from cargo. This means we get an upper bound of 2x NUM_JOBS jobs, since the cargo jobserver and makes jobserver coexist.We can remove this workaround once cargo supports fifo style jobservers.
Minimum reproducer:
gmake -j2 --jobserver-style=pipe -f MakefileMakefile:
sub.mk:
Output:
gmake[1]: warning: jobserver unavailable: using -j1. Add '+' to parent make rule.Testing: This is specific to make 4.4, and hence not covered by automated tests
Fixes: #375
Servo PR: servo/servo#44346