[PATCH v2 0/5] Jobserver support

[PATCH v2 0/5] Jobserver support

[Martin Hundebøll]

The parallelism of bitbake easily uses every available core on the build
host. But since every task is run with the same number of parallel
threads/processes, multiple tasks might load the CPU excessively, which
in turn slows down the build due to scheduling overhead.

This patch series adds a class that creates a fifo filled with
PARALLEL_MAKE tokens. The path to the created fifo is then configured in
the MAKEFLAGS environment variable, which is read by make and a patched
ninja (and gcc if doing lto).

The benefits from using the jobserver depends on the set of executed
tasks: running multiple large do_compile tasks simultaneously benefits
more than multiple do_fetch tasks. A simple test building the following
tasks (and all their dependencies) yields a ~5% improvement in build
time (20:20 -> 19:20):

  nodejs-native
  rust-llvm-native
  rust-native
  linux-yocto
  qemu-native

On build machines shared by multiple users, a single jobserver can be
shared between multiple builds (using the JOBSERVER_FIFO variable).
Running the above build in two different build directories at the same
time gives a ~12% improvement (43:17 -> 37:55).

Finally, the memory pressure from e.g. compiling multiple c++ based
projects is also reduced. In our case, a cloud based build machine (with
32 cores and 32GB RAM) fails to compile llvm-rust-native (in parallel to
nodejs) without the jobserver due to a lack of memory.

This patch set is roughly based on previous work by Richard[1]. That
patch lists three TODO items, which are all addressed by these patches:
 * The fifo path defaults to TMPDIR/jobserver_fifo, but can be
   configured using JOBSERVER_FIFO.
 * The number of make threads defaults to the value from PARALLEL_MAKE
   (which is then redundant).
 * If PARALLEL_MAKE is unset, the jobserver functionality is skipped.

Further work in addition to this patch set could be to make bitbake
tasks jobserver aware.

Changes since v1[2]:
 * Fixed typos in various places
 * Added JOBSERVER_IGNORE variable to jobserver.bbclass in patch 1
 * Changed ninja to use kitware fork with only two patches added in
   patch 3
 * Updated upstream patch submission link in qemu in patch 4

Changes since the RFC[3]:
 * The ninja src uri change in patch 3 is converted to a set of patches
 * The qemu fix in patch 4 is converted to a submitted patch

[1] https://lore.kernel.org/openembedded-core/1423223184.20217.15.camel@linuxfoundation.org/
[2] https://lore.kernel.org/openembedded-core/20240403070204.367470-1-martin@geanix.com/
[3] https://lore.kernel.org/openembedded-core/20230828124834.376779-1-martin@geanix.com/

Martin Hundebøll (5):
  classes: jobserver: support gnu make fifo jobserver
  scripts: build-env: allow passing JOBSERVER_FIFO from environment
  ninja: build modified version with GNU Make jobserver support
  qemu: enable parallel builds when using the jobserver class
  contrib: add python service and systemd unit to run shared jobserver

 contrib/jobserver/jobserver.py                |  78 +++++
 contrib/jobserver/jobserver.service           |  10 +
 meta/classes-global/jobserver.bbclass         |  87 ++++++
 meta/conf/bitbake.conf                        |   2 +-
 ...ename-TokenPool-Setup-to-SetupClient.patch | 113 ++++++++
 ...-jobserver-fifo-style-client-support.patch | 271 ++++++++++++++++++
 meta/recipes-devtools/ninja/ninja_1.11.1.bb   |   8 +-
 meta/recipes-devtools/qemu/qemu.inc           |   1 +
 ...e-jobserver-auth-argument-when-calli.patch |  37 +++
 scripts/oe-buildenv-internal                  |   2 +-
 10 files changed, 605 insertions(+), 4 deletions(-)
 create mode 100644 contrib/jobserver/jobserver.py
 create mode 100644 contrib/jobserver/jobserver.service
 create mode 100644 meta/classes-global/jobserver.bbclass
 create mode 100644 meta/recipes-devtools/ninja/files/0001-Rename-TokenPool-Setup-to-SetupClient.patch
 create mode 100644 meta/recipes-devtools/ninja/files/0002-Add-GNU-make-jobserver-fifo-style-client-support.patch
 create mode 100644 meta/recipes-devtools/qemu/qemu/0013-Makefile-preserve-jobserver-auth-argument-when-calli.patch

-- 
2.44.0

[PATCH v2 1/5] classes: jobserver: support gnu make fifo jobserver

[Martin Hundebøll]

Add a class to implement the gnu make fifo style jobserver. The class
can be activated by simply adding an `INHERIT += "jobserver"` to the
local configuration.

Furthermore, one can configure an external jobserver (i.e. a server
shared between multiple builds), by configuring the `JOBSERVER_FIFO`
variable to point at an existing jobserver fifo.

The jobserver class uses the fifo style jobserver, which doesn't require
passing open file descriptors around. It does, however, require
make-4.4, which isn't available in common distro yet. To work around
this, the class makes all recipes (except make and its dependencies
itself) depend on `virtual/make-native`.

Signed-off-by: Martin Hundebøll <martin@geanix.com>
---
 meta/classes-global/jobserver.bbclass | 87 +++++++++++++++++++++++++++
 meta/conf/bitbake.conf                |  2 +-
 2 files changed, 88 insertions(+), 1 deletion(-)
 create mode 100644 meta/classes-global/jobserver.bbclass

diff --git a/meta/classes-global/jobserver.bbclass b/meta/classes-global/jobserver.bbclass
new file mode 100644
index 0000000000..3866b473e6
--- /dev/null
+++ b/meta/classes-global/jobserver.bbclass
@@ -0,0 +1,87 @@
+JOBSERVER_FIFO ?= ""
+JOBSERVER_FIFO[doc] = "Path to external jobserver fifo to use instead of creating a per-build server."
+
+JOBSERVER_IGNORE ?= ""
+JOBSERVER_IGNORE[doc] = "Space separated list of packages that shouldn't be configured to use the jobserver feature."
+
+addhandler jobserver_setup_fifo
+jobserver_setup_fifo[eventmask] = "bb.event.ConfigParsed"
+
+python jobserver_setup_fifo() {
+    # don't setup a per-build fifo, if an external one is configured
+    if d.getVar("JOBSERVER_FIFO"):
+        return
+
+    # don't use a job-server if no parallelism is configured
+    jobs = oe.utils.parallel_make(d)
+    if jobs in (None, 1):
+        return
+
+    # reduce jobs by one as a token has implicitly been handed to the
+    # process requesting tokens
+    jobs -= 1
+
+    fifo = d.getVar("TMPDIR") + "/jobserver_fifo"
+
+    # an old fifo might be lingering; remove it
+    if os.path.exists(fifo):
+        os.remove(fifo)
+
+    # create a new fifo to use for communicating tokens
+    os.mkfifo(fifo)
+
+    # fill the fifo with the number of tokens to hand out
+    wfd = os.open(fifo, os.O_RDWR)
+    written = os.write(wfd, b"+" * jobs)
+    if written != (jobs):
+        bb.error("Failed to fil make fifo: {} != {}".format(written, jobs))
+
+    # configure the per-build fifo path to use
+    d.setVar("JOBSERVER_FIFO", fifo)
+}
+
+python () {
+    # don't configure the fifo if none is defined
+    fifo = d.getVar("JOBSERVER_FIFO")
+    if not fifo:
+        return
+
+    # don't configure the fifo if the package wants to ignore it
+    if d.getVar("PN") in (d.getVar("JOBSERVER_IGNORE") or "").split():
+        return
+
+    # avoid making make-native or its dependencies depend on make-native itself
+    if d.getVar("PN") in (
+                "make-native",
+                "libtool-native",
+                "pkgconfig-native",
+                "automake-native",
+                "autoconf-native",
+                "m4-native",
+                "texinfo-dummy-native",
+                "gettext-minimal-native",
+                "quilt-native",
+                "gnu-config-native",
+            ):
+        return
+
+    # don't make unwilling recipes depend on make-native
+    if d.getVar('INHIBIT_DEFAULT_DEPS', False):
+        return
+
+    # make other recipes depend on make-native to make sure it is new enough to
+    # support the --jobserver-auth=fifo:<path> syntax (from make-4.4 and onwards)
+    d.appendVar("DEPENDS", " virtual/make-native")
+
+    # disable the "-j <jobs>" flag, as that overrides the jobserver fifo tokens
+    d.setVar("PARALLEL_MAKE", "")
+    d.setVar("PARALLEL_MAKEINST", "")
+
+    # set and export the jobserver in the environment
+    d.appendVar("MAKEFLAGS", " --jobserver-auth=fifo:" + fifo)
+    d.setVarFlag("MAKEFLAGS", "export", "1")
+
+    # ignore the jobserver argument part of MAKEFLAGS in the hash, as that
+    # shouldn't change the build output
+    d.appendVarFlag("MAKEFLAGS", "vardepvalueexclude", "| --jobserver-auth=fifo:" + fifo)
+}
diff --git a/meta/conf/bitbake.conf b/meta/conf/bitbake.conf
index 6f180d18b0..23a016b31e 100644
--- a/meta/conf/bitbake.conf
+++ b/meta/conf/bitbake.conf
@@ -960,7 +960,7 @@ BB_HASHEXCLUDE_COMMON ?= "TMPDIR FILE PATH PWD BB_TASKHASH BBPATH BBSERVER DL_DI
     BB_WORKERCONTEXT BB_LIMITEDDEPS BB_UNIHASH extend_recipe_sysroot DEPLOY_DIR \
     SSTATE_HASHEQUIV_METHOD SSTATE_HASHEQUIV_REPORT_TASKDATA \
     SSTATE_HASHEQUIV_OWNER CCACHE_TOP_DIR BB_HASHSERVE GIT_CEILING_DIRECTORIES \
-    OMP_NUM_THREADS BB_CURRENTTASK"
+    OMP_NUM_THREADS BB_CURRENTTASK JOBSERVER_FIFO"
 BB_BASEHASH_IGNORE_VARS ?= "${BB_HASHEXCLUDE_COMMON} PSEUDO_IGNORE_PATHS BUILDHISTORY_DIR \
     SSTATE_DIR SOURCE_DATE_EPOCH RUST_BUILD_SYS RUST_HOST_SYS RUST_TARGET_SYS"
 BB_HASHCONFIG_IGNORE_VARS ?= "${BB_HASHEXCLUDE_COMMON} DATE TIME SSH_AGENT_PID \
-- 
2.44.0

[PATCH v2 2/5] scripts: build-env: allow passing JOBSERVER_FIFO from environment

[Martin Hundebøll]

Sharing a common jobserver fifo between multiple (containerized) builds
is much easier, if an administrator can configure said jobserver fifo
path in the environment. Append the JOBSERVER_FIFO variable name to the
list of variables configurable through the environment.

Signed-off-by: Martin Hundebøll <martin@geanix.com>
---
 scripts/oe-buildenv-internal | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/scripts/oe-buildenv-internal b/scripts/oe-buildenv-internal
index 2fdb19565a..c8e67ffb8f 100755
--- a/scripts/oe-buildenv-internal
+++ b/scripts/oe-buildenv-internal
@@ -112,7 +112,7 @@ HTTPS_PROXY https_proxy FTP_PROXY ftp_proxy FTPS_PROXY ftps_proxy ALL_PROXY \
 all_proxy NO_PROXY no_proxy SSH_AGENT_PID SSH_AUTH_SOCK BB_SRCREV_POLICY \
 SDKMACHINE BB_NUMBER_THREADS BB_NO_NETWORK PARALLEL_MAKE GIT_PROXY_COMMAND \
 SOCKS5_PASSWD SOCKS5_USER SCREENDIR STAMPS_DIR BBPATH_EXTRA BB_SETSCENE_ENFORCE \
-BB_LOGCONFIG"
+BB_LOGCONFIG JOBSERVER_FIFO"
 
 BB_ENV_PASSTHROUGH_ADDITIONS="$(echo $BB_ENV_PASSTHROUGH_ADDITIONS $BB_ENV_PASSTHROUGH_ADDITIONS_OE | tr ' ' '\n' | LC_ALL=C sort --unique | tr '\n' ' ')"
 
-- 
2.44.0

[PATCH v2 3/5] ninja: build modified version with GNU Make jobserver support

[Martin Hundebøll]

Ninja doesn't (yet) support the GNU Make jobserver out of the box, but
there is a pull request adding that support[1]. Since that pull request
(and its derived three-part pull requests) seem to be ignored by
upstream, kitware (creator/maintainer of cmake) has created a fork[2]
only to carry the jobserver patches. Change the source uri to point at
the kitware fork of ninja, and add two patches from the original pull
request to also support the new-style fifo jobserver feature.

Note that the kitware fork of ninja is also used by buildroot[3].

[1] https://github.com/ninja-build/ninja/pull/2263
[2] https://github.com/Kitware/ninja
[3] https://gitlab.com/buildroot.org/buildroot/-/blob/master/package/ninja/ninja.mk

Signed-off-by: Martin Hundebøll <martin@geanix.com>
---
 ...ename-TokenPool-Setup-to-SetupClient.patch | 113 ++++++++
 ...-jobserver-fifo-style-client-support.patch | 271 ++++++++++++++++++
 meta/recipes-devtools/ninja/ninja_1.11.1.bb   |   8 +-
 3 files changed, 390 insertions(+), 2 deletions(-)
 create mode 100644 meta/recipes-devtools/ninja/files/0001-Rename-TokenPool-Setup-to-SetupClient.patch
 create mode 100644 meta/recipes-devtools/ninja/files/0002-Add-GNU-make-jobserver-fifo-style-client-support.patch

diff --git a/meta/recipes-devtools/ninja/files/0001-Rename-TokenPool-Setup-to-SetupClient.patch b/meta/recipes-devtools/ninja/files/0001-Rename-TokenPool-Setup-to-SetupClient.patch
new file mode 100644
index 0000000000..a503f8c75f
--- /dev/null
+++ b/meta/recipes-devtools/ninja/files/0001-Rename-TokenPool-Setup-to-SetupClient.patch
@@ -0,0 +1,113 @@
+From f5642d8b49688dfc84679451b531d92f3b6e7cb0 Mon Sep 17 00:00:00 2001
+From: Stefan Becker <chemobejk@gmail.com>
+Date: Sat, 15 Dec 2018 19:29:42 +0200
+Subject: [PATCH 1/2] Rename TokenPool::Setup() to SetupClient()
+MIME-Version: 1.0
+Content-Type: text/plain; charset=UTF-8
+Content-Transfer-Encoding: 8bit
+
+Make space to add new API to set up token pool master.
+
+Signed-off-by: Martin Hundebøll <martin@geanix.com>
+Upstream-Status: Submitted [https://github.com/Kitware/ninja/pull/2]
+---
+ src/build.cc              | 6 +++---
+ src/subprocess_test.cc    | 3 ++-
+ src/tokenpool-gnu-make.cc | 6 +++---
+ src/tokenpool-gnu-make.h  | 3 ++-
+ src/tokenpool.h           | 3 ++-
+ src/tokenpool_test.cc     | 2 +-
+ 6 files changed, 13 insertions(+), 10 deletions(-)
+
+diff --git a/src/build.cc b/src/build.cc
+index 53e4405..d8a6dff 100644
+--- a/src/build.cc
++++ b/src/build.cc
+@@ -474,9 +474,9 @@ struct RealCommandRunner : public CommandRunner {
+ RealCommandRunner::RealCommandRunner(const BuildConfig& config) : config_(config) {
+   max_load_average_ = config.max_load_average;
+   if ((tokens_ = TokenPool::Get()) != NULL) {
+-    if (!tokens_->Setup(config_.parallelism_from_cmdline,
+-                        config_.verbosity == BuildConfig::VERBOSE,
+-                        max_load_average_)) {
++    if (!tokens_->SetupClient(config_.parallelism_from_cmdline,
++                              config_.verbosity == BuildConfig::VERBOSE,
++                              max_load_average_)) {
+       delete tokens_;
+       tokens_ = NULL;
+     }
+diff --git a/src/subprocess_test.cc b/src/subprocess_test.cc
+index eddc110..222b59b 100644
+--- a/src/subprocess_test.cc
++++ b/src/subprocess_test.cc
+@@ -40,7 +40,8 @@ struct TestTokenPool : public TokenPool {
+   void Reserve()     {}
+   void Release()     {}
+   void Clear()       {}
+-  bool Setup(bool ignore_unused, bool verbose, double& max_load_average) { return false; }
++  bool SetupClient(bool ignore_unused, bool verbose,
++                   double& max_load_average) { return false; }
+ 
+ #ifdef _WIN32
+   bool _token_available;
+diff --git a/src/tokenpool-gnu-make.cc b/src/tokenpool-gnu-make.cc
+index 60e0552..6fb72a6 100644
+--- a/src/tokenpool-gnu-make.cc
++++ b/src/tokenpool-gnu-make.cc
+@@ -28,9 +28,9 @@ GNUmakeTokenPool::GNUmakeTokenPool() : available_(1), used_(0) {
+ GNUmakeTokenPool::~GNUmakeTokenPool() {
+ }
+ 
+-bool GNUmakeTokenPool::Setup(bool ignore,
+-                             bool verbose,
+-                             double& max_load_average) {
++bool GNUmakeTokenPool::SetupClient(bool ignore,
++                                   bool verbose,
++                                   double& max_load_average) {
+   const char* value = GetEnv("MAKEFLAGS");
+   if (!value)
+     return false;
+diff --git a/src/tokenpool-gnu-make.h b/src/tokenpool-gnu-make.h
+index c94cca5..f4ab8d7 100644
+--- a/src/tokenpool-gnu-make.h
++++ b/src/tokenpool-gnu-make.h
+@@ -24,7 +24,8 @@ struct GNUmakeTokenPool : public TokenPool {
+   virtual void Reserve();
+   virtual void Release();
+   virtual void Clear();
+-  virtual bool Setup(bool ignore, bool verbose, double& max_load_average);
++  virtual bool SetupClient(bool ignore, bool verbose,
++                           double& max_load_average);
+ 
+   // platform specific implementation
+   virtual const char* GetEnv(const char* name) = 0;
+diff --git a/src/tokenpool.h b/src/tokenpool.h
+index 931c227..ce2bf48 100644
+--- a/src/tokenpool.h
++++ b/src/tokenpool.h
+@@ -26,7 +26,8 @@ struct TokenPool {
+   virtual void Clear() = 0;
+ 
+   // returns false if token pool setup failed
+-  virtual bool Setup(bool ignore, bool verbose, double& max_load_average) = 0;
++  virtual bool SetupClient(bool ignore, bool verbose,
++                           double& max_load_average) = 0;
+ 
+ #ifdef _WIN32
+   virtual void WaitForTokenAvailability(HANDLE ioport) = 0;
+diff --git a/src/tokenpool_test.cc b/src/tokenpool_test.cc
+index 1bc1c84..9d07146 100644
+--- a/src/tokenpool_test.cc
++++ b/src/tokenpool_test.cc
+@@ -77,7 +77,7 @@ struct TokenPoolTest : public testing::Test {
+       ENVIRONMENT_INIT(buf_);
+     }
+     if ((tokens_ = TokenPool::Get()) != NULL) {
+-      if (!tokens_->Setup(ignore_jobserver, false, load_avg_)) {
++      if (!tokens_->SetupClient(ignore_jobserver, false, load_avg_)) {
+         delete tokens_;
+         tokens_ = NULL;
+       }
+-- 
+2.44.0
+
diff --git a/meta/recipes-devtools/ninja/files/0002-Add-GNU-make-jobserver-fifo-style-client-support.patch b/meta/recipes-devtools/ninja/files/0002-Add-GNU-make-jobserver-fifo-style-client-support.patch
new file mode 100644
index 0000000000..770a53fbc9
--- /dev/null
+++ b/meta/recipes-devtools/ninja/files/0002-Add-GNU-make-jobserver-fifo-style-client-support.patch
@@ -0,0 +1,271 @@
+From 830b7fb396383171c5fac2aae7de880ac83ce955 Mon Sep 17 00:00:00 2001
+From: Stefan Becker <chemobejk@gmail.com>
+Date: Mon, 7 Nov 2022 20:45:27 +0200
+Subject: [PATCH 2/2] Add GNU make jobserver fifo style client support
+MIME-Version: 1.0
+Content-Type: text/plain; charset=UTF-8
+Content-Transfer-Encoding: 8bit
+
+GNU make 4.4 introduced a new jobserver style "fifo" for POSIX systems
+which passes a named pipe down to the clients.
+
+- update auth parser to recognize "fifo:<name>" format
+- open named pipe for reading and writing
+- make sure the file descriptors are closed in the destructor
+- add 2 tests that aren't compiled for WIN32
+
+Signed-off-by: Martin Hundebøll <martin@geanix.com>
+Upstream-Status: Submitted [https://github.com/Kitware/ninja/pull/2]
+---
+ src/tokenpool-gnu-make-posix.cc | 46 ++++++++++++++++-
+ src/tokenpool_test.cc           | 88 ++++++++++++++++++++++++++++++---
+ 2 files changed, 127 insertions(+), 7 deletions(-)
+
+diff --git a/src/tokenpool-gnu-make-posix.cc b/src/tokenpool-gnu-make-posix.cc
+index 8447070..a82cce4 100644
+--- a/src/tokenpool-gnu-make-posix.cc
++++ b/src/tokenpool-gnu-make-posix.cc
+@@ -40,6 +40,7 @@ struct GNUmakeTokenPoolPosix : public GNUmakeTokenPool {
+  private:
+   int rfd_;
+   int wfd_;
++  bool closeFds_;
+ 
+   struct sigaction old_act_;
+   bool restore_;
+@@ -48,14 +49,19 @@ struct GNUmakeTokenPoolPosix : public GNUmakeTokenPool {
+   static void CloseDupRfd(int signum);
+ 
+   bool CheckFd(int fd);
++  bool CheckFifo(const char* fifo);
+   bool SetAlarmHandler();
+ };
+ 
+-GNUmakeTokenPoolPosix::GNUmakeTokenPoolPosix() : rfd_(-1), wfd_(-1), restore_(false) {
++GNUmakeTokenPoolPosix::GNUmakeTokenPoolPosix() : rfd_(-1), wfd_(-1), closeFds_(false), restore_(false) {
+ }
+ 
+ GNUmakeTokenPoolPosix::~GNUmakeTokenPoolPosix() {
+   Clear();
++  if (closeFds_) {
++    close(wfd_);
++    close(rfd_);
++  }
+   if (restore_)
+     sigaction(SIGALRM, &old_act_, NULL);
+ }
+@@ -69,6 +75,36 @@ bool GNUmakeTokenPoolPosix::CheckFd(int fd) {
+   return true;
+ }
+ 
++bool GNUmakeTokenPoolPosix::CheckFifo(const char* fifo) {
++  // remove possible junk from end of fifo name
++  char *filename = strdup(fifo);
++  char *end;
++  if ((end = strchr(filename, ' ')) != NULL) {
++    *end = '\0';
++  }
++
++  int rfd = open(filename, O_RDONLY);
++  if (rfd < 0) {
++    free(filename);
++    return false;
++  }
++
++  int wfd = open(filename, O_WRONLY);
++  if (wfd < 0) {
++    close(rfd);
++    free(filename);
++    return false;
++  }
++
++  free(filename);
++
++  rfd_ = rfd;
++  wfd_ = wfd;
++  closeFds_ = true;
++
++  return true;
++}
++
+ int GNUmakeTokenPoolPosix::dup_rfd_ = -1;
+ 
+ void GNUmakeTokenPoolPosix::CloseDupRfd(int signum) {
+@@ -89,6 +125,13 @@ bool GNUmakeTokenPoolPosix::SetAlarmHandler() {
+ }
+ 
+ bool GNUmakeTokenPoolPosix::ParseAuth(const char* jobserver) {
++  // check for jobserver-fifo style
++  const char* fifo;
++  if (((fifo = strstr(jobserver, "=fifo:")) != NULL) &&
++      CheckFifo(fifo + 6))
++    return SetAlarmHandler();
++
++  // check for legacy simple pipe style
+   int rfd = -1;
+   int wfd = -1;
+   if ((sscanf(jobserver, "%*[^=]=%d,%d", &rfd, &wfd) == 2) &&
+@@ -100,6 +143,7 @@ bool GNUmakeTokenPoolPosix::ParseAuth(const char* jobserver) {
+     return true;
+   }
+ 
++  // some jobserver style we don't support
+   return false;
+ }
+ 
+diff --git a/src/tokenpool_test.cc b/src/tokenpool_test.cc
+index 9d07146..e15da6c 100644
+--- a/src/tokenpool_test.cc
++++ b/src/tokenpool_test.cc
+@@ -19,7 +19,10 @@
+ #ifdef _WIN32
+ #include <windows.h>
+ #else
++#include <fcntl.h>
+ #include <unistd.h>
++#include <sys/types.h>
++#include <sys/stat.h>
+ #endif
+ 
+ #include <stdio.h>
+@@ -35,6 +38,8 @@
+ #define AUTH_FORMAT(tmpl)   "foo " tmpl "=%d,%d bar"
+ #define ENVIRONMENT_CLEAR() unsetenv("MAKEFLAGS")
+ #define ENVIRONMENT_INIT(v) setenv("MAKEFLAGS", v, true)
++
++#define FIFO_NAME "ninja-test-tokenpool-fifo"
+ #endif
+ 
+ namespace {
+@@ -60,11 +65,24 @@ struct TokenPoolTest : public testing::Test {
+     semaphore_name_ = SEMAPHORE_NAME;
+     if ((semaphore_ = CreateSemaphore(0, 0, 2, SEMAPHORE_NAME)) == NULL)
+ #else
++    if (mkfifo(FIFO_NAME, 0600) < 0) {
++      ASSERT_TRUE(false);
++    }
++
+     if (pipe(fds_) < 0)
+ #endif
+       ASSERT_TRUE(false);
+   }
+ 
++  void GetPool(bool ignore_jobserver) {
++    if ((tokens_ = TokenPool::Get()) != NULL) {
++      if (!tokens_->SetupClient(ignore_jobserver, false, load_avg_)) {
++        delete tokens_;
++        tokens_ = NULL;
++      }
++    }
++  }
++
+   void CreatePool(const char* format, bool ignore_jobserver = false) {
+     if (format) {
+       sprintf(buf_, format,
+@@ -76,18 +94,30 @@ struct TokenPoolTest : public testing::Test {
+       );
+       ENVIRONMENT_INIT(buf_);
+     }
+-    if ((tokens_ = TokenPool::Get()) != NULL) {
+-      if (!tokens_->SetupClient(ignore_jobserver, false, load_avg_)) {
+-        delete tokens_;
+-        tokens_ = NULL;
+-      }
+-    }
++    GetPool(ignore_jobserver);
+   }
+ 
+   void CreateDefaultPool() {
+     CreatePool(AUTH_FORMAT("--jobserver-auth"));
+   }
+ 
++#ifndef _WIN32
++  void CreateFifoPool() {
++    // close simple pipe fds...
++    close(fds_[0]);
++    close(fds_[1]);
++
++    // ... and open named pipe instead
++    if ((fds_[0] = open(FIFO_NAME, O_RDONLY|O_NONBLOCK)) < 0)
++      ASSERT_TRUE(false);
++    if ((fds_[1] = open(FIFO_NAME, O_WRONLY)) < 0)
++      ASSERT_TRUE(false);
++
++    ENVIRONMENT_INIT("foo --jobserver-auth=fifo:" FIFO_NAME " bar");
++    GetPool(false);
++  }
++#endif
++
+   virtual void TearDown() {
+     if (tokens_)
+       delete tokens_;
+@@ -96,6 +126,7 @@ struct TokenPoolTest : public testing::Test {
+ #else
+     close(fds_[0]);
+     close(fds_[1]);
++    unlink(FIFO_NAME);
+ #endif
+     ENVIRONMENT_CLEAR();
+   }
+@@ -167,6 +198,15 @@ TEST_F(TokenPoolTest, MonitorFD) {
+ 
+   EXPECT_EQ(fds_[0], tokens_->GetMonitorFd());
+ }
++
++TEST_F(TokenPoolTest, MonitorFDFifo) {
++  CreateFifoPool();
++
++  ASSERT_NE(NULL, tokens_);
++  EXPECT_EQ(kLoadAverageDefault, load_avg_);
++
++  EXPECT_NE(-1, tokens_->GetMonitorFd());
++}
+ #endif
+ 
+ TEST_F(TokenPoolTest, ImplicitToken) {
+@@ -226,6 +266,42 @@ TEST_F(TokenPoolTest, TwoTokens) {
+   EXPECT_TRUE(tokens_->Acquire());
+ }
+ 
++#ifndef _WIN32
++TEST_F(TokenPoolTest, TwoTokensFifo) {
++  CreateFifoPool();
++
++  ASSERT_NE(NULL, tokens_);
++  EXPECT_EQ(kLoadAverageDefault, load_avg_);
++
++  // implicit token
++  EXPECT_TRUE(tokens_->Acquire());
++  tokens_->Reserve();
++  EXPECT_FALSE(tokens_->Acquire());
++
++  // jobserver offers 2nd token
++  char test_tokens[1] = { '+' };
++  ASSERT_EQ(1u, write(fds_[1], test_tokens, sizeof(test_tokens)));
++  EXPECT_TRUE(tokens_->Acquire());
++  tokens_->Reserve();
++  EXPECT_FALSE(tokens_->Acquire());
++
++  // release 2nd token
++  tokens_->Release();
++  EXPECT_TRUE(tokens_->Acquire());
++
++  // release implicit token - must return 2nd token back to jobserver
++  tokens_->Release();
++  EXPECT_TRUE(tokens_->Acquire());
++
++  // there must be one token available
++  EXPECT_EQ(1u, read(fds_[0], buf_, sizeof(buf_)));
++  EXPECT_EQ(test_tokens[0], buf_[0]);
++
++  // implicit token
++  EXPECT_TRUE(tokens_->Acquire());
++}
++#endif
++
+ TEST_F(TokenPoolTest, Clear) {
+   CreateDefaultPool();
+ 
+-- 
+2.44.0
+
diff --git a/meta/recipes-devtools/ninja/ninja_1.11.1.bb b/meta/recipes-devtools/ninja/ninja_1.11.1.bb
index 8e297ec4d4..dbff2e191a 100644
--- a/meta/recipes-devtools/ninja/ninja_1.11.1.bb
+++ b/meta/recipes-devtools/ninja/ninja_1.11.1.bb
@@ -6,9 +6,13 @@ LIC_FILES_CHKSUM = "file://COPYING;md5=a81586a64ad4e476c791cda7e2f2c52e"
 
 DEPENDS = "re2c-native ninja-native"
 
-SRCREV = "a524bf3f6bacd1b4ad85d719eed2737d8562f27a"
+SRCREV = "95dee2a91d96c409d54f9fa0b70ea9aa2bdf8e63"
 
-SRC_URI = "git://github.com/ninja-build/ninja.git;branch=release;protocol=https"
+SRC_URI = " \
+    git://github.com/kitware/ninja.git;branch=kitware-staged-features;protocol=https \
+    file://0001-Rename-TokenPool-Setup-to-SetupClient.patch \
+    file://0002-Add-GNU-make-jobserver-fifo-style-client-support.patch \
+"
 UPSTREAM_CHECK_GITTAGREGEX = "v(?P<pver>.*)"
 
 S = "${WORKDIR}/git"
-- 
2.44.0

Re: [OE-core] [PATCH v2 3/5] ninja: build modified version with GNU Make jobserver support

[Alexandre Belloni]

Hello,

This patch doesn't apply on master anymore because ninja has been updated.


On 04/04/2024 13:16:11+0200, Martin Hundeb?ll wrote:
> Ninja doesn't (yet) support the GNU Make jobserver out of the box, but
> there is a pull request adding that support[1]. Since that pull request
> (and its derived three-part pull requests) seem to be ignored by
> upstream, kitware (creator/maintainer of cmake) has created a fork[2]
> only to carry the jobserver patches. Change the source uri to point at
> the kitware fork of ninja, and add two patches from the original pull
> request to also support the new-style fifo jobserver feature.
> 
> Note that the kitware fork of ninja is also used by buildroot[3].
> 
> [1] https://github.com/ninja-build/ninja/pull/2263
> [2] https://github.com/Kitware/ninja
> [3] https://gitlab.com/buildroot.org/buildroot/-/blob/master/package/ninja/ninja.mk
> 
> Signed-off-by: Martin Hundeb�ll <martin@geanix.com>
> ---
>  ...ename-TokenPool-Setup-to-SetupClient.patch | 113 ++++++++
>  ...-jobserver-fifo-style-client-support.patch | 271 ++++++++++++++++++
>  meta/recipes-devtools/ninja/ninja_1.11.1.bb   |   8 +-
>  3 files changed, 390 insertions(+), 2 deletions(-)
>  create mode 100644 meta/recipes-devtools/ninja/files/0001-Rename-TokenPool-Setup-to-SetupClient.patch
>  create mode 100644 meta/recipes-devtools/ninja/files/0002-Add-GNU-make-jobserver-fifo-style-client-support.patch
> 
> diff --git a/meta/recipes-devtools/ninja/files/0001-Rename-TokenPool-Setup-to-SetupClient.patch b/meta/recipes-devtools/ninja/files/0001-Rename-TokenPool-Setup-to-SetupClient.patch
> new file mode 100644
> index 0000000000..a503f8c75f
> --- /dev/null
> +++ b/meta/recipes-devtools/ninja/files/0001-Rename-TokenPool-Setup-to-SetupClient.patch
> @@ -0,0 +1,113 @@
> +From f5642d8b49688dfc84679451b531d92f3b6e7cb0 Mon Sep 17 00:00:00 2001
> +From: Stefan Becker <chemobejk@gmail.com>
> +Date: Sat, 15 Dec 2018 19:29:42 +0200
> +Subject: [PATCH 1/2] Rename TokenPool::Setup() to SetupClient()
> +MIME-Version: 1.0
> +Content-Type: text/plain; charset=UTF-8
> +Content-Transfer-Encoding: 8bit
> +
> +Make space to add new API to set up token pool master.
> +
> +Signed-off-by: Martin Hundeb�ll <martin@geanix.com>
> +Upstream-Status: Submitted [https://github.com/Kitware/ninja/pull/2]
> +---
> + src/build.cc              | 6 +++---
> + src/subprocess_test.cc    | 3 ++-
> + src/tokenpool-gnu-make.cc | 6 +++---
> + src/tokenpool-gnu-make.h  | 3 ++-
> + src/tokenpool.h           | 3 ++-
> + src/tokenpool_test.cc     | 2 +-
> + 6 files changed, 13 insertions(+), 10 deletions(-)
> +
> +diff --git a/src/build.cc b/src/build.cc
> +index 53e4405..d8a6dff 100644
> +--- a/src/build.cc
> ++++ b/src/build.cc
> +@@ -474,9 +474,9 @@ struct RealCommandRunner : public CommandRunner {
> + RealCommandRunner::RealCommandRunner(const BuildConfig& config) : config_(config) {
> +   max_load_average_ = config.max_load_average;
> +   if ((tokens_ = TokenPool::Get()) != NULL) {
> +-    if (!tokens_->Setup(config_.parallelism_from_cmdline,
> +-                        config_.verbosity == BuildConfig::VERBOSE,
> +-                        max_load_average_)) {
> ++    if (!tokens_->SetupClient(config_.parallelism_from_cmdline,
> ++                              config_.verbosity == BuildConfig::VERBOSE,
> ++                              max_load_average_)) {
> +       delete tokens_;
> +       tokens_ = NULL;
> +     }
> +diff --git a/src/subprocess_test.cc b/src/subprocess_test.cc
> +index eddc110..222b59b 100644
> +--- a/src/subprocess_test.cc
> ++++ b/src/subprocess_test.cc
> +@@ -40,7 +40,8 @@ struct TestTokenPool : public TokenPool {
> +   void Reserve()     {}
> +   void Release()     {}
> +   void Clear()       {}
> +-  bool Setup(bool ignore_unused, bool verbose, double& max_load_average) { return false; }
> ++  bool SetupClient(bool ignore_unused, bool verbose,
> ++                   double& max_load_average) { return false; }
> + 
> + #ifdef _WIN32
> +   bool _token_available;
> +diff --git a/src/tokenpool-gnu-make.cc b/src/tokenpool-gnu-make.cc
> +index 60e0552..6fb72a6 100644
> +--- a/src/tokenpool-gnu-make.cc
> ++++ b/src/tokenpool-gnu-make.cc
> +@@ -28,9 +28,9 @@ GNUmakeTokenPool::GNUmakeTokenPool() : available_(1), used_(0) {
> + GNUmakeTokenPool::~GNUmakeTokenPool() {
> + }
> + 
> +-bool GNUmakeTokenPool::Setup(bool ignore,
> +-                             bool verbose,
> +-                             double& max_load_average) {
> ++bool GNUmakeTokenPool::SetupClient(bool ignore,
> ++                                   bool verbose,
> ++                                   double& max_load_average) {
> +   const char* value = GetEnv("MAKEFLAGS");
> +   if (!value)
> +     return false;
> +diff --git a/src/tokenpool-gnu-make.h b/src/tokenpool-gnu-make.h
> +index c94cca5..f4ab8d7 100644
> +--- a/src/tokenpool-gnu-make.h
> ++++ b/src/tokenpool-gnu-make.h
> +@@ -24,7 +24,8 @@ struct GNUmakeTokenPool : public TokenPool {
> +   virtual void Reserve();
> +   virtual void Release();
> +   virtual void Clear();
> +-  virtual bool Setup(bool ignore, bool verbose, double& max_load_average);
> ++  virtual bool SetupClient(bool ignore, bool verbose,
> ++                           double& max_load_average);
> + 
> +   // platform specific implementation
> +   virtual const char* GetEnv(const char* name) = 0;
> +diff --git a/src/tokenpool.h b/src/tokenpool.h
> +index 931c227..ce2bf48 100644
> +--- a/src/tokenpool.h
> ++++ b/src/tokenpool.h
> +@@ -26,7 +26,8 @@ struct TokenPool {
> +   virtual void Clear() = 0;
> + 
> +   // returns false if token pool setup failed
> +-  virtual bool Setup(bool ignore, bool verbose, double& max_load_average) = 0;
> ++  virtual bool SetupClient(bool ignore, bool verbose,
> ++                           double& max_load_average) = 0;
> + 
> + #ifdef _WIN32
> +   virtual void WaitForTokenAvailability(HANDLE ioport) = 0;
> +diff --git a/src/tokenpool_test.cc b/src/tokenpool_test.cc
> +index 1bc1c84..9d07146 100644
> +--- a/src/tokenpool_test.cc
> ++++ b/src/tokenpool_test.cc
> +@@ -77,7 +77,7 @@ struct TokenPoolTest : public testing::Test {
> +       ENVIRONMENT_INIT(buf_);
> +     }
> +     if ((tokens_ = TokenPool::Get()) != NULL) {
> +-      if (!tokens_->Setup(ignore_jobserver, false, load_avg_)) {
> ++      if (!tokens_->SetupClient(ignore_jobserver, false, load_avg_)) {
> +         delete tokens_;
> +         tokens_ = NULL;
> +       }
> +-- 
> +2.44.0
> +
> diff --git a/meta/recipes-devtools/ninja/files/0002-Add-GNU-make-jobserver-fifo-style-client-support.patch b/meta/recipes-devtools/ninja/files/0002-Add-GNU-make-jobserver-fifo-style-client-support.patch
> new file mode 100644
> index 0000000000..770a53fbc9
> --- /dev/null
> +++ b/meta/recipes-devtools/ninja/files/0002-Add-GNU-make-jobserver-fifo-style-client-support.patch
> @@ -0,0 +1,271 @@
> +From 830b7fb396383171c5fac2aae7de880ac83ce955 Mon Sep 17 00:00:00 2001
> +From: Stefan Becker <chemobejk@gmail.com>
> +Date: Mon, 7 Nov 2022 20:45:27 +0200
> +Subject: [PATCH 2/2] Add GNU make jobserver fifo style client support
> +MIME-Version: 1.0
> +Content-Type: text/plain; charset=UTF-8
> +Content-Transfer-Encoding: 8bit
> +
> +GNU make 4.4 introduced a new jobserver style "fifo" for POSIX systems
> +which passes a named pipe down to the clients.
> +
> +- update auth parser to recognize "fifo:<name>" format
> +- open named pipe for reading and writing
> +- make sure the file descriptors are closed in the destructor
> +- add 2 tests that aren't compiled for WIN32
> +
> +Signed-off-by: Martin Hundeb�ll <martin@geanix.com>
> +Upstream-Status: Submitted [https://github.com/Kitware/ninja/pull/2]
> +---
> + src/tokenpool-gnu-make-posix.cc | 46 ++++++++++++++++-
> + src/tokenpool_test.cc           | 88 ++++++++++++++++++++++++++++++---
> + 2 files changed, 127 insertions(+), 7 deletions(-)
> +
> +diff --git a/src/tokenpool-gnu-make-posix.cc b/src/tokenpool-gnu-make-posix.cc
> +index 8447070..a82cce4 100644
> +--- a/src/tokenpool-gnu-make-posix.cc
> ++++ b/src/tokenpool-gnu-make-posix.cc
> +@@ -40,6 +40,7 @@ struct GNUmakeTokenPoolPosix : public GNUmakeTokenPool {
> +  private:
> +   int rfd_;
> +   int wfd_;
> ++  bool closeFds_;
> + 
> +   struct sigaction old_act_;
> +   bool restore_;
> +@@ -48,14 +49,19 @@ struct GNUmakeTokenPoolPosix : public GNUmakeTokenPool {
> +   static void CloseDupRfd(int signum);
> + 
> +   bool CheckFd(int fd);
> ++  bool CheckFifo(const char* fifo);
> +   bool SetAlarmHandler();
> + };
> + 
> +-GNUmakeTokenPoolPosix::GNUmakeTokenPoolPosix() : rfd_(-1), wfd_(-1), restore_(false) {
> ++GNUmakeTokenPoolPosix::GNUmakeTokenPoolPosix() : rfd_(-1), wfd_(-1), closeFds_(false), restore_(false) {
> + }
> + 
> + GNUmakeTokenPoolPosix::~GNUmakeTokenPoolPosix() {
> +   Clear();
> ++  if (closeFds_) {
> ++    close(wfd_);
> ++    close(rfd_);
> ++  }
> +   if (restore_)
> +     sigaction(SIGALRM, &old_act_, NULL);
> + }
> +@@ -69,6 +75,36 @@ bool GNUmakeTokenPoolPosix::CheckFd(int fd) {
> +   return true;
> + }
> + 
> ++bool GNUmakeTokenPoolPosix::CheckFifo(const char* fifo) {
> ++  // remove possible junk from end of fifo name
> ++  char *filename = strdup(fifo);
> ++  char *end;
> ++  if ((end = strchr(filename, ' ')) != NULL) {
> ++    *end = '\0';
> ++  }
> ++
> ++  int rfd = open(filename, O_RDONLY);
> ++  if (rfd < 0) {
> ++    free(filename);
> ++    return false;
> ++  }
> ++
> ++  int wfd = open(filename, O_WRONLY);
> ++  if (wfd < 0) {
> ++    close(rfd);
> ++    free(filename);
> ++    return false;
> ++  }
> ++
> ++  free(filename);
> ++
> ++  rfd_ = rfd;
> ++  wfd_ = wfd;
> ++  closeFds_ = true;
> ++
> ++  return true;
> ++}
> ++
> + int GNUmakeTokenPoolPosix::dup_rfd_ = -1;
> + 
> + void GNUmakeTokenPoolPosix::CloseDupRfd(int signum) {
> +@@ -89,6 +125,13 @@ bool GNUmakeTokenPoolPosix::SetAlarmHandler() {
> + }
> + 
> + bool GNUmakeTokenPoolPosix::ParseAuth(const char* jobserver) {
> ++  // check for jobserver-fifo style
> ++  const char* fifo;
> ++  if (((fifo = strstr(jobserver, "=fifo:")) != NULL) &&
> ++      CheckFifo(fifo + 6))
> ++    return SetAlarmHandler();
> ++
> ++  // check for legacy simple pipe style
> +   int rfd = -1;
> +   int wfd = -1;
> +   if ((sscanf(jobserver, "%*[^=]=%d,%d", &rfd, &wfd) == 2) &&
> +@@ -100,6 +143,7 @@ bool GNUmakeTokenPoolPosix::ParseAuth(const char* jobserver) {
> +     return true;
> +   }
> + 
> ++  // some jobserver style we don't support
> +   return false;
> + }
> + 
> +diff --git a/src/tokenpool_test.cc b/src/tokenpool_test.cc
> +index 9d07146..e15da6c 100644
> +--- a/src/tokenpool_test.cc
> ++++ b/src/tokenpool_test.cc
> +@@ -19,7 +19,10 @@
> + #ifdef _WIN32
> + #include <windows.h>
> + #else
> ++#include <fcntl.h>
> + #include <unistd.h>
> ++#include <sys/types.h>
> ++#include <sys/stat.h>
> + #endif
> + 
> + #include <stdio.h>
> +@@ -35,6 +38,8 @@
> + #define AUTH_FORMAT(tmpl)   "foo " tmpl "=%d,%d bar"
> + #define ENVIRONMENT_CLEAR() unsetenv("MAKEFLAGS")
> + #define ENVIRONMENT_INIT(v) setenv("MAKEFLAGS", v, true)
> ++
> ++#define FIFO_NAME "ninja-test-tokenpool-fifo"
> + #endif
> + 
> + namespace {
> +@@ -60,11 +65,24 @@ struct TokenPoolTest : public testing::Test {
> +     semaphore_name_ = SEMAPHORE_NAME;
> +     if ((semaphore_ = CreateSemaphore(0, 0, 2, SEMAPHORE_NAME)) == NULL)
> + #else
> ++    if (mkfifo(FIFO_NAME, 0600) < 0) {
> ++      ASSERT_TRUE(false);
> ++    }
> ++
> +     if (pipe(fds_) < 0)
> + #endif
> +       ASSERT_TRUE(false);
> +   }
> + 
> ++  void GetPool(bool ignore_jobserver) {
> ++    if ((tokens_ = TokenPool::Get()) != NULL) {
> ++      if (!tokens_->SetupClient(ignore_jobserver, false, load_avg_)) {
> ++        delete tokens_;
> ++        tokens_ = NULL;
> ++      }
> ++    }
> ++  }
> ++
> +   void CreatePool(const char* format, bool ignore_jobserver = false) {
> +     if (format) {
> +       sprintf(buf_, format,
> +@@ -76,18 +94,30 @@ struct TokenPoolTest : public testing::Test {
> +       );
> +       ENVIRONMENT_INIT(buf_);
> +     }
> +-    if ((tokens_ = TokenPool::Get()) != NULL) {
> +-      if (!tokens_->SetupClient(ignore_jobserver, false, load_avg_)) {
> +-        delete tokens_;
> +-        tokens_ = NULL;
> +-      }
> +-    }
> ++    GetPool(ignore_jobserver);
> +   }
> + 
> +   void CreateDefaultPool() {
> +     CreatePool(AUTH_FORMAT("--jobserver-auth"));
> +   }
> + 
> ++#ifndef _WIN32
> ++  void CreateFifoPool() {
> ++    // close simple pipe fds...
> ++    close(fds_[0]);
> ++    close(fds_[1]);
> ++
> ++    // ... and open named pipe instead
> ++    if ((fds_[0] = open(FIFO_NAME, O_RDONLY|O_NONBLOCK)) < 0)
> ++      ASSERT_TRUE(false);
> ++    if ((fds_[1] = open(FIFO_NAME, O_WRONLY)) < 0)
> ++      ASSERT_TRUE(false);
> ++
> ++    ENVIRONMENT_INIT("foo --jobserver-auth=fifo:" FIFO_NAME " bar");
> ++    GetPool(false);
> ++  }
> ++#endif
> ++
> +   virtual void TearDown() {
> +     if (tokens_)
> +       delete tokens_;
> +@@ -96,6 +126,7 @@ struct TokenPoolTest : public testing::Test {
> + #else
> +     close(fds_[0]);
> +     close(fds_[1]);
> ++    unlink(FIFO_NAME);
> + #endif
> +     ENVIRONMENT_CLEAR();
> +   }
> +@@ -167,6 +198,15 @@ TEST_F(TokenPoolTest, MonitorFD) {
> + 
> +   EXPECT_EQ(fds_[0], tokens_->GetMonitorFd());
> + }
> ++
> ++TEST_F(TokenPoolTest, MonitorFDFifo) {
> ++  CreateFifoPool();
> ++
> ++  ASSERT_NE(NULL, tokens_);
> ++  EXPECT_EQ(kLoadAverageDefault, load_avg_);
> ++
> ++  EXPECT_NE(-1, tokens_->GetMonitorFd());
> ++}
> + #endif
> + 
> + TEST_F(TokenPoolTest, ImplicitToken) {
> +@@ -226,6 +266,42 @@ TEST_F(TokenPoolTest, TwoTokens) {
> +   EXPECT_TRUE(tokens_->Acquire());
> + }
> + 
> ++#ifndef _WIN32
> ++TEST_F(TokenPoolTest, TwoTokensFifo) {
> ++  CreateFifoPool();
> ++
> ++  ASSERT_NE(NULL, tokens_);
> ++  EXPECT_EQ(kLoadAverageDefault, load_avg_);
> ++
> ++  // implicit token
> ++  EXPECT_TRUE(tokens_->Acquire());
> ++  tokens_->Reserve();
> ++  EXPECT_FALSE(tokens_->Acquire());
> ++
> ++  // jobserver offers 2nd token
> ++  char test_tokens[1] = { '+' };
> ++  ASSERT_EQ(1u, write(fds_[1], test_tokens, sizeof(test_tokens)));
> ++  EXPECT_TRUE(tokens_->Acquire());
> ++  tokens_->Reserve();
> ++  EXPECT_FALSE(tokens_->Acquire());
> ++
> ++  // release 2nd token
> ++  tokens_->Release();
> ++  EXPECT_TRUE(tokens_->Acquire());
> ++
> ++  // release implicit token - must return 2nd token back to jobserver
> ++  tokens_->Release();
> ++  EXPECT_TRUE(tokens_->Acquire());
> ++
> ++  // there must be one token available
> ++  EXPECT_EQ(1u, read(fds_[0], buf_, sizeof(buf_)));
> ++  EXPECT_EQ(test_tokens[0], buf_[0]);
> ++
> ++  // implicit token
> ++  EXPECT_TRUE(tokens_->Acquire());
> ++}
> ++#endif
> ++
> + TEST_F(TokenPoolTest, Clear) {
> +   CreateDefaultPool();
> + 
> +-- 
> +2.44.0
> +
> diff --git a/meta/recipes-devtools/ninja/ninja_1.11.1.bb b/meta/recipes-devtools/ninja/ninja_1.11.1.bb
> index 8e297ec4d4..dbff2e191a 100644
> --- a/meta/recipes-devtools/ninja/ninja_1.11.1.bb
> +++ b/meta/recipes-devtools/ninja/ninja_1.11.1.bb
> @@ -6,9 +6,13 @@ LIC_FILES_CHKSUM = "file://COPYING;md5=a81586a64ad4e476c791cda7e2f2c52e"
>  
>  DEPENDS = "re2c-native ninja-native"
>  
> -SRCREV = "a524bf3f6bacd1b4ad85d719eed2737d8562f27a"
> +SRCREV = "95dee2a91d96c409d54f9fa0b70ea9aa2bdf8e63"
>  
> -SRC_URI = "git://github.com/ninja-build/ninja.git;branch=release;protocol=https"
> +SRC_URI = " \
> +    git://github.com/kitware/ninja.git;branch=kitware-staged-features;protocol=https \
> +    file://0001-Rename-TokenPool-Setup-to-SetupClient.patch \
> +    file://0002-Add-GNU-make-jobserver-fifo-style-client-support.patch \
> +"
>  UPSTREAM_CHECK_GITTAGREGEX = "v(?P<pver>.*)"
>  
>  S = "${WORKDIR}/git"
> -- 
> 2.44.0
> 

> 
> -=-=-=-=-=-=-=-=-=-=-=-
> Links: You receive all messages sent to this group.
> View/Reply Online (#197956): https://lists.openembedded.org/g/openembedded-core/message/197956
> Mute This Topic: https://lists.openembedded.org/mt/105326967/3617179
> Group Owner: openembedded-core+owner@lists.openembedded.org
> Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [alexandre.belloni@bootlin.com]
> -=-=-=-=-=-=-=-=-=-=-=-
> 


-- 
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Re: [OE-core] [PATCH v2 3/5] ninja: build modified version with GNU Make jobserver support

[Martin Hundebøll]

Hi Alexandre,

I see you keep carrying these patches. I'm still working with ninja
upstream to add jobserver support there[1]. I was planning to redo
these patches once ninja merges that. But I can do a respin with my
patches added to ninja if you like?

[1] https://github.com/ninja-build/ninja/pull/2450

On Thu, 2024-04-25 at 13:30 +0200, Alexandre Belloni wrote:
> Hello,
> 
> This patch doesn't apply on master anymore because ninja has been
> updated.
> 
> 
> On 04/04/2024 13:16:11+0200, Martin Hundeb?ll wrote:
> > Ninja doesn't (yet) support the GNU Make jobserver out of the box,
> > but
> > there is a pull request adding that support[1]. Since that pull
> > request
> > (and its derived three-part pull requests) seem to be ignored by
> > upstream, kitware (creator/maintainer of cmake) has created a
> > fork[2]
> > only to carry the jobserver patches. Change the source uri to point
> > at
> > the kitware fork of ninja, and add two patches from the original
> > pull
> > request to also support the new-style fifo jobserver feature.
> > 
> > Note that the kitware fork of ninja is also used by buildroot[3].
> > 
> > [1] https://github.com/ninja-build/ninja/pull/2263
> > [2] https://github.com/Kitware/ninja
> > [3]
> > https://gitlab.com/buildroot.org/buildroot/-/blob/master/package/ninja/ninja.mk
> > 
> > Signed-off-by: Martin Hundebøll <martin@geanix.com>
> > ---
> >  ...ename-TokenPool-Setup-to-SetupClient.patch | 113 ++++++++
> >  ...-jobserver-fifo-style-client-support.patch | 271
> > ++++++++++++++++++
> >  meta/recipes-devtools/ninja/ninja_1.11.1.bb   |   8 +-
> >  3 files changed, 390 insertions(+), 2 deletions(-)
> >  create mode 100644 meta/recipes-devtools/ninja/files/0001-Rename-
> > TokenPool-Setup-to-SetupClient.patch
> >  create mode 100644 meta/recipes-devtools/ninja/files/0002-Add-GNU-
> > make-jobserver-fifo-style-client-support.patch
> > 
> > diff --git a/meta/recipes-devtools/ninja/files/0001-Rename-
> > TokenPool-Setup-to-SetupClient.patch b/meta/recipes-
> > devtools/ninja/files/0001-Rename-TokenPool-Setup-to-
> > SetupClient.patch
> > new file mode 100644
> > index 0000000000..a503f8c75f
> > --- /dev/null
> > +++ b/meta/recipes-devtools/ninja/files/0001-Rename-TokenPool-
> > Setup-to-SetupClient.patch
> > @@ -0,0 +1,113 @@
> > +From f5642d8b49688dfc84679451b531d92f3b6e7cb0 Mon Sep 17 00:00:00
> > 2001
> > +From: Stefan Becker <chemobejk@gmail.com>
> > +Date: Sat, 15 Dec 2018 19:29:42 +0200
> > +Subject: [PATCH 1/2] Rename TokenPool::Setup() to SetupClient()
> > +MIME-Version: 1.0
> > +Content-Type: text/plain; charset=UTF-8
> > +Content-Transfer-Encoding: 8bit
> > +
> > +Make space to add new API to set up token pool master.
> > +
> > +Signed-off-by: Martin Hundebøll <martin@geanix.com>
> > +Upstream-Status: Submitted
> > [https://github.com/Kitware/ninja/pull/2]
> > +---
> > + src/build.cc              | 6 +++---
> > + src/subprocess_test.cc    | 3 ++-
> > + src/tokenpool-gnu-make.cc | 6 +++---
> > + src/tokenpool-gnu-make.h  | 3 ++-
> > + src/tokenpool.h           | 3 ++-
> > + src/tokenpool_test.cc     | 2 +-
> > + 6 files changed, 13 insertions(+), 10 deletions(-)
> > +
> > +diff --git a/src/build.cc b/src/build.cc
> > +index 53e4405..d8a6dff 100644
> > +--- a/src/build.cc
> > ++++ b/src/build.cc
> > +@@ -474,9 +474,9 @@ struct RealCommandRunner : public
> > CommandRunner {
> > + RealCommandRunner::RealCommandRunner(const BuildConfig& config) :
> > config_(config) {
> > +   max_load_average_ = config.max_load_average;
> > +   if ((tokens_ = TokenPool::Get()) != NULL) {
> > +-    if (!tokens_->Setup(config_.parallelism_from_cmdline,
> > +-                        config_.verbosity ==
> > BuildConfig::VERBOSE,
> > +-                        max_load_average_)) {
> > ++    if (!tokens_->SetupClient(config_.parallelism_from_cmdline,
> > ++                              config_.verbosity ==
> > BuildConfig::VERBOSE,
> > ++                              max_load_average_)) {
> > +       delete tokens_;
> > +       tokens_ = NULL;
> > +     }
> > +diff --git a/src/subprocess_test.cc b/src/subprocess_test.cc
> > +index eddc110..222b59b 100644
> > +--- a/src/subprocess_test.cc
> > ++++ b/src/subprocess_test.cc
> > +@@ -40,7 +40,8 @@ struct TestTokenPool : public TokenPool {
> > +   void Reserve()     {}
> > +   void Release()     {}
> > +   void Clear()       {}
> > +-  bool Setup(bool ignore_unused, bool verbose, double&
> > max_load_average) { return false; }
> > ++  bool SetupClient(bool ignore_unused, bool verbose,
> > ++                   double& max_load_average) { return false; }
> > + 
> > + #ifdef _WIN32
> > +   bool _token_available;
> > +diff --git a/src/tokenpool-gnu-make.cc b/src/tokenpool-gnu-make.cc
> > +index 60e0552..6fb72a6 100644
> > +--- a/src/tokenpool-gnu-make.cc
> > ++++ b/src/tokenpool-gnu-make.cc
> > +@@ -28,9 +28,9 @@ GNUmakeTokenPool::GNUmakeTokenPool() :
> > available_(1), used_(0) {
> > + GNUmakeTokenPool::~GNUmakeTokenPool() {
> > + }
> > + 
> > +-bool GNUmakeTokenPool::Setup(bool ignore,
> > +-                             bool verbose,
> > +-                             double& max_load_average) {
> > ++bool GNUmakeTokenPool::SetupClient(bool ignore,
> > ++                                   bool verbose,
> > ++                                   double& max_load_average) {
> > +   const char* value = GetEnv("MAKEFLAGS");
> > +   if (!value)
> > +     return false;
> > +diff --git a/src/tokenpool-gnu-make.h b/src/tokenpool-gnu-make.h
> > +index c94cca5..f4ab8d7 100644
> > +--- a/src/tokenpool-gnu-make.h
> > ++++ b/src/tokenpool-gnu-make.h
> > +@@ -24,7 +24,8 @@ struct GNUmakeTokenPool : public TokenPool {
> > +   virtual void Reserve();
> > +   virtual void Release();
> > +   virtual void Clear();
> > +-  virtual bool Setup(bool ignore, bool verbose, double&
> > max_load_average);
> > ++  virtual bool SetupClient(bool ignore, bool verbose,
> > ++                           double& max_load_average);
> > + 
> > +   // platform specific implementation
> > +   virtual const char* GetEnv(const char* name) = 0;
> > +diff --git a/src/tokenpool.h b/src/tokenpool.h
> > +index 931c227..ce2bf48 100644
> > +--- a/src/tokenpool.h
> > ++++ b/src/tokenpool.h
> > +@@ -26,7 +26,8 @@ struct TokenPool {
> > +   virtual void Clear() = 0;
> > + 
> > +   // returns false if token pool setup failed
> > +-  virtual bool Setup(bool ignore, bool verbose, double&
> > max_load_average) = 0;
> > ++  virtual bool SetupClient(bool ignore, bool verbose,
> > ++                           double& max_load_average) = 0;
> > + 
> > + #ifdef _WIN32
> > +   virtual void WaitForTokenAvailability(HANDLE ioport) = 0;
> > +diff --git a/src/tokenpool_test.cc b/src/tokenpool_test.cc
> > +index 1bc1c84..9d07146 100644
> > +--- a/src/tokenpool_test.cc
> > ++++ b/src/tokenpool_test.cc
> > +@@ -77,7 +77,7 @@ struct TokenPoolTest : public testing::Test {
> > +       ENVIRONMENT_INIT(buf_);
> > +     }
> > +     if ((tokens_ = TokenPool::Get()) != NULL) {
> > +-      if (!tokens_->Setup(ignore_jobserver, false, load_avg_)) {
> > ++      if (!tokens_->SetupClient(ignore_jobserver, false,
> > load_avg_)) {
> > +         delete tokens_;
> > +         tokens_ = NULL;
> > +       }
> > +-- 
> > +2.44.0
> > +
> > diff --git a/meta/recipes-devtools/ninja/files/0002-Add-GNU-make-
> > jobserver-fifo-style-client-support.patch b/meta/recipes-
> > devtools/ninja/files/0002-Add-GNU-make-jobserver-fifo-style-client-
> > support.patch
> > new file mode 100644
> > index 0000000000..770a53fbc9
> > --- /dev/null
> > +++ b/meta/recipes-devtools/ninja/files/0002-Add-GNU-make-
> > jobserver-fifo-style-client-support.patch
> > @@ -0,0 +1,271 @@
> > +From 830b7fb396383171c5fac2aae7de880ac83ce955 Mon Sep 17 00:00:00
> > 2001
> > +From: Stefan Becker <chemobejk@gmail.com>
> > +Date: Mon, 7 Nov 2022 20:45:27 +0200
> > +Subject: [PATCH 2/2] Add GNU make jobserver fifo style client
> > support
> > +MIME-Version: 1.0
> > +Content-Type: text/plain; charset=UTF-8
> > +Content-Transfer-Encoding: 8bit
> > +
> > +GNU make 4.4 introduced a new jobserver style "fifo" for POSIX
> > systems
> > +which passes a named pipe down to the clients.
> > +
> > +- update auth parser to recognize "fifo:<name>" format
> > +- open named pipe for reading and writing
> > +- make sure the file descriptors are closed in the destructor
> > +- add 2 tests that aren't compiled for WIN32
> > +
> > +Signed-off-by: Martin Hundebøll <martin@geanix.com>
> > +Upstream-Status: Submitted
> > [https://github.com/Kitware/ninja/pull/2]
> > +---
> > + src/tokenpool-gnu-make-posix.cc | 46 ++++++++++++++++-
> > + src/tokenpool_test.cc           | 88
> > ++++++++++++++++++++++++++++++---
> > + 2 files changed, 127 insertions(+), 7 deletions(-)
> > +
> > +diff --git a/src/tokenpool-gnu-make-posix.cc b/src/tokenpool-gnu-
> > make-posix.cc
> > +index 8447070..a82cce4 100644
> > +--- a/src/tokenpool-gnu-make-posix.cc
> > ++++ b/src/tokenpool-gnu-make-posix.cc
> > +@@ -40,6 +40,7 @@ struct GNUmakeTokenPoolPosix : public
> > GNUmakeTokenPool {
> > +  private:
> > +   int rfd_;
> > +   int wfd_;
> > ++  bool closeFds_;
> > + 
> > +   struct sigaction old_act_;
> > +   bool restore_;
> > +@@ -48,14 +49,19 @@ struct GNUmakeTokenPoolPosix : public
> > GNUmakeTokenPool {
> > +   static void CloseDupRfd(int signum);
> > + 
> > +   bool CheckFd(int fd);
> > ++  bool CheckFifo(const char* fifo);
> > +   bool SetAlarmHandler();
> > + };
> > + 
> > +-GNUmakeTokenPoolPosix::GNUmakeTokenPoolPosix() : rfd_(-1), wfd_(-
> > 1), restore_(false) {
> > ++GNUmakeTokenPoolPosix::GNUmakeTokenPoolPosix() : rfd_(-1), wfd_(-
> > 1), closeFds_(false), restore_(false) {
> > + }
> > + 
> > + GNUmakeTokenPoolPosix::~GNUmakeTokenPoolPosix() {
> > +   Clear();
> > ++  if (closeFds_) {
> > ++    close(wfd_);
> > ++    close(rfd_);
> > ++  }
> > +   if (restore_)
> > +     sigaction(SIGALRM, &old_act_, NULL);
> > + }
> > +@@ -69,6 +75,36 @@ bool GNUmakeTokenPoolPosix::CheckFd(int fd) {
> > +   return true;
> > + }
> > + 
> > ++bool GNUmakeTokenPoolPosix::CheckFifo(const char* fifo) {
> > ++  // remove possible junk from end of fifo name
> > ++  char *filename = strdup(fifo);
> > ++  char *end;
> > ++  if ((end = strchr(filename, ' ')) != NULL) {
> > ++    *end = '\0';
> > ++  }
> > ++
> > ++  int rfd = open(filename, O_RDONLY);
> > ++  if (rfd < 0) {
> > ++    free(filename);
> > ++    return false;
> > ++  }
> > ++
> > ++  int wfd = open(filename, O_WRONLY);
> > ++  if (wfd < 0) {
> > ++    close(rfd);
> > ++    free(filename);
> > ++    return false;
> > ++  }
> > ++
> > ++  free(filename);
> > ++
> > ++  rfd_ = rfd;
> > ++  wfd_ = wfd;
> > ++  closeFds_ = true;
> > ++
> > ++  return true;
> > ++}
> > ++
> > + int GNUmakeTokenPoolPosix::dup_rfd_ = -1;
> > + 
> > + void GNUmakeTokenPoolPosix::CloseDupRfd(int signum) {
> > +@@ -89,6 +125,13 @@ bool GNUmakeTokenPoolPosix::SetAlarmHandler()
> > {
> > + }
> > + 
> > + bool GNUmakeTokenPoolPosix::ParseAuth(const char* jobserver) {
> > ++  // check for jobserver-fifo style
> > ++  const char* fifo;
> > ++  if (((fifo = strstr(jobserver, "=fifo:")) != NULL) &&
> > ++      CheckFifo(fifo + 6))
> > ++    return SetAlarmHandler();
> > ++
> > ++  // check for legacy simple pipe style
> > +   int rfd = -1;
> > +   int wfd = -1;
> > +   if ((sscanf(jobserver, "%*[^=]=%d,%d", &rfd, &wfd) == 2) &&
> > +@@ -100,6 +143,7 @@ bool GNUmakeTokenPoolPosix::ParseAuth(const
> > char* jobserver) {
> > +     return true;
> > +   }
> > + 
> > ++  // some jobserver style we don't support
> > +   return false;
> > + }
> > + 
> > +diff --git a/src/tokenpool_test.cc b/src/tokenpool_test.cc
> > +index 9d07146..e15da6c 100644
> > +--- a/src/tokenpool_test.cc
> > ++++ b/src/tokenpool_test.cc
> > +@@ -19,7 +19,10 @@
> > + #ifdef _WIN32
> > + #include <windows.h>
> > + #else
> > ++#include <fcntl.h>
> > + #include <unistd.h>
> > ++#include <sys/types.h>
> > ++#include <sys/stat.h>
> > + #endif
> > + 
> > + #include <stdio.h>
> > +@@ -35,6 +38,8 @@
> > + #define AUTH_FORMAT(tmpl)   "foo " tmpl "=%d,%d bar"
> > + #define ENVIRONMENT_CLEAR() unsetenv("MAKEFLAGS")
> > + #define ENVIRONMENT_INIT(v) setenv("MAKEFLAGS", v, true)
> > ++
> > ++#define FIFO_NAME "ninja-test-tokenpool-fifo"
> > + #endif
> > + 
> > + namespace {
> > +@@ -60,11 +65,24 @@ struct TokenPoolTest : public testing::Test {
> > +     semaphore_name_ = SEMAPHORE_NAME;
> > +     if ((semaphore_ = CreateSemaphore(0, 0, 2, SEMAPHORE_NAME))
> > == NULL)
> > + #else
> > ++    if (mkfifo(FIFO_NAME, 0600) < 0) {
> > ++      ASSERT_TRUE(false);
> > ++    }
> > ++
> > +     if (pipe(fds_) < 0)
> > + #endif
> > +       ASSERT_TRUE(false);
> > +   }
> > + 
> > ++  void GetPool(bool ignore_jobserver) {
> > ++    if ((tokens_ = TokenPool::Get()) != NULL) {
> > ++      if (!tokens_->SetupClient(ignore_jobserver, false,
> > load_avg_)) {
> > ++        delete tokens_;
> > ++        tokens_ = NULL;
> > ++      }
> > ++    }
> > ++  }
> > ++
> > +   void CreatePool(const char* format, bool ignore_jobserver =
> > false) {
> > +     if (format) {
> > +       sprintf(buf_, format,
> > +@@ -76,18 +94,30 @@ struct TokenPoolTest : public testing::Test {
> > +       );
> > +       ENVIRONMENT_INIT(buf_);
> > +     }
> > +-    if ((tokens_ = TokenPool::Get()) != NULL) {
> > +-      if (!tokens_->SetupClient(ignore_jobserver, false,
> > load_avg_)) {
> > +-        delete tokens_;
> > +-        tokens_ = NULL;
> > +-      }
> > +-    }
> > ++    GetPool(ignore_jobserver);
> > +   }
> > + 
> > +   void CreateDefaultPool() {
> > +     CreatePool(AUTH_FORMAT("--jobserver-auth"));
> > +   }
> > + 
> > ++#ifndef _WIN32
> > ++  void CreateFifoPool() {
> > ++    // close simple pipe fds...
> > ++    close(fds_[0]);
> > ++    close(fds_[1]);
> > ++
> > ++    // ... and open named pipe instead
> > ++    if ((fds_[0] = open(FIFO_NAME, O_RDONLY|O_NONBLOCK)) < 0)
> > ++      ASSERT_TRUE(false);
> > ++    if ((fds_[1] = open(FIFO_NAME, O_WRONLY)) < 0)
> > ++      ASSERT_TRUE(false);
> > ++
> > ++    ENVIRONMENT_INIT("foo --jobserver-auth=fifo:" FIFO_NAME "
> > bar");
> > ++    GetPool(false);
> > ++  }
> > ++#endif
> > ++
> > +   virtual void TearDown() {
> > +     if (tokens_)
> > +       delete tokens_;
> > +@@ -96,6 +126,7 @@ struct TokenPoolTest : public testing::Test {
> > + #else
> > +     close(fds_[0]);
> > +     close(fds_[1]);
> > ++    unlink(FIFO_NAME);
> > + #endif
> > +     ENVIRONMENT_CLEAR();
> > +   }
> > +@@ -167,6 +198,15 @@ TEST_F(TokenPoolTest, MonitorFD) {
> > + 
> > +   EXPECT_EQ(fds_[0], tokens_->GetMonitorFd());
> > + }
> > ++
> > ++TEST_F(TokenPoolTest, MonitorFDFifo) {
> > ++  CreateFifoPool();
> > ++
> > ++  ASSERT_NE(NULL, tokens_);
> > ++  EXPECT_EQ(kLoadAverageDefault, load_avg_);
> > ++
> > ++  EXPECT_NE(-1, tokens_->GetMonitorFd());
> > ++}
> > + #endif
> > + 
> > + TEST_F(TokenPoolTest, ImplicitToken) {
> > +@@ -226,6 +266,42 @@ TEST_F(TokenPoolTest, TwoTokens) {
> > +   EXPECT_TRUE(tokens_->Acquire());
> > + }
> > + 
> > ++#ifndef _WIN32
> > ++TEST_F(TokenPoolTest, TwoTokensFifo) {
> > ++  CreateFifoPool();
> > ++
> > ++  ASSERT_NE(NULL, tokens_);
> > ++  EXPECT_EQ(kLoadAverageDefault, load_avg_);
> > ++
> > ++  // implicit token
> > ++  EXPECT_TRUE(tokens_->Acquire());
> > ++  tokens_->Reserve();
> > ++  EXPECT_FALSE(tokens_->Acquire());
> > ++
> > ++  // jobserver offers 2nd token
> > ++  char test_tokens[1] = { '+' };
> > ++  ASSERT_EQ(1u, write(fds_[1], test_tokens,
> > sizeof(test_tokens)));
> > ++  EXPECT_TRUE(tokens_->Acquire());
> > ++  tokens_->Reserve();
> > ++  EXPECT_FALSE(tokens_->Acquire());
> > ++
> > ++  // release 2nd token
> > ++  tokens_->Release();
> > ++  EXPECT_TRUE(tokens_->Acquire());
> > ++
> > ++  // release implicit token - must return 2nd token back to
> > jobserver
> > ++  tokens_->Release();
> > ++  EXPECT_TRUE(tokens_->Acquire());
> > ++
> > ++  // there must be one token available
> > ++  EXPECT_EQ(1u, read(fds_[0], buf_, sizeof(buf_)));
> > ++  EXPECT_EQ(test_tokens[0], buf_[0]);
> > ++
> > ++  // implicit token
> > ++  EXPECT_TRUE(tokens_->Acquire());
> > ++}
> > ++#endif
> > ++
> > + TEST_F(TokenPoolTest, Clear) {
> > +   CreateDefaultPool();
> > + 
> > +-- 
> > +2.44.0
> > +
> > diff --git a/meta/recipes-devtools/ninja/ninja_1.11.1.bb
> > b/meta/recipes-devtools/ninja/ninja_1.11.1.bb
> > index 8e297ec4d4..dbff2e191a 100644
> > --- a/meta/recipes-devtools/ninja/ninja_1.11.1.bb
> > +++ b/meta/recipes-devtools/ninja/ninja_1.11.1.bb
> > @@ -6,9 +6,13 @@ LIC_FILES_CHKSUM =
> > "file://COPYING;md5=a81586a64ad4e476c791cda7e2f2c52e"
> >  
> >  DEPENDS = "re2c-native ninja-native"
> >  
> > -SRCREV = "a524bf3f6bacd1b4ad85d719eed2737d8562f27a"
> > +SRCREV = "95dee2a91d96c409d54f9fa0b70ea9aa2bdf8e63"
> >  
> > -SRC_URI = "git://github.com/ninja-
> > build/ninja.git;branch=release;protocol=https"
> > +SRC_URI = " \
> > +    git://github.com/kitware/ninja.git;branch=kitware-staged-
> > features;protocol=https \
> > +    file://0001-Rename-TokenPool-Setup-to-SetupClient.patch \
> > +   
> > file://0002-Add-GNU-make-jobserver-fifo-style-client-support.patch 
> > \
> > +"
> >  UPSTREAM_CHECK_GITTAGREGEX = "v(?P<pver>.*)"
> >  
> >  S = "${WORKDIR}/git"
> > -- 
> > 2.44.0
> > 
> 
> > 
> > -=-=-=-=-=-=-=-=-=-=-=-
> > Links: You receive all messages sent to this group.
> > View/Reply Online (#197956):
> > https://lists.openembedded.org/g/openembedded-core/message/197956
> > Mute This Topic:
> > https://lists.openembedded.org/mt/105326967/3617179
> > Group Owner: openembedded-core+owner@lists.openembedded.org
> > Unsubscribe:
> > https://lists.openembedded.org/g/openembedded-core/unsub [
> > alexandre.belloni@bootlin.com]
> > -=-=-=-=-=-=-=-=-=-=-=-
> > 
> 
> 

Re: [OE-core] [PATCH v2 3/5] ninja: build modified version with GNU Make jobserver support

[Alexandre Belloni]

On 02/08/2024 12:24:06+0200, Martin Hundeb?ll wrote:
> Hi Alexandre,
> 
> I see you keep carrying these patches. I'm still working with ninja
> upstream to add jobserver support there[1]. I was planning to redo
> these patches once ninja merges that. But I can do a respin with my
> patches added to ninja if you like?
> 
> [1] https://github.com/ninja-build/ninja/pull/2450

I carry them so they get some testing, I can drop until you submit the
new version.

> 
> On Thu, 2024-04-25 at 13:30 +0200, Alexandre Belloni wrote:
> > Hello,
> > 
> > This patch doesn't apply on master anymore because ninja has been
> > updated.
> > 
> > 
> > On 04/04/2024 13:16:11+0200, Martin Hundeb?ll wrote:
> > > Ninja doesn't (yet) support the GNU Make jobserver out of the box,
> > > but
> > > there is a pull request adding that support[1]. Since that pull
> > > request
> > > (and its derived three-part pull requests) seem to be ignored by
> > > upstream, kitware (creator/maintainer of cmake) has created a
> > > fork[2]
> > > only to carry the jobserver patches. Change the source uri to point
> > > at
> > > the kitware fork of ninja, and add two patches from the original
> > > pull
> > > request to also support the new-style fifo jobserver feature.
> > > 
> > > Note that the kitware fork of ninja is also used by buildroot[3].
> > > 
> > > [1] https://github.com/ninja-build/ninja/pull/2263
> > > [2] https://github.com/Kitware/ninja
> > > [3]
> > > https://gitlab.com/buildroot.org/buildroot/-/blob/master/package/ninja/ninja.mk
> > > 
> > > Signed-off-by: Martin Hundeb�ll <martin@geanix.com>
> > > ---
> > > �...ename-TokenPool-Setup-to-SetupClient.patch | 113 ++++++++
> > > �...-jobserver-fifo-style-client-support.patch | 271
> > > ++++++++++++++++++
> > > �meta/recipes-devtools/ninja/ninja_1.11.1.bb�� |�� 8 +-
> > > �3 files changed, 390 insertions(+), 2 deletions(-)
> > > �create mode 100644 meta/recipes-devtools/ninja/files/0001-Rename-
> > > TokenPool-Setup-to-SetupClient.patch
> > > �create mode 100644 meta/recipes-devtools/ninja/files/0002-Add-GNU-
> > > make-jobserver-fifo-style-client-support.patch
> > > 
> > > diff --git a/meta/recipes-devtools/ninja/files/0001-Rename-
> > > TokenPool-Setup-to-SetupClient.patch b/meta/recipes-
> > > devtools/ninja/files/0001-Rename-TokenPool-Setup-to-
> > > SetupClient.patch
> > > new file mode 100644
> > > index 0000000000..a503f8c75f
> > > --- /dev/null
> > > +++ b/meta/recipes-devtools/ninja/files/0001-Rename-TokenPool-
> > > Setup-to-SetupClient.patch
> > > @@ -0,0 +1,113 @@
> > > +From f5642d8b49688dfc84679451b531d92f3b6e7cb0 Mon Sep 17 00:00:00
> > > 2001
> > > +From: Stefan Becker <chemobejk@gmail.com>
> > > +Date: Sat, 15 Dec 2018 19:29:42 +0200
> > > +Subject: [PATCH 1/2] Rename TokenPool::Setup() to SetupClient()
> > > +MIME-Version: 1.0
> > > +Content-Type: text/plain; charset=UTF-8
> > > +Content-Transfer-Encoding: 8bit
> > > +
> > > +Make space to add new API to set up token pool master.
> > > +
> > > +Signed-off-by: Martin Hundeb�ll <martin@geanix.com>
> > > +Upstream-Status: Submitted
> > > [https://github.com/Kitware/ninja/pull/2]
> > > +---
> > > + src/build.cc������������� | 6 +++---
> > > + src/subprocess_test.cc��� | 3 ++-
> > > + src/tokenpool-gnu-make.cc | 6 +++---
> > > + src/tokenpool-gnu-make.h� | 3 ++-
> > > + src/tokenpool.h���������� | 3 ++-
> > > + src/tokenpool_test.cc���� | 2 +-
> > > + 6 files changed, 13 insertions(+), 10 deletions(-)
> > > +
> > > +diff --git a/src/build.cc b/src/build.cc
> > > +index 53e4405..d8a6dff 100644
> > > +--- a/src/build.cc
> > > ++++ b/src/build.cc
> > > +@@ -474,9 +474,9 @@ struct RealCommandRunner : public
> > > CommandRunner {
> > > + RealCommandRunner::RealCommandRunner(const BuildConfig& config) :
> > > config_(config) {
> > > +�� max_load_average_ = config.max_load_average;
> > > +�� if ((tokens_ = TokenPool::Get()) != NULL) {
> > > +-��� if (!tokens_->Setup(config_.parallelism_from_cmdline,
> > > +-����������������������� config_.verbosity ==
> > > BuildConfig::VERBOSE,
> > > +-����������������������� max_load_average_)) {
> > > ++��� if (!tokens_->SetupClient(config_.parallelism_from_cmdline,
> > > ++����������������������������� config_.verbosity ==
> > > BuildConfig::VERBOSE,
> > > ++����������������������������� max_load_average_)) {
> > > +������ delete tokens_;
> > > +������ tokens_ = NULL;
> > > +���� }
> > > +diff --git a/src/subprocess_test.cc b/src/subprocess_test.cc
> > > +index eddc110..222b59b 100644
> > > +--- a/src/subprocess_test.cc
> > > ++++ b/src/subprocess_test.cc
> > > +@@ -40,7 +40,8 @@ struct TestTokenPool : public TokenPool {
> > > +�� void Reserve()���� {}
> > > +�� void Release()���� {}
> > > +�� void Clear()������ {}
> > > +-� bool Setup(bool ignore_unused, bool verbose, double&
> > > max_load_average) { return false; }
> > > ++� bool SetupClient(bool ignore_unused, bool verbose,
> > > ++������������������ double& max_load_average) { return false; }
> > > + 
> > > + #ifdef _WIN32
> > > +�� bool _token_available;
> > > +diff --git a/src/tokenpool-gnu-make.cc b/src/tokenpool-gnu-make.cc
> > > +index 60e0552..6fb72a6 100644
> > > +--- a/src/tokenpool-gnu-make.cc
> > > ++++ b/src/tokenpool-gnu-make.cc
> > > +@@ -28,9 +28,9 @@ GNUmakeTokenPool::GNUmakeTokenPool() :
> > > available_(1), used_(0) {
> > > + GNUmakeTokenPool::~GNUmakeTokenPool() {
> > > + }
> > > + 
> > > +-bool GNUmakeTokenPool::Setup(bool ignore,
> > > +-���������������������������� bool verbose,
> > > +-���������������������������� double& max_load_average) {
> > > ++bool GNUmakeTokenPool::SetupClient(bool ignore,
> > > ++���������������������������������� bool verbose,
> > > ++���������������������������������� double& max_load_average) {
> > > +�� const char* value = GetEnv("MAKEFLAGS");
> > > +�� if (!value)
> > > +���� return false;
> > > +diff --git a/src/tokenpool-gnu-make.h b/src/tokenpool-gnu-make.h
> > > +index c94cca5..f4ab8d7 100644
> > > +--- a/src/tokenpool-gnu-make.h
> > > ++++ b/src/tokenpool-gnu-make.h
> > > +@@ -24,7 +24,8 @@ struct GNUmakeTokenPool : public TokenPool {
> > > +�� virtual void Reserve();
> > > +�� virtual void Release();
> > > +�� virtual void Clear();
> > > +-� virtual bool Setup(bool ignore, bool verbose, double&
> > > max_load_average);
> > > ++� virtual bool SetupClient(bool ignore, bool verbose,
> > > ++�������������������������� double& max_load_average);
> > > + 
> > > +�� // platform specific implementation
> > > +�� virtual const char* GetEnv(const char* name) = 0;
> > > +diff --git a/src/tokenpool.h b/src/tokenpool.h
> > > +index 931c227..ce2bf48 100644
> > > +--- a/src/tokenpool.h
> > > ++++ b/src/tokenpool.h
> > > +@@ -26,7 +26,8 @@ struct TokenPool {
> > > +�� virtual void Clear() = 0;
> > > + 
> > > +�� // returns false if token pool setup failed
> > > +-� virtual bool Setup(bool ignore, bool verbose, double&
> > > max_load_average) = 0;
> > > ++� virtual bool SetupClient(bool ignore, bool verbose,
> > > ++�������������������������� double& max_load_average) = 0;
> > > + 
> > > + #ifdef _WIN32
> > > +�� virtual void WaitForTokenAvailability(HANDLE ioport) = 0;
> > > +diff --git a/src/tokenpool_test.cc b/src/tokenpool_test.cc
> > > +index 1bc1c84..9d07146 100644
> > > +--- a/src/tokenpool_test.cc
> > > ++++ b/src/tokenpool_test.cc
> > > +@@ -77,7 +77,7 @@ struct TokenPoolTest : public testing::Test {
> > > +������ ENVIRONMENT_INIT(buf_);
> > > +���� }
> > > +���� if ((tokens_ = TokenPool::Get()) != NULL) {
> > > +-����� if (!tokens_->Setup(ignore_jobserver, false, load_avg_)) {
> > > ++����� if (!tokens_->SetupClient(ignore_jobserver, false,
> > > load_avg_)) {
> > > +�������� delete tokens_;
> > > +�������� tokens_ = NULL;
> > > +������ }
> > > +-- 
> > > +2.44.0
> > > +
> > > diff --git a/meta/recipes-devtools/ninja/files/0002-Add-GNU-make-
> > > jobserver-fifo-style-client-support.patch b/meta/recipes-
> > > devtools/ninja/files/0002-Add-GNU-make-jobserver-fifo-style-client-
> > > support.patch
> > > new file mode 100644
> > > index 0000000000..770a53fbc9
> > > --- /dev/null
> > > +++ b/meta/recipes-devtools/ninja/files/0002-Add-GNU-make-
> > > jobserver-fifo-style-client-support.patch
> > > @@ -0,0 +1,271 @@
> > > +From 830b7fb396383171c5fac2aae7de880ac83ce955 Mon Sep 17 00:00:00
> > > 2001
> > > +From: Stefan Becker <chemobejk@gmail.com>
> > > +Date: Mon, 7 Nov 2022 20:45:27 +0200
> > > +Subject: [PATCH 2/2] Add GNU make jobserver fifo style client
> > > support
> > > +MIME-Version: 1.0
> > > +Content-Type: text/plain; charset=UTF-8
> > > +Content-Transfer-Encoding: 8bit
> > > +
> > > +GNU make 4.4 introduced a new jobserver style "fifo" for POSIX
> > > systems
> > > +which passes a named pipe down to the clients.
> > > +
> > > +- update auth parser to recognize "fifo:<name>" format
> > > +- open named pipe for reading and writing
> > > +- make sure the file descriptors are closed in the destructor
> > > +- add 2 tests that aren't compiled for WIN32
> > > +
> > > +Signed-off-by: Martin Hundeb�ll <martin@geanix.com>
> > > +Upstream-Status: Submitted
> > > [https://github.com/Kitware/ninja/pull/2]
> > > +---
> > > + src/tokenpool-gnu-make-posix.cc | 46 ++++++++++++++++-
> > > + src/tokenpool_test.cc���������� | 88
> > > ++++++++++++++++++++++++++++++---
> > > + 2 files changed, 127 insertions(+), 7 deletions(-)
> > > +
> > > +diff --git a/src/tokenpool-gnu-make-posix.cc b/src/tokenpool-gnu-
> > > make-posix.cc
> > > +index 8447070..a82cce4 100644
> > > +--- a/src/tokenpool-gnu-make-posix.cc
> > > ++++ b/src/tokenpool-gnu-make-posix.cc
> > > +@@ -40,6 +40,7 @@ struct GNUmakeTokenPoolPosix : public
> > > GNUmakeTokenPool {
> > > +� private:
> > > +�� int rfd_;
> > > +�� int wfd_;
> > > ++� bool closeFds_;
> > > + 
> > > +�� struct sigaction old_act_;
> > > +�� bool restore_;
> > > +@@ -48,14 +49,19 @@ struct GNUmakeTokenPoolPosix : public
> > > GNUmakeTokenPool {
> > > +�� static void CloseDupRfd(int signum);
> > > + 
> > > +�� bool CheckFd(int fd);
> > > ++� bool CheckFifo(const char* fifo);
> > > +�� bool SetAlarmHandler();
> > > + };
> > > + 
> > > +-GNUmakeTokenPoolPosix::GNUmakeTokenPoolPosix() : rfd_(-1), wfd_(-
> > > 1), restore_(false) {
> > > ++GNUmakeTokenPoolPosix::GNUmakeTokenPoolPosix() : rfd_(-1), wfd_(-
> > > 1), closeFds_(false), restore_(false) {
> > > + }
> > > + 
> > > + GNUmakeTokenPoolPosix::~GNUmakeTokenPoolPosix() {
> > > +�� Clear();
> > > ++� if (closeFds_) {
> > > ++��� close(wfd_);
> > > ++��� close(rfd_);
> > > ++� }
> > > +�� if (restore_)
> > > +���� sigaction(SIGALRM, &old_act_, NULL);
> > > + }
> > > +@@ -69,6 +75,36 @@ bool GNUmakeTokenPoolPosix::CheckFd(int fd) {
> > > +�� return true;
> > > + }
> > > + 
> > > ++bool GNUmakeTokenPoolPosix::CheckFifo(const char* fifo) {
> > > ++� // remove possible junk from end of fifo name
> > > ++� char *filename = strdup(fifo);
> > > ++� char *end;
> > > ++� if ((end = strchr(filename, ' ')) != NULL) {
> > > ++��� *end = '\0';
> > > ++� }
> > > ++
> > > ++� int rfd = open(filename, O_RDONLY);
> > > ++� if (rfd < 0) {
> > > ++��� free(filename);
> > > ++��� return false;
> > > ++� }
> > > ++
> > > ++� int wfd = open(filename, O_WRONLY);
> > > ++� if (wfd < 0) {
> > > ++��� close(rfd);
> > > ++��� free(filename);
> > > ++��� return false;
> > > ++� }
> > > ++
> > > ++� free(filename);
> > > ++
> > > ++� rfd_ = rfd;
> > > ++� wfd_ = wfd;
> > > ++� closeFds_ = true;
> > > ++
> > > ++� return true;
> > > ++}
> > > ++
> > > + int GNUmakeTokenPoolPosix::dup_rfd_ = -1;
> > > + 
> > > + void GNUmakeTokenPoolPosix::CloseDupRfd(int signum) {
> > > +@@ -89,6 +125,13 @@ bool GNUmakeTokenPoolPosix::SetAlarmHandler()
> > > {
> > > + }
> > > + 
> > > + bool GNUmakeTokenPoolPosix::ParseAuth(const char* jobserver) {
> > > ++� // check for jobserver-fifo style
> > > ++� const char* fifo;
> > > ++� if (((fifo = strstr(jobserver, "=fifo:")) != NULL) &&
> > > ++����� CheckFifo(fifo + 6))
> > > ++��� return SetAlarmHandler();
> > > ++
> > > ++� // check for legacy simple pipe style
> > > +�� int rfd = -1;
> > > +�� int wfd = -1;
> > > +�� if ((sscanf(jobserver, "%*[^=]=%d,%d", &rfd, &wfd) == 2) &&
> > > +@@ -100,6 +143,7 @@ bool GNUmakeTokenPoolPosix::ParseAuth(const
> > > char* jobserver) {
> > > +���� return true;
> > > +�� }
> > > + 
> > > ++� // some jobserver style we don't support
> > > +�� return false;
> > > + }
> > > + 
> > > +diff --git a/src/tokenpool_test.cc b/src/tokenpool_test.cc
> > > +index 9d07146..e15da6c 100644
> > > +--- a/src/tokenpool_test.cc
> > > ++++ b/src/tokenpool_test.cc
> > > +@@ -19,7 +19,10 @@
> > > + #ifdef _WIN32
> > > + #include <windows.h>
> > > + #else
> > > ++#include <fcntl.h>
> > > + #include <unistd.h>
> > > ++#include <sys/types.h>
> > > ++#include <sys/stat.h>
> > > + #endif
> > > + 
> > > + #include <stdio.h>
> > > +@@ -35,6 +38,8 @@
> > > + #define AUTH_FORMAT(tmpl)�� "foo " tmpl "=%d,%d bar"
> > > + #define ENVIRONMENT_CLEAR() unsetenv("MAKEFLAGS")
> > > + #define ENVIRONMENT_INIT(v) setenv("MAKEFLAGS", v, true)
> > > ++
> > > ++#define FIFO_NAME "ninja-test-tokenpool-fifo"
> > > + #endif
> > > + 
> > > + namespace {
> > > +@@ -60,11 +65,24 @@ struct TokenPoolTest : public testing::Test {
> > > +���� semaphore_name_ = SEMAPHORE_NAME;
> > > +���� if ((semaphore_ = CreateSemaphore(0, 0, 2, SEMAPHORE_NAME))
> > > == NULL)
> > > + #else
> > > ++��� if (mkfifo(FIFO_NAME, 0600) < 0) {
> > > ++����� ASSERT_TRUE(false);
> > > ++��� }
> > > ++
> > > +���� if (pipe(fds_) < 0)
> > > + #endif
> > > +������ ASSERT_TRUE(false);
> > > +�� }
> > > + 
> > > ++� void GetPool(bool ignore_jobserver) {
> > > ++��� if ((tokens_ = TokenPool::Get()) != NULL) {
> > > ++����� if (!tokens_->SetupClient(ignore_jobserver, false,
> > > load_avg_)) {
> > > ++������� delete tokens_;
> > > ++������� tokens_ = NULL;
> > > ++����� }
> > > ++��� }
> > > ++� }
> > > ++
> > > +�� void CreatePool(const char* format, bool ignore_jobserver =
> > > false) {
> > > +���� if (format) {
> > > +������ sprintf(buf_, format,
> > > +@@ -76,18 +94,30 @@ struct TokenPoolTest : public testing::Test {
> > > +������ );
> > > +������ ENVIRONMENT_INIT(buf_);
> > > +���� }
> > > +-��� if ((tokens_ = TokenPool::Get()) != NULL) {
> > > +-����� if (!tokens_->SetupClient(ignore_jobserver, false,
> > > load_avg_)) {
> > > +-������� delete tokens_;
> > > +-������� tokens_ = NULL;
> > > +-����� }
> > > +-��� }
> > > ++��� GetPool(ignore_jobserver);
> > > +�� }
> > > + 
> > > +�� void CreateDefaultPool() {
> > > +���� CreatePool(AUTH_FORMAT("--jobserver-auth"));
> > > +�� }
> > > + 
> > > ++#ifndef _WIN32
> > > ++� void CreateFifoPool() {
> > > ++��� // close simple pipe fds...
> > > ++��� close(fds_[0]);
> > > ++��� close(fds_[1]);
> > > ++
> > > ++��� // ... and open named pipe instead
> > > ++��� if ((fds_[0] = open(FIFO_NAME, O_RDONLY|O_NONBLOCK)) < 0)
> > > ++����� ASSERT_TRUE(false);
> > > ++��� if ((fds_[1] = open(FIFO_NAME, O_WRONLY)) < 0)
> > > ++����� ASSERT_TRUE(false);
> > > ++
> > > ++��� ENVIRONMENT_INIT("foo --jobserver-auth=fifo:" FIFO_NAME "
> > > bar");
> > > ++��� GetPool(false);
> > > ++� }
> > > ++#endif
> > > ++
> > > +�� virtual void TearDown() {
> > > +���� if (tokens_)
> > > +������ delete tokens_;
> > > +@@ -96,6 +126,7 @@ struct TokenPoolTest : public testing::Test {
> > > + #else
> > > +���� close(fds_[0]);
> > > +���� close(fds_[1]);
> > > ++��� unlink(FIFO_NAME);
> > > + #endif
> > > +���� ENVIRONMENT_CLEAR();
> > > +�� }
> > > +@@ -167,6 +198,15 @@ TEST_F(TokenPoolTest, MonitorFD) {
> > > + 
> > > +�� EXPECT_EQ(fds_[0], tokens_->GetMonitorFd());
> > > + }
> > > ++
> > > ++TEST_F(TokenPoolTest, MonitorFDFifo) {
> > > ++� CreateFifoPool();
> > > ++
> > > ++� ASSERT_NE(NULL, tokens_);
> > > ++� EXPECT_EQ(kLoadAverageDefault, load_avg_);
> > > ++
> > > ++� EXPECT_NE(-1, tokens_->GetMonitorFd());
> > > ++}
> > > + #endif
> > > + 
> > > + TEST_F(TokenPoolTest, ImplicitToken) {
> > > +@@ -226,6 +266,42 @@ TEST_F(TokenPoolTest, TwoTokens) {
> > > +�� EXPECT_TRUE(tokens_->Acquire());
> > > + }
> > > + 
> > > ++#ifndef _WIN32
> > > ++TEST_F(TokenPoolTest, TwoTokensFifo) {
> > > ++� CreateFifoPool();
> > > ++
> > > ++� ASSERT_NE(NULL, tokens_);
> > > ++� EXPECT_EQ(kLoadAverageDefault, load_avg_);
> > > ++
> > > ++� // implicit token
> > > ++� EXPECT_TRUE(tokens_->Acquire());
> > > ++� tokens_->Reserve();
> > > ++� EXPECT_FALSE(tokens_->Acquire());
> > > ++
> > > ++� // jobserver offers 2nd token
> > > ++� char test_tokens[1] = { '+' };
> > > ++� ASSERT_EQ(1u, write(fds_[1], test_tokens,
> > > sizeof(test_tokens)));
> > > ++� EXPECT_TRUE(tokens_->Acquire());
> > > ++� tokens_->Reserve();
> > > ++� EXPECT_FALSE(tokens_->Acquire());
> > > ++
> > > ++� // release 2nd token
> > > ++� tokens_->Release();
> > > ++� EXPECT_TRUE(tokens_->Acquire());
> > > ++
> > > ++� // release implicit token - must return 2nd token back to
> > > jobserver
> > > ++� tokens_->Release();
> > > ++� EXPECT_TRUE(tokens_->Acquire());
> > > ++
> > > ++� // there must be one token available
> > > ++� EXPECT_EQ(1u, read(fds_[0], buf_, sizeof(buf_)));
> > > ++� EXPECT_EQ(test_tokens[0], buf_[0]);
> > > ++
> > > ++� // implicit token
> > > ++� EXPECT_TRUE(tokens_->Acquire());
> > > ++}
> > > ++#endif
> > > ++
> > > + TEST_F(TokenPoolTest, Clear) {
> > > +�� CreateDefaultPool();
> > > + 
> > > +-- 
> > > +2.44.0
> > > +
> > > diff --git a/meta/recipes-devtools/ninja/ninja_1.11.1.bb
> > > b/meta/recipes-devtools/ninja/ninja_1.11.1.bb
> > > index 8e297ec4d4..dbff2e191a 100644
> > > --- a/meta/recipes-devtools/ninja/ninja_1.11.1.bb
> > > +++ b/meta/recipes-devtools/ninja/ninja_1.11.1.bb
> > > @@ -6,9 +6,13 @@ LIC_FILES_CHKSUM =
> > > "file://COPYING;md5=a81586a64ad4e476c791cda7e2f2c52e"
> > > �
> > > �DEPENDS = "re2c-native ninja-native"
> > > �
> > > -SRCREV = "a524bf3f6bacd1b4ad85d719eed2737d8562f27a"
> > > +SRCREV = "95dee2a91d96c409d54f9fa0b70ea9aa2bdf8e63"
> > > �
> > > -SRC_URI = "git://github.com/ninja-
> > > build/ninja.git;branch=release;protocol=https"
> > > +SRC_URI = " \
> > > +��� git://github.com/kitware/ninja.git;branch=kitware-staged-
> > > features;protocol=https \
> > > +��� file://0001-Rename-TokenPool-Setup-to-SetupClient.patch�\
> > > +���
> > > file://0002-Add-GNU-make-jobserver-fifo-style-client-support.patch�
> > > \
> > > +"
> > > �UPSTREAM_CHECK_GITTAGREGEX = "v(?P<pver>.*)"
> > > �
> > > �S = "${WORKDIR}/git"
> > > -- 
> > > 2.44.0
> > > 
> > 
> > > 
> > > 
> > > 
> > 
> > 
> 

> 
> -=-=-=-=-=-=-=-=-=-=-=-
> Links: You receive all messages sent to this group.
> View/Reply Online (#202897): https://lists.openembedded.org/g/openembedded-core/message/202897
> Mute This Topic: https://lists.openembedded.org/mt/105326967/3617179
> Group Owner: openembedded-core+owner@lists.openembedded.org
> Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [alexandre.belloni@bootlin.com]
> -=-=-=-=-=-=-=-=-=-=-=-
> 


-- 
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Re: [OE-core] [PATCH v2 3/5] ninja: build modified version with GNU Make jobserver support -> (jobserver, loadfactor, pressure and all that!)

[Randy MacLeod]

[-- Attachment #1: Type: text/plain, Size: 21128 bytes --]

On 2024-08-02 6:41 a.m., Alexandre Belloni wrote:
> On 02/08/2024 12:24:06+0200, Martin Hundeb?ll wrote:
>> Hi Alexandre,
>>
>> I see you keep carrying these patches. I'm still working with ninja
>> upstream to add jobserver support there[1]. I was planning to redo
>> these patches once ninja merges that. But I can do a respin with my
>> patches added to ninja if you like?
>>
>> [1]https://github.com/ninja-build/ninja/pull/2450
> I carry them so they get some testing, I can drop until you submit the
> new version.

Hi Martin, et. al.

I'm going to do some experimenting with the job server patches for a
talk that somehow got approved for the 2024.12 YP summit!

https://pretalx.com/yocto-project-summit-2024-12/talk/WGPUTX/

As expected, the patch doesn't apply with all that has changed since April.

Do you have a a branch that you are keeping updated.
If so can you same me some time otherwise, I can rebase and fix things up.

I'll of course give you all the credit for the real work.
I plan to mention the performance/behaviour improvements that you 
mentioned in your cover letter:

---

On build machines shared by multiple users, a single jobserver can be
shared between multiple builds (using the JOBSERVER_FIFO variable).
Running the above build in two different build directories at the same
time gives a ~12% improvement (43:17 -> 37:55).

Finally, the memory pressure from e.g. compiling multiple c++ based
projects is also reduced. In our case, a cloud based build machine (with
32 cores and 32GB RAM) fails to compile llvm-rust-native (in parallel to
nodejs) without the jobserver due to a lack of memory.

---

Is there anything else you'd like people to know about?

Does anyone else have jobserver, BB_PRESSURE_*, BB_LOADFACTOR_MAX use 
cases or benchmarks
that they'd like to share with the YP communtiy? All input is welcome 
and appreciated.

Thanks,

../Randy





>
>> On Thu, 2024-04-25 at 13:30 +0200, Alexandre Belloni wrote:
>>> Hello,
>>>
>>> This patch doesn't apply on master anymore because ninja has been
>>> updated.
>>>
>>>
>>> On 04/04/2024 13:16:11+0200, Martin Hundeb?ll wrote:
>>>> Ninja doesn't (yet) support the GNU Make jobserver out of the box,
>>>> but
>>>> there is a pull request adding that support[1]. Since that pull
>>>> request
>>>> (and its derived three-part pull requests) seem to be ignored by
>>>> upstream, kitware (creator/maintainer of cmake) has created a
>>>> fork[2]
>>>> only to carry the jobserver patches. Change the source uri to point
>>>> at
>>>> the kitware fork of ninja, and add two patches from the original
>>>> pull
>>>> request to also support the new-style fifo jobserver feature.
>>>>
>>>> Note that the kitware fork of ninja is also used by buildroot[3].
>>>>
>>>> [1]https://github.com/ninja-build/ninja/pull/2263
>>>> [2]https://github.com/Kitware/ninja
>>>> [3]
>>>> https://gitlab.com/buildroot.org/buildroot/-/blob/master/package/ninja/ninja.mk
>>>>
>>>> Signed-off-by: Martin Hundebøll<martin@geanix.com>
>>>> ---
>>>>   ...ename-TokenPool-Setup-to-SetupClient.patch | 113 ++++++++
>>>>   ...-jobserver-fifo-style-client-support.patch | 271
>>>> ++++++++++++++++++
>>>>   meta/recipes-devtools/ninja/ninja_1.11.1.bb   |   8 +-
>>>>   3 files changed, 390 insertions(+), 2 deletions(-)
>>>>   create mode 100644 meta/recipes-devtools/ninja/files/0001-Rename-
>>>> TokenPool-Setup-to-SetupClient.patch
>>>>   create mode 100644 meta/recipes-devtools/ninja/files/0002-Add-GNU-
>>>> make-jobserver-fifo-style-client-support.patch
>>>>
>>>> diff --git a/meta/recipes-devtools/ninja/files/0001-Rename-
>>>> TokenPool-Setup-to-SetupClient.patch b/meta/recipes-
>>>> devtools/ninja/files/0001-Rename-TokenPool-Setup-to-
>>>> SetupClient.patch
>>>> new file mode 100644
>>>> index 0000000000..a503f8c75f
>>>> --- /dev/null
>>>> +++ b/meta/recipes-devtools/ninja/files/0001-Rename-TokenPool-
>>>> Setup-to-SetupClient.patch
>>>> @@ -0,0 +1,113 @@
>>>> +From f5642d8b49688dfc84679451b531d92f3b6e7cb0 Mon Sep 17 00:00:00
>>>> 2001
>>>> +From: Stefan Becker<chemobejk@gmail.com>
>>>> +Date: Sat, 15 Dec 2018 19:29:42 +0200
>>>> +Subject: [PATCH 1/2] Rename TokenPool::Setup() to SetupClient()
>>>> +MIME-Version: 1.0
>>>> +Content-Type: text/plain; charset=UTF-8
>>>> +Content-Transfer-Encoding: 8bit
>>>> +
>>>> +Make space to add new API to set up token pool master.
>>>> +
>>>> +Signed-off-by: Martin Hundebøll<martin@geanix.com>
>>>> +Upstream-Status: Submitted
>>>> [https://github.com/Kitware/ninja/pull/2]
>>>> +---
>>>> + src/build.cc              | 6 +++---
>>>> + src/subprocess_test.cc    | 3 ++-
>>>> + src/tokenpool-gnu-make.cc | 6 +++---
>>>> + src/tokenpool-gnu-make.h  | 3 ++-
>>>> + src/tokenpool.h           | 3 ++-
>>>> + src/tokenpool_test.cc     | 2 +-
>>>> + 6 files changed, 13 insertions(+), 10 deletions(-)
>>>> +
>>>> +diff --git a/src/build.cc b/src/build.cc
>>>> +index 53e4405..d8a6dff 100644
>>>> +--- a/src/build.cc
>>>> ++++ b/src/build.cc
>>>> +@@ -474,9 +474,9 @@ struct RealCommandRunner : public
>>>> CommandRunner {
>>>> + RealCommandRunner::RealCommandRunner(const BuildConfig& config) :
>>>> config_(config) {
>>>> +   max_load_average_ = config.max_load_average;
>>>> +   if ((tokens_ = TokenPool::Get()) != NULL) {
>>>> +-    if (!tokens_->Setup(config_.parallelism_from_cmdline,
>>>> +-                        config_.verbosity ==
>>>> BuildConfig::VERBOSE,
>>>> +-                        max_load_average_)) {
>>>> ++    if (!tokens_->SetupClient(config_.parallelism_from_cmdline,
>>>> ++                              config_.verbosity ==
>>>> BuildConfig::VERBOSE,
>>>> ++                              max_load_average_)) {
>>>> +       delete tokens_;
>>>> +       tokens_ = NULL;
>>>> +     }
>>>> +diff --git a/src/subprocess_test.cc b/src/subprocess_test.cc
>>>> +index eddc110..222b59b 100644
>>>> +--- a/src/subprocess_test.cc
>>>> ++++ b/src/subprocess_test.cc
>>>> +@@ -40,7 +40,8 @@ struct TestTokenPool : public TokenPool {
>>>> +   void Reserve()     {}
>>>> +   void Release()     {}
>>>> +   void Clear()       {}
>>>> +-  bool Setup(bool ignore_unused, bool verbose, double&
>>>> max_load_average) { return false; }
>>>> ++  bool SetupClient(bool ignore_unused, bool verbose,
>>>> ++                   double& max_load_average) { return false; }
>>>> +
>>>> + #ifdef _WIN32
>>>> +   bool _token_available;
>>>> +diff --git a/src/tokenpool-gnu-make.cc b/src/tokenpool-gnu-make.cc
>>>> +index 60e0552..6fb72a6 100644
>>>> +--- a/src/tokenpool-gnu-make.cc
>>>> ++++ b/src/tokenpool-gnu-make.cc
>>>> +@@ -28,9 +28,9 @@ GNUmakeTokenPool::GNUmakeTokenPool() :
>>>> available_(1), used_(0) {
>>>> + GNUmakeTokenPool::~GNUmakeTokenPool() {
>>>> + }
>>>> +
>>>> +-bool GNUmakeTokenPool::Setup(bool ignore,
>>>> +-                             bool verbose,
>>>> +-                             double& max_load_average) {
>>>> ++bool GNUmakeTokenPool::SetupClient(bool ignore,
>>>> ++                                   bool verbose,
>>>> ++                                   double& max_load_average) {
>>>> +   const char* value = GetEnv("MAKEFLAGS");
>>>> +   if (!value)
>>>> +     return false;
>>>> +diff --git a/src/tokenpool-gnu-make.h b/src/tokenpool-gnu-make.h
>>>> +index c94cca5..f4ab8d7 100644
>>>> +--- a/src/tokenpool-gnu-make.h
>>>> ++++ b/src/tokenpool-gnu-make.h
>>>> +@@ -24,7 +24,8 @@ struct GNUmakeTokenPool : public TokenPool {
>>>> +   virtual void Reserve();
>>>> +   virtual void Release();
>>>> +   virtual void Clear();
>>>> +-  virtual bool Setup(bool ignore, bool verbose, double&
>>>> max_load_average);
>>>> ++  virtual bool SetupClient(bool ignore, bool verbose,
>>>> ++                           double& max_load_average);
>>>> +
>>>> +   // platform specific implementation
>>>> +   virtual const char* GetEnv(const char* name) = 0;
>>>> +diff --git a/src/tokenpool.h b/src/tokenpool.h
>>>> +index 931c227..ce2bf48 100644
>>>> +--- a/src/tokenpool.h
>>>> ++++ b/src/tokenpool.h
>>>> +@@ -26,7 +26,8 @@ struct TokenPool {
>>>> +   virtual void Clear() = 0;
>>>> +
>>>> +   // returns false if token pool setup failed
>>>> +-  virtual bool Setup(bool ignore, bool verbose, double&
>>>> max_load_average) = 0;
>>>> ++  virtual bool SetupClient(bool ignore, bool verbose,
>>>> ++                           double& max_load_average) = 0;
>>>> +
>>>> + #ifdef _WIN32
>>>> +   virtual void WaitForTokenAvailability(HANDLE ioport) = 0;
>>>> +diff --git a/src/tokenpool_test.cc b/src/tokenpool_test.cc
>>>> +index 1bc1c84..9d07146 100644
>>>> +--- a/src/tokenpool_test.cc
>>>> ++++ b/src/tokenpool_test.cc
>>>> +@@ -77,7 +77,7 @@ struct TokenPoolTest : public testing::Test {
>>>> +       ENVIRONMENT_INIT(buf_);
>>>> +     }
>>>> +     if ((tokens_ = TokenPool::Get()) != NULL) {
>>>> +-      if (!tokens_->Setup(ignore_jobserver, false, load_avg_)) {
>>>> ++      if (!tokens_->SetupClient(ignore_jobserver, false,
>>>> load_avg_)) {
>>>> +         delete tokens_;
>>>> +         tokens_ = NULL;
>>>> +       }
>>>> +--
>>>> +2.44.0
>>>> +
>>>> diff --git a/meta/recipes-devtools/ninja/files/0002-Add-GNU-make-
>>>> jobserver-fifo-style-client-support.patch b/meta/recipes-
>>>> devtools/ninja/files/0002-Add-GNU-make-jobserver-fifo-style-client-
>>>> support.patch
>>>> new file mode 100644
>>>> index 0000000000..770a53fbc9
>>>> --- /dev/null
>>>> +++ b/meta/recipes-devtools/ninja/files/0002-Add-GNU-make-
>>>> jobserver-fifo-style-client-support.patch
>>>> @@ -0,0 +1,271 @@
>>>> +From 830b7fb396383171c5fac2aae7de880ac83ce955 Mon Sep 17 00:00:00
>>>> 2001
>>>> +From: Stefan Becker<chemobejk@gmail.com>
>>>> +Date: Mon, 7 Nov 2022 20:45:27 +0200
>>>> +Subject: [PATCH 2/2] Add GNU make jobserver fifo style client
>>>> support
>>>> +MIME-Version: 1.0
>>>> +Content-Type: text/plain; charset=UTF-8
>>>> +Content-Transfer-Encoding: 8bit
>>>> +
>>>> +GNU make 4.4 introduced a new jobserver style "fifo" for POSIX
>>>> systems
>>>> +which passes a named pipe down to the clients.
>>>> +
>>>> +- update auth parser to recognize "fifo:<name>" format
>>>> +- open named pipe for reading and writing
>>>> +- make sure the file descriptors are closed in the destructor
>>>> +- add 2 tests that aren't compiled for WIN32
>>>> +
>>>> +Signed-off-by: Martin Hundebøll<martin@geanix.com>
>>>> +Upstream-Status: Submitted
>>>> [https://github.com/Kitware/ninja/pull/2]
>>>> +---
>>>> + src/tokenpool-gnu-make-posix.cc | 46 ++++++++++++++++-
>>>> + src/tokenpool_test.cc           | 88
>>>> ++++++++++++++++++++++++++++++---
>>>> + 2 files changed, 127 insertions(+), 7 deletions(-)
>>>> +
>>>> +diff --git a/src/tokenpool-gnu-make-posix.cc b/src/tokenpool-gnu-
>>>> make-posix.cc
>>>> +index 8447070..a82cce4 100644
>>>> +--- a/src/tokenpool-gnu-make-posix.cc
>>>> ++++ b/src/tokenpool-gnu-make-posix.cc
>>>> +@@ -40,6 +40,7 @@ struct GNUmakeTokenPoolPosix : public
>>>> GNUmakeTokenPool {
>>>> +  private:
>>>> +   int rfd_;
>>>> +   int wfd_;
>>>> ++  bool closeFds_;
>>>> +
>>>> +   struct sigaction old_act_;
>>>> +   bool restore_;
>>>> +@@ -48,14 +49,19 @@ struct GNUmakeTokenPoolPosix : public
>>>> GNUmakeTokenPool {
>>>> +   static void CloseDupRfd(int signum);
>>>> +
>>>> +   bool CheckFd(int fd);
>>>> ++  bool CheckFifo(const char* fifo);
>>>> +   bool SetAlarmHandler();
>>>> + };
>>>> +
>>>> +-GNUmakeTokenPoolPosix::GNUmakeTokenPoolPosix() : rfd_(-1), wfd_(-
>>>> 1), restore_(false) {
>>>> ++GNUmakeTokenPoolPosix::GNUmakeTokenPoolPosix() : rfd_(-1), wfd_(-
>>>> 1), closeFds_(false), restore_(false) {
>>>> + }
>>>> +
>>>> + GNUmakeTokenPoolPosix::~GNUmakeTokenPoolPosix() {
>>>> +   Clear();
>>>> ++  if (closeFds_) {
>>>> ++    close(wfd_);
>>>> ++    close(rfd_);
>>>> ++  }
>>>> +   if (restore_)
>>>> +     sigaction(SIGALRM, &old_act_, NULL);
>>>> + }
>>>> +@@ -69,6 +75,36 @@ bool GNUmakeTokenPoolPosix::CheckFd(int fd) {
>>>> +   return true;
>>>> + }
>>>> +
>>>> ++bool GNUmakeTokenPoolPosix::CheckFifo(const char* fifo) {
>>>> ++  // remove possible junk from end of fifo name
>>>> ++  char *filename = strdup(fifo);
>>>> ++  char *end;
>>>> ++  if ((end = strchr(filename, ' ')) != NULL) {
>>>> ++    *end = '\0';
>>>> ++  }
>>>> ++
>>>> ++  int rfd = open(filename, O_RDONLY);
>>>> ++  if (rfd < 0) {
>>>> ++    free(filename);
>>>> ++    return false;
>>>> ++  }
>>>> ++
>>>> ++  int wfd = open(filename, O_WRONLY);
>>>> ++  if (wfd < 0) {
>>>> ++    close(rfd);
>>>> ++    free(filename);
>>>> ++    return false;
>>>> ++  }
>>>> ++
>>>> ++  free(filename);
>>>> ++
>>>> ++  rfd_ = rfd;
>>>> ++  wfd_ = wfd;
>>>> ++  closeFds_ = true;
>>>> ++
>>>> ++  return true;
>>>> ++}
>>>> ++
>>>> + int GNUmakeTokenPoolPosix::dup_rfd_ = -1;
>>>> +
>>>> + void GNUmakeTokenPoolPosix::CloseDupRfd(int signum) {
>>>> +@@ -89,6 +125,13 @@ bool GNUmakeTokenPoolPosix::SetAlarmHandler()
>>>> {
>>>> + }
>>>> +
>>>> + bool GNUmakeTokenPoolPosix::ParseAuth(const char* jobserver) {
>>>> ++  // check for jobserver-fifo style
>>>> ++  const char* fifo;
>>>> ++  if (((fifo = strstr(jobserver, "=fifo:")) != NULL) &&
>>>> ++      CheckFifo(fifo + 6))
>>>> ++    return SetAlarmHandler();
>>>> ++
>>>> ++  // check for legacy simple pipe style
>>>> +   int rfd = -1;
>>>> +   int wfd = -1;
>>>> +   if ((sscanf(jobserver, "%*[^=]=%d,%d", &rfd, &wfd) == 2) &&
>>>> +@@ -100,6 +143,7 @@ bool GNUmakeTokenPoolPosix::ParseAuth(const
>>>> char* jobserver) {
>>>> +     return true;
>>>> +   }
>>>> +
>>>> ++  // some jobserver style we don't support
>>>> +   return false;
>>>> + }
>>>> +
>>>> +diff --git a/src/tokenpool_test.cc b/src/tokenpool_test.cc
>>>> +index 9d07146..e15da6c 100644
>>>> +--- a/src/tokenpool_test.cc
>>>> ++++ b/src/tokenpool_test.cc
>>>> +@@ -19,7 +19,10 @@
>>>> + #ifdef _WIN32
>>>> + #include <windows.h>
>>>> + #else
>>>> ++#include <fcntl.h>
>>>> + #include <unistd.h>
>>>> ++#include <sys/types.h>
>>>> ++#include <sys/stat.h>
>>>> + #endif
>>>> +
>>>> + #include <stdio.h>
>>>> +@@ -35,6 +38,8 @@
>>>> + #define AUTH_FORMAT(tmpl)   "foo " tmpl "=%d,%d bar"
>>>> + #define ENVIRONMENT_CLEAR() unsetenv("MAKEFLAGS")
>>>> + #define ENVIRONMENT_INIT(v) setenv("MAKEFLAGS", v, true)
>>>> ++
>>>> ++#define FIFO_NAME "ninja-test-tokenpool-fifo"
>>>> + #endif
>>>> +
>>>> + namespace {
>>>> +@@ -60,11 +65,24 @@ struct TokenPoolTest : public testing::Test {
>>>> +     semaphore_name_ = SEMAPHORE_NAME;
>>>> +     if ((semaphore_ = CreateSemaphore(0, 0, 2, SEMAPHORE_NAME))
>>>> == NULL)
>>>> + #else
>>>> ++    if (mkfifo(FIFO_NAME, 0600) < 0) {
>>>> ++      ASSERT_TRUE(false);
>>>> ++    }
>>>> ++
>>>> +     if (pipe(fds_) < 0)
>>>> + #endif
>>>> +       ASSERT_TRUE(false);
>>>> +   }
>>>> +
>>>> ++  void GetPool(bool ignore_jobserver) {
>>>> ++    if ((tokens_ = TokenPool::Get()) != NULL) {
>>>> ++      if (!tokens_->SetupClient(ignore_jobserver, false,
>>>> load_avg_)) {
>>>> ++        delete tokens_;
>>>> ++        tokens_ = NULL;
>>>> ++      }
>>>> ++    }
>>>> ++  }
>>>> ++
>>>> +   void CreatePool(const char* format, bool ignore_jobserver =
>>>> false) {
>>>> +     if (format) {
>>>> +       sprintf(buf_, format,
>>>> +@@ -76,18 +94,30 @@ struct TokenPoolTest : public testing::Test {
>>>> +       );
>>>> +       ENVIRONMENT_INIT(buf_);
>>>> +     }
>>>> +-    if ((tokens_ = TokenPool::Get()) != NULL) {
>>>> +-      if (!tokens_->SetupClient(ignore_jobserver, false,
>>>> load_avg_)) {
>>>> +-        delete tokens_;
>>>> +-        tokens_ = NULL;
>>>> +-      }
>>>> +-    }
>>>> ++    GetPool(ignore_jobserver);
>>>> +   }
>>>> +
>>>> +   void CreateDefaultPool() {
>>>> +     CreatePool(AUTH_FORMAT("--jobserver-auth"));
>>>> +   }
>>>> +
>>>> ++#ifndef _WIN32
>>>> ++  void CreateFifoPool() {
>>>> ++    // close simple pipe fds...
>>>> ++    close(fds_[0]);
>>>> ++    close(fds_[1]);
>>>> ++
>>>> ++    // ... and open named pipe instead
>>>> ++    if ((fds_[0] = open(FIFO_NAME, O_RDONLY|O_NONBLOCK)) < 0)
>>>> ++      ASSERT_TRUE(false);
>>>> ++    if ((fds_[1] = open(FIFO_NAME, O_WRONLY)) < 0)
>>>> ++      ASSERT_TRUE(false);
>>>> ++
>>>> ++    ENVIRONMENT_INIT("foo --jobserver-auth=fifo:" FIFO_NAME "
>>>> bar");
>>>> ++    GetPool(false);
>>>> ++  }
>>>> ++#endif
>>>> ++
>>>> +   virtual void TearDown() {
>>>> +     if (tokens_)
>>>> +       delete tokens_;
>>>> +@@ -96,6 +126,7 @@ struct TokenPoolTest : public testing::Test {
>>>> + #else
>>>> +     close(fds_[0]);
>>>> +     close(fds_[1]);
>>>> ++    unlink(FIFO_NAME);
>>>> + #endif
>>>> +     ENVIRONMENT_CLEAR();
>>>> +   }
>>>> +@@ -167,6 +198,15 @@ TEST_F(TokenPoolTest, MonitorFD) {
>>>> +
>>>> +   EXPECT_EQ(fds_[0], tokens_->GetMonitorFd());
>>>> + }
>>>> ++
>>>> ++TEST_F(TokenPoolTest, MonitorFDFifo) {
>>>> ++  CreateFifoPool();
>>>> ++
>>>> ++  ASSERT_NE(NULL, tokens_);
>>>> ++  EXPECT_EQ(kLoadAverageDefault, load_avg_);
>>>> ++
>>>> ++  EXPECT_NE(-1, tokens_->GetMonitorFd());
>>>> ++}
>>>> + #endif
>>>> +
>>>> + TEST_F(TokenPoolTest, ImplicitToken) {
>>>> +@@ -226,6 +266,42 @@ TEST_F(TokenPoolTest, TwoTokens) {
>>>> +   EXPECT_TRUE(tokens_->Acquire());
>>>> + }
>>>> +
>>>> ++#ifndef _WIN32
>>>> ++TEST_F(TokenPoolTest, TwoTokensFifo) {
>>>> ++  CreateFifoPool();
>>>> ++
>>>> ++  ASSERT_NE(NULL, tokens_);
>>>> ++  EXPECT_EQ(kLoadAverageDefault, load_avg_);
>>>> ++
>>>> ++  // implicit token
>>>> ++  EXPECT_TRUE(tokens_->Acquire());
>>>> ++  tokens_->Reserve();
>>>> ++  EXPECT_FALSE(tokens_->Acquire());
>>>> ++
>>>> ++  // jobserver offers 2nd token
>>>> ++  char test_tokens[1] = { '+' };
>>>> ++  ASSERT_EQ(1u, write(fds_[1], test_tokens,
>>>> sizeof(test_tokens)));
>>>> ++  EXPECT_TRUE(tokens_->Acquire());
>>>> ++  tokens_->Reserve();
>>>> ++  EXPECT_FALSE(tokens_->Acquire());
>>>> ++
>>>> ++  // release 2nd token
>>>> ++  tokens_->Release();
>>>> ++  EXPECT_TRUE(tokens_->Acquire());
>>>> ++
>>>> ++  // release implicit token - must return 2nd token back to
>>>> jobserver
>>>> ++  tokens_->Release();
>>>> ++  EXPECT_TRUE(tokens_->Acquire());
>>>> ++
>>>> ++  // there must be one token available
>>>> ++  EXPECT_EQ(1u, read(fds_[0], buf_, sizeof(buf_)));
>>>> ++  EXPECT_EQ(test_tokens[0], buf_[0]);
>>>> ++
>>>> ++  // implicit token
>>>> ++  EXPECT_TRUE(tokens_->Acquire());
>>>> ++}
>>>> ++#endif
>>>> ++
>>>> + TEST_F(TokenPoolTest, Clear) {
>>>> +   CreateDefaultPool();
>>>> +
>>>> +--
>>>> +2.44.0
>>>> +
>>>> diff --git a/meta/recipes-devtools/ninja/ninja_1.11.1.bb
>>>> b/meta/recipes-devtools/ninja/ninja_1.11.1.bb
>>>> index 8e297ec4d4..dbff2e191a 100644
>>>> --- a/meta/recipes-devtools/ninja/ninja_1.11.1.bb
>>>> +++ b/meta/recipes-devtools/ninja/ninja_1.11.1.bb
>>>> @@ -6,9 +6,13 @@ LIC_FILES_CHKSUM =
>>>> "file://COPYING;md5=a81586a64ad4e476c791cda7e2f2c52e"
>>>>   
>>>>   DEPENDS = "re2c-native ninja-native"
>>>>   
>>>> -SRCREV = "a524bf3f6bacd1b4ad85d719eed2737d8562f27a"
>>>> +SRCREV = "95dee2a91d96c409d54f9fa0b70ea9aa2bdf8e63"
>>>>   
>>>> -SRC_URI = "git://github.com/ninja-
>>>> build/ninja.git;branch=release;protocol=https"
>>>> +SRC_URI = " \
>>>> +    git://github.com/kitware/ninja.git;branch=kitware-staged-
>>>> features;protocol=https \
>>>> +file://0001-Rename-TokenPool-Setup-to-SetupClient.patch \
>>>> +
>>>> file://0002-Add-GNU-make-jobserver-fifo-style-client-support.patch 
>>>> \
>>>> +"
>>>>   UPSTREAM_CHECK_GITTAGREGEX = "v(?P<pver>.*)"
>>>>   
>>>>   S = "${WORKDIR}/git"
>>>> -- 
>>>> 2.44.0
>>>>
>>>>
>> -=-=-=-=-=-=-=-=-=-=-=-
>> Links: You receive all messages sent to this group.
>> View/Reply Online (#202897):https://lists.openembedded.org/g/openembedded-core/message/202897
>> Mute This Topic:https://lists.openembedded.org/mt/105326967/3617179
>> Group Owner:openembedded-core+owner@lists.openembedded.org
>> Unsubscribe:https://lists.openembedded.org/g/openembedded-core/unsub [alexandre.belloni@bootlin.com]
>> -=-=-=-=-=-=-=-=-=-=-=-
>>

-- 
# Randy MacLeod
# Wind River Linux

[-- Attachment #2: Type: text/html, Size: 25897 bytes --]

Re: [OE-core] [PATCH v2 3/5] ninja: build modified version with GNU Make jobserver support -> (jobserver, loadfactor, pressure and all that!)

[Martin Hundebøll]

Hi Randy,

On Sun, 2024-11-17 at 21:29 -0500, Randy MacLeod wrote:
> On 2024-08-02 6:41 a.m., Alexandre Belloni wrote:
> > On 02/08/2024 12:24:06+0200, Martin Hundeb?ll wrote:
> > > Hi Alexandre,
> > > 
> > > I see you keep carrying these patches. I'm still working with
> > > ninja
> > > upstream to add jobserver support there[1]. I was planning to
> > > redo
> > > these patches once ninja merges that. But I can do a respin with
> > > my
> > > patches added to ninja if you like?
> > > 
> > > [1] https://github.com/ninja-build/ninja/pull/2450
> > 
> > I carry them so they get some testing, I can drop until you submit
> > the new version.
> >   
> Hi Martin, et. al.
>  
> I'm going to do some experimenting with the job server patches for a 
>  talk that somehow got approved for the 2024.12 YP summit! 
>    https://pretalx.com/yocto-project-summit-2024-12/talk/WGPUTX/
> 


Thanks for picking this up. I got assigned to other projects, so this
was put on hold for now.

> As expected, the patch doesn't apply with all that has changed since
> April.
> 
> Do you have a a branch that you are keeping updated.
> If so can you same me some time otherwise, I can rebase and fix
> things up.

I have no branch, no. But I did try to get jobserver client support
into ninja. My PR got replaced by two other PRs, and I assume you are
following the progress on the latest one:
https://github.com/ninja-build/ninja/pull/2506

It looks like that PR will go into ninja eventually.

The qemu patches are no longer needed, so just the ninja patches, the
jobserver bbclass, and the python server script that are needed.

> I'll of course give you all the credit for the real work.
> I plan to mention the performance/behaviour improvements that you
> mentioned in your cover letter:
>  
> ---   
> On build machines shared by multiple users, a single jobserver can be
> shared between multiple builds (using the JOBSERVER_FIFO variable).
> Running the above build in two different build directories at the
> same time gives a ~12% improvement (43:17 -> 37:55).
> 
> Finally, the memory pressure from e.g. compiling multiple c++ based
> projects is also reduced. In our case, a cloud based build machine
> (with 32 cores and 32GB RAM) fails to compile llvm-rust-native (in
> parallel to nodejs) without the jobserver due to a lack of memory.
> ---
>  
> Is there anything else you'd like people to know about?

Feel free to include my comment wherever you see fit. The credit is
fine, but not important to me. Again, do what you think is appropriate.

> Does anyone else have jobserver, BB_PRESSURE_*, BB_LOADFACTOR_MAX use
> cases or benchmarks that they'd like to share with the YP communtiy?
> All input is welcome and appreciated.

There is a comment on the github PR that mentions yocto / bitbake:
https://github.com/ninja-build/ninja/pull/2506#issuecomment-2437267474

Good luck :)

// Martin

Re: [OE-core] [PATCH v2 3/5] ninja: build modified version with GNU Make jobserver support -> (jobserver, loadfactor, pressure and all that!)

[Randy MacLeod]

[-- Attachment #1: Type: text/plain, Size: 3503 bytes --]

On 2024-12-02 3:07 p.m., Martin Hundebøll wrote:
> Hi Randy,
>
> On Sun, 2024-11-17 at 21:29 -0500, Randy MacLeod wrote:
>> On 2024-08-02 6:41 a.m., Alexandre Belloni wrote:
>>> On 02/08/2024 12:24:06+0200, Martin Hundeb?ll wrote:
>>>> Hi Alexandre,
>>>>
>>>> I see you keep carrying these patches. I'm still working with
>>>> ninja
>>>> upstream to add jobserver support there[1]. I was planning to
>>>> redo
>>>> these patches once ninja merges that. But I can do a respin with
>>>> my
>>>> patches added to ninja if you like?
>>>>
>>>> [1]https://github.com/ninja-build/ninja/pull/2450
>>> I carry them so they get some testing, I can drop until you submit
>>> the new version.
>>>    
>> Hi Martin, et. al.
>>   
>> I'm going to do some experimenting with the job server patches for a
>>   talk that somehow got approved for the 2024.12 YP summit!
>>     https://pretalx.com/yocto-project-summit-2024-12/talk/WGPUTX/
>>
> Thanks for picking this up. I got assigned to other projects, so this
> was put on hold for now.

Hi Martin et. al.,

Errr, some things came up and I didn't feel that I had enough new to say so
I'm not presenting this week.


Hopefully as you say, the ninja patch merges and I can get time to do 
more evaluation
and comparison and one of us can get a patch merged to master.

If all goes well, there will be a presentation at the next YP Summit!

Thanks,

../Randy



>
>> As expected, the patch doesn't apply with all that has changed since
>> April.
>>
>> Do you have a a branch that you are keeping updated.
>> If so can you same me some time otherwise, I can rebase and fix
>> things up.
> I have no branch, no. But I did try to get jobserver client support
> into ninja. My PR got replaced by two other PRs, and I assume you are
> following the progress on the latest one:
> https://github.com/ninja-build/ninja/pull/2506
>
> It looks like that PR will go into ninja eventually.
>
> The qemu patches are no longer needed, so just the ninja patches, the
> jobserver bbclass, and the python server script that are needed.
>
>> I'll of course give you all the credit for the real work.
>> I plan to mention the performance/behaviour improvements that you
>> mentioned in your cover letter:
>>   
>> ---
>> On build machines shared by multiple users, a single jobserver can be
>> shared between multiple builds (using the JOBSERVER_FIFO variable).
>> Running the above build in two different build directories at the
>> same time gives a ~12% improvement (43:17 -> 37:55).
>>
>> Finally, the memory pressure from e.g. compiling multiple c++ based
>> projects is also reduced. In our case, a cloud based build machine
>> (with 32 cores and 32GB RAM) fails to compile llvm-rust-native (in
>> parallel to nodejs) without the jobserver due to a lack of memory.
>> ---
>>   
>> Is there anything else you'd like people to know about?
> Feel free to include my comment wherever you see fit. The credit is
> fine, but not important to me. Again, do what you think is appropriate.
>
>> Does anyone else have jobserver, BB_PRESSURE_*, BB_LOADFACTOR_MAX use
>> cases or benchmarks that they'd like to share with the YP communtiy?
>> All input is welcome and appreciated.
> There is a comment on the github PR that mentions yocto / bitbake:
> https://github.com/ninja-build/ninja/pull/2506#issuecomment-2437267474
>
> Good luck :)
>
> // Martin


-- 
# Randy MacLeod
# Wind River Linux

[-- Attachment #2: Type: text/html, Size: 5483 bytes --]

Re: [OE-core] [PATCH v2 3/5] ninja: build modified version with GNU Make jobserver support -> (jobserver, loadfactor, pressure and all that!)

[Martin Hundebøll]

On Mon, 2024-12-02 at 15:25 -0500, Randy MacLeod wrote:
> On 2024-12-02 3:07 p.m., Martin Hundebøll wrote: 
> > Hi Randy,
> > On Sun, 2024-11-17 at 21:29 -0500, Randy MacLeod wrote: 
> > > On 2024-08-02 6:41 a.m., Alexandre Belloni wrote:
> > > > On 02/08/2024 12:24:06+0200, Martin Hundeb?ll wrote:
> > > > > Hi Alexandre,
> > > > > 
> > > > > I see you keep carrying these patches. I'm still working with
> > > > > ninja
> > > > > upstream to add jobserver support there[1]. I was planning to
> > > > > redo
> > > > > these patches once ninja merges that. But I can do a respin
> > > > > with
> > > > > my
> > > > > patches added to ninja if you like?
> > > > > 
> > > > > [1] https://github.com/ninja-build/ninja/pull/2450
> > > > >  
> > > > I carry them so they get some testing, I can drop until you
> > > > submit
> > > > the new version.
> > > >   
> > > Hi Martin, et. al.
> > >  
> > > I'm going to do some experimenting with the job server patches
> > > for a 
> > >  talk that somehow got approved for the 2024.12 YP summit! 
> > >    https://pretalx.com/yocto-project-summit-2024-12/talk/WGPUTX/
> > >  
> > Thanks for picking this up. I got assigned to other projects, so
> > this was put on hold for now.
> >  
> Hi Martin et. al., 
> 
> Errr, some things came up and I didn't feel that I had enough new to
> say so I'm not presenting this week. 
> 
> Hopefully as you say, the ninja patch merges and I can get time to do
> more evaluation  and comparison and one of us can get a patch merged
> to master.
> 
> If all goes well, there will be a presentation at the next YP Summit!

Hi Randy,

No problem. We'll see if/when one of us get the time to look into this
again.

// Martin
> 
> > >  
> > > As expected, the patch doesn't apply with all that has changed
> > > since
> > > April.
> > > 
> > > Do you have a a branch that you are keeping updated.
> > > If so can you same me some time otherwise, I can rebase and fix
> > > things up.
> > >  
> >  
> > I have no branch, no. But I did try to get jobserver client support
> > into ninja. My PR got replaced by two other PRs, and I assume you
> > are
> > following the progress on the latest one:
> > https://github.com/ninja-build/ninja/pull/2506
> > 
> > It looks like that PR will go into ninja eventually.
> > 
> > The qemu patches are no longer needed, so just the ninja patches,
> > the
> > jobserver bbclass, and the python server script that are needed.
> > 
> >  
> > >  
> > > I'll of course give you all the credit for the real work.
> > > I plan to mention the performance/behaviour improvements that you
> > > mentioned in your cover letter:
> > >  
> > > ---   
> > > On build machines shared by multiple users, a single jobserver
> > > can be
> > > shared between multiple builds (using the JOBSERVER_FIFO
> > > variable).
> > > Running the above build in two different build directories at the
> > > same time gives a ~12% improvement (43:17 -> 37:55).
> > > 
> > > Finally, the memory pressure from e.g. compiling multiple c++
> > > based
> > > projects is also reduced. In our case, a cloud based build
> > > machine
> > > (with 32 cores and 32GB RAM) fails to compile llvm-rust-native
> > > (in
> > > parallel to nodejs) without the jobserver due to a lack of
> > > memory.
> > > ---
> > >  
> > > Is there anything else you'd like people to know about?
> > >  
> >  
> > Feel free to include my comment wherever you see fit. The credit is
> > fine, but not important to me. Again, do what you think is
> > appropriate.
> > 
> >  
> > >  
> > > Does anyone else have jobserver, BB_PRESSURE_*, BB_LOADFACTOR_MAX
> > > use
> > > cases or benchmarks that they'd like to share with the YP
> > > communtiy?
> > > All input is welcome and appreciated.
> > >  
> >  
> > There is a comment on the github PR that mentions yocto / bitbake:
> > https://github.com/ninja-build/ninja/pull/2506#issuecomment-2437267474
> > 
> > Good luck :)
> > 
> > // Martin
> >  
>  
> 
>  
>  

[PATCH v2 4/5] qemu: enable parallel builds when using the jobserver class

[Martin Hundebøll]

If the jobserver class is enabled, the PARALLEL_MAKE variable is unset in
favor of configuring a shared jobserver in the MAKEFLAGS variable. However,
the qemu makefile translates the missing `-j<N>` argument to `-j1` when
calling into meson / ninja. Add a patch to make the qemu makefile
consider the --jobserver-auth option too.

Signed-off-by: Martin Hundebøll <martin@geanix.com>
---
 meta/recipes-devtools/qemu/qemu.inc           |  1 +
 ...e-jobserver-auth-argument-when-calli.patch | 37 +++++++++++++++++++
 2 files changed, 38 insertions(+)
 create mode 100644 meta/recipes-devtools/qemu/qemu/0013-Makefile-preserve-jobserver-auth-argument-when-calli.patch

diff --git a/meta/recipes-devtools/qemu/qemu.inc b/meta/recipes-devtools/qemu/qemu.inc
index 4501f84c2b..1f86bf5a44 100644
--- a/meta/recipes-devtools/qemu/qemu.inc
+++ b/meta/recipes-devtools/qemu/qemu.inc
@@ -39,6 +39,7 @@ SRC_URI = "https://download.qemu.org/${BPN}-${PV}.tar.xz \
            file://0003-linux-user-Add-strace-for-shmat.patch \
            file://0004-linux-user-Rewrite-target_shmat.patch \
            file://0005-tests-tcg-Check-that-shmat-does-not-break-proc-self-.patch \
+           file://0013-Makefile-preserve-jobserver-auth-argument-when-calli.patch \
            file://CVE-2023-6683.patch \
            file://qemu-guest-agent.init \
            file://qemu-guest-agent.udev \
diff --git a/meta/recipes-devtools/qemu/qemu/0013-Makefile-preserve-jobserver-auth-argument-when-calli.patch b/meta/recipes-devtools/qemu/qemu/0013-Makefile-preserve-jobserver-auth-argument-when-calli.patch
new file mode 100644
index 0000000000..0dbc32eed0
--- /dev/null
+++ b/meta/recipes-devtools/qemu/qemu/0013-Makefile-preserve-jobserver-auth-argument-when-calli.patch
@@ -0,0 +1,37 @@
+From 730530c5f01e00cdc3754ebb8f3d7ff995f3376e Mon Sep 17 00:00:00 2001
+From: =?UTF-8?q?Martin=20Hundeb=C3=B8ll?= <martin@geanix.com>
+Date: Thu, 21 Sep 2023 10:57:45 +0200
+Subject: [PATCH] Makefile: preserve --jobserver-auth argument when calling
+ ninja
+MIME-Version: 1.0
+Content-Type: text/plain; charset=UTF-8
+Content-Transfer-Encoding: 8bit
+
+Qemu wraps its call to ninja in a Makefile. Since ninja, as opposed to
+make, utilizes all CPU cores by default, the qemu Makefile translates
+the absense of a `-jN` argument into `-j1`. This breaks jobserver
+functionality, so update the -jN mangling to take the --jobserver-auth
+argument into considerationa too.
+
+Signed-off-by: Martin Hundebøll <martin@geanix.com>
+Upstream-Status: Submitted [https://lists.nongnu.org/archive/html/qemu-devel/2024-04/msg00172.html]
+---
+ Makefile | 2 +-
+ 1 file changed, 1 insertion(+), 1 deletion(-)
+
+diff --git a/Makefile b/Makefile
+index 8f36990335..183756018f 100644
+--- a/Makefile
++++ b/Makefile
+@@ -142,7 +142,7 @@ MAKE.k = $(findstring k,$(firstword $(filter-out --%,$(MAKEFLAGS))))
+ MAKE.q = $(findstring q,$(firstword $(filter-out --%,$(MAKEFLAGS))))
+ MAKE.nq = $(if $(word 2, $(MAKE.n) $(MAKE.q)),nq)
+ NINJAFLAGS = $(if $V,-v) $(if $(MAKE.n), -n) $(if $(MAKE.k), -k0) \
+-        $(filter-out -j, $(lastword -j1 $(filter -l% -j%, $(MAKEFLAGS)))) \
++        $(or $(filter -l% -j%, $(MAKEFLAGS)), $(if $(filter --jobserver-auth=%, $(MAKEFLAGS)),, -j1)) \
+         -d keepdepfile
+ ninja-cmd-goals = $(or $(MAKECMDGOALS), all)
+ ninja-cmd-goals += $(foreach g, $(MAKECMDGOALS), $(.ninja-goals.$g))
+-- 
+2.44.0
+
-- 
2.44.0

[PATCH v2 5/5] contrib: add python service and systemd unit to run shared jobserver

[Martin Hundebøll]

For CI setups that might end up building multiple yocto builds in
parallel, a shared jobserver can reduce the total load of the system.
Setting up such a jobserver is simple, but it does require a process
hanging around to keep the jobserver fifo open (to avoid blocking token
requests).

Add a simple python script that creates such a jobserver fifo and waits
forever. Also add a systemd unit file to start the python service at
boot.

The systemd unit can be installed in $HOME/.config/systemd/user/, but
one might need to add a droplet config (i.e. `systemctl --user edit
jobserver.service`) to setup the PYTHONPATH variable to make the python
script loadable.

Signed-off-by: Martin Hundebøll <martin@geanix.com>
---
 contrib/jobserver/jobserver.py      | 78 +++++++++++++++++++++++++++++
 contrib/jobserver/jobserver.service | 10 ++++
 2 files changed, 88 insertions(+)
 create mode 100644 contrib/jobserver/jobserver.py
 create mode 100644 contrib/jobserver/jobserver.service

diff --git a/contrib/jobserver/jobserver.py b/contrib/jobserver/jobserver.py
new file mode 100644
index 0000000000..52f9711277
--- /dev/null
+++ b/contrib/jobserver/jobserver.py
@@ -0,0 +1,78 @@
+#!/usr/bin/env python3
+
+from pathlib import Path
+from threading import Event
+import argparse
+import os
+import shutil
+import signal
+
+resumed = Event()
+runtime_dir = os.environ.get("XDG_RUNTIME_DIR", "/run")
+
+def signal_handler(signum, _frame):
+    """Wait for an external signal to exit the process gracefully."""
+    resumed.set()
+
+
+def main(path, user, group, mode, jobs):
+    """Setup a fifo to use as jobserver shared between builds."""
+    try:
+        path.unlink(missing_ok=True)
+        os.mkfifo(path)
+        shutil.chown(path, user, group)
+        os.chmod(path, mode)
+    except (FileNotFoundError, PermissionError) as exc:
+        raise SystemExit(f"failed to create fifo: {path}: {exc.strerror}")
+
+    print(f"jobserver: {path}: {jobs} jobs")
+    fifo = os.open(path, os.O_RDWR)
+    os.write(fifo, b"+" * jobs)
+
+    print("jobserver: ready; waiting indefinitely")
+    signal.signal(signal.SIGTERM, signal_handler)
+    signal.signal(signal.SIGINT, signal_handler)
+    resumed.wait()
+
+    print("jobserver: exiting")
+    path.unlink()
+    os.close(fifo)
+
+
+if __name__ == "__main__":
+    parser = argparse.ArgumentParser(
+        prog='Make jobserver',
+        description='Simple application to instantiate a jobserver fifo and hang around',
+    )
+    parser.add_argument(
+        "--mode",
+        help="Permission to apply to jobserver fifo",
+        type=lambda v: int(v, 8),
+        default=0o0666,
+    )
+    parser.add_argument(
+        "--user",
+        help="Username or id to assign ownership of fifo to",
+        default=os.getuid(),
+    )
+    parser.add_argument(
+        "--group",
+        help="Groupname or id to assign ownership of fifo to",
+        default=os.getgid(),
+    )
+    parser.add_argument(
+        "path",
+        help="Path to jobserver fifo",
+        type=Path,
+        nargs='?',
+        default=f"{runtime_dir}/jobserver",
+    )
+    parser.add_argument(
+        "jobs",
+        help="Number of tokens to load jobserver with",
+        type=int,
+        nargs='?',
+        default=os.cpu_count(),
+    )
+    args = parser.parse_args()
+    main(args.path, args.user, args.group, args.mode, args.jobs)
diff --git a/contrib/jobserver/jobserver.service b/contrib/jobserver/jobserver.service
new file mode 100644
index 0000000000..bbc7167ac0
--- /dev/null
+++ b/contrib/jobserver/jobserver.service
@@ -0,0 +1,10 @@
+[Unit]
+Description=Shared jobserver fifo
+
+[Service]
+Type=simple
+Environment=PYTHONUNBUFFERED=1
+ExecStart=python jobserver.py
+
+[Install]
+WantedBy=multi-user.target
-- 
2.44.0