Parallel compilation is broken with GNU Make 4.4

[mukilan]

GNU Make 4.4 breaks the parallel builds in mozjs and behaves as though -j1 was given, leading to long build times.

Relevant logs from cargo build -vv:

[mozjs_sys 0.68.2] make -f Makefile
[mozjs_sys 0.68.2] make[1]: Entering directory '/home/mukilan/dev/mozjs/target/debug/build/mozjs_sys-094547916dc1fb70/out/build'
[mozjs_sys 0.68.2] make[2]: warning: jobserver unavailable: using -j1.  Add '+' to parent make rule.
[mozjs_sys 0.68.2] make[2]: Entering directory '/home/mukilan/dev/mozjs/target/debug/build/mozjs_sys-094547916dc1fb70/out/build'
[mozjs_sys 0.68.2] make recurse_pre-export

Th release notes for GNU Make 4.4 and the linked bugs note that when using 'old-style' anonymous pipes for jobserver, the file descriptors are closed when parent make invokes a recursive sub-make when the sub-make call is not direct

* When the pipe-based jobserver is enabled and GNU Make decides it is invoking
  a non-make sub-process and closes the jobserver pipes, it will now add a new
  option to the MAKEFLAGS environment variable that disables the jobserver.
  This prevents sub-processes that invoke make from accidentally using other
  open file descriptors as jobserver pipes.  For more information see
  https://savannah.gnu.org/bugs/?57242 and https://savannah.gnu.org/bugs/?62397

SpiderMonkey's Makefile seem to implement recursive invocations that are not direct $(MAKE) invocations, so these sub-makes fail to open the closed jobserver pipes and fallback to -j1 mode.

This issue can be fixed by using a version of Cargo that has support for the new 'named fifo' based jobserver as GNU Make 4.4 introduced them to allow any process to integrate with the jobserver and thus should work with both direct and indirect sub-make invocations.

This affects Servo (on distros that ship GNU Make 4.4 such as Fedora 39, Arch, NixOS 23.05 etc) since it currently uses 2023-02-01 build of rust toolchain where the Cargo doesn't have support for named fifo jobserver.

[sagudev]

I think the best way would be to simply update servo's rust version. There is already PR for that: https://github.com/servo/servo/pull/29993

[mukilan]

Yep, I've called that out in the PR in servo to pin GNU Make :)

[delan]

It seems moving to Rust 1.74 was not enough to get make 4.4 working well :(

When building mozjs_sys as part of https://github.com/servo/servo/commit/355567186400d0531657d0eedda7625de9d0e93a with nixpkgs pinned to nixos-23.05 (https://github.com/NixOS/nixpkgs/commit/70bdadeb94ffc8806c0570eb5c2695ad29f0e421), the build script takes 634 seconds (cargo-timing-20240110T071341Z.html.gz).

When gnumake is downgraded from 4.4.1 to 4.3 (https://github.com/NixOS/nixpkgs/commit/6adf48f53d819a7b6e15672817fa1e78e5f4e84f), the build script takes only 71 seconds on the same 16-core 32-thread machine (cargo-timing-20240110T074057Z.html.gz).

diff --git a/etc/shell.nix b/etc/shell.nix
index 306b25acb9..81deee77fe 100644
--- a/etc/shell.nix
+++ b/etc/shell.nix
@@ -2,7 +2,7 @@
 # NOTE: This does not work offline or for nix-build

 with import (builtins.fetchTarball {
-  url = "https://github.com/NixOS/nixpkgs/archive/6adf48f53d819a7b6e15672817fa1e78e5f4e84f.tar.gz";
+  url = "https://github.com/NixOS/nixpkgs/archive/70bdadeb94ffc8806c0570eb5c2695ad29f0e421.tar.gz";
 }) {
   overlays = [
     (import (builtins.fetchTarball {
@@ -17,6 +17,9 @@ let
       cargo = rustToolchain;
       rustc = rustToolchain;
     };
+    pkgs_gnumake_4_3 = import (builtins.fetchTarball {
+      url = "https://github.com/NixOS/nixpkgs/archive/6adf48f53d819a7b6e15672817fa1e78e5f4e84f.tar.gz";
+    }) {};
 in
 clangStdenv.mkDerivation rec {
   name = "servo-env";
@@ -42,7 +45,7 @@ clangStdenv.mkDerivation rec {
     # functionality in mozjs and causes builds to be extremely
     # slow as it behaves as if -j1 was passed.
     # See https://github.com/servo/mozjs/issues/375
-    gnumake
+    pkgs_gnumake_4_3.gnumake

     # crown needs to be in our Cargo workspace so we can test it with `mach test`. This means its
     # dependency tree is listed in the main Cargo.lock, making it awkward to build with Nix because

$ nice ./mach build -d
$ rm -Rf target/debug/{build,deps,incremental}/mozjs*
$ find target/ -samefile target/debug/servo -print -delete
$ nice ./mach build -d

[sagudev]

Could this be problematic? https://github.com/servo/mozjs/blob/7184f658b7e2de8a8d288ff39001364513115782/mozjs-sys/makefile.cargo#L12

[mukilan]

I printed the value of CARGO_MAKEFLAGS when building mozjsys and it looks like it still uses pipes

  "-j --jobserver-fds=7,8 --jobserver-auth=7,8"

It looks like cargo for now only supports inheriting and forwarding named fifo arguments when invoked under make but new jobservers that a top-level cargo itself creates will still use pipes:

From https://docs.rs/jobserver/0.1.27/jobserver/

Starting from GNU make version 4.4, named pipe becomes the default way in communication on Unix. This crate also supports that feature in the sense of inheriting and forwarding the correct environment.

And https://github.com/rust-lang/jobserver-rs/pull/49

This commit makes sure that the new style --jobserver-auth=fifo:PATH can be forwarded to inherited processes correctly. The support of creating a new client with named pipe will come as follow-up pull request.

[delan]

Could this be problematic?

https://github.com/servo/mozjs/blob/7184f658b7e2de8a8d288ff39001364513115782/mozjs-sys/makefile.cargo#L12

Not familiar with the mozjs build system so maybe I’m doing it wrong, but I tried other values of --build-backends that I saw on searchfox and couldn’t get them working. With --build-backends=FasterMake:

  --- stderr
  WARNING: The value of LD is not used by this build system.
  Reticulating splines...
  Finished reading 62 moz.build files in 0.04s
  Read 0 gyp files in parallel contributing 0.00s to total wall time
  Processed into 252 build config descriptors in 0.04s
  FasterMake backend executed in 0.03s
    5 total backend files; 5 created; 0 updated; 0 unchanged; 0 deleted
  Total wall time: 0.15s; CPU time: 0.15s; Efficiency: 100%; Untracked: 0.03s
  make[1]: Makefile: No such file or directory
  make[1]: *** No rule to make target 'Makefile'.  Stop.
  make: *** [/home/delan/.cargo/git/checkouts/servo-mozjs-2ce5c9bd515ed398/bd22403/mozjs-sys/makefile.cargo:146: all] Error 2
  thread 'main' panicked at /home/delan/.cargo/git/checkouts/servo-mozjs-2ce5c9bd515ed398/bd22403/mozjs-sys/build.rs:247:5:
  assertion failed: result.success()
  note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

With --build-backends=FasterMake+RecursiveMake or --build-backends=FasterMake,RecursiveMake:

  --- stderr
  WARNING: The value of LD is not used by this build system.
  Reticulating splines...
  Finished reading 62 moz.build files in 0.04s
  Read 0 gyp files in parallel contributing 0.00s to total wall time
  Processed into 252 build config descriptors in 0.04s
  FasterMake+RecursiveMake backend executed in 0.04s
    177 total backend files; 174 created; 3 updated; 0 unchanged; 0 deleted
  Total wall time: 0.22s; CPU time: 0.22s; Efficiency: 100%; Untracked: 0.10s
  make[2]: warning: jobserver unavailable: using -j1.  Add '+' to parent make rule.
  make[5]: *** No rule to make target '../RegExp.o', needed by 'libjs_static.a'.  Stop.
  make[4]: *** [/home/delan/.cargo/git/checkouts/servo-mozjs-2ce5c9bd515ed398/52925fc/mozjs-sys/mozjs/config/faster/rules.mk:70: /cuffs/build/servo/target/debug/build/mozjs_sys-d3b03afeb0021c7c/out/build/js/src/build/libjs_static.a] Error 2
  make[3]: *** [Makefile:83: faster] Error 2
  make[2]: *** [/home/delan/.cargo/git/checkouts/servo-mozjs-2ce5c9bd515ed398/52925fc/mozjs-sys/mozjs/config/recurse.mk:34: pre-export] Error 2
  make[1]: *** [/home/delan/.cargo/git/checkouts/servo-mozjs-2ce5c9bd515ed398/52925fc/mozjs-sys/mozjs/config/rules.mk:361: default] Error 2
  make: *** [/home/delan/.cargo/git/checkouts/servo-mozjs-2ce5c9bd515ed398/52925fc/mozjs-sys/makefile.cargo:146: all] Error 2
  thread 'main' panicked at /home/delan/.cargo/git/checkouts/servo-mozjs-2ce5c9bd515ed398/52925fc/mozjs-sys/build.rs:247:5:
  assertion failed: result.success()
  note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

@mukilan, it looks like that followup PR hasn’t happened yet. Can the upstream Makefile be fixed, or can we run a fifo-based jobserver with jobslot?

[mukilan]

Can the upstream Makefile be fixed

I tried a couple of approaches to fix the makefile back when I was investigating this issue, but I didn't find any working solutions. Admittedly my knowledge of Make and SM build system is also limited, so in theory a fix to the Makefile might be still be possible.

Can we run a fifo-based jobserver with jobslot?

That's an interesting idea! I wasn't aware of jobslot. For the whole Servo build to use the same job server, we'd have to build a binary that runs the jobserver and spawns cargo with correct CARGO_MAKEFLAGS (during `./mach build). But since make already has the jobserver functionality, it might be simpler be to wrap the cargo invocation in a dummy GNU Make invocation in a way that forwards all arguments to cargo. But these approaches seem less than ideal to me as they add more moving parts to the build system.

I guess pushing for/contributing to upstream fix (either in SM or cargo adopting jobslot) and keeping make pinned via nix seems better, now that we have the ability to use nix on other distros with your recent PR!