diff mbox series

[RFC,1/5] classes: jobserver: support gnu make fifo jobserver

Message ID 20230828124834.376779-1-martin@geanix.com
State New
Headers show
Series [RFC,1/5] classes: jobserver: support gnu make fifo jobserver | expand

Commit Message

Martin Hundebøll Aug. 28, 2023, 12:48 p.m. UTC
Add a class to implement the gnu make fifo style jobserver. The class
can be activated by symply adding an `INHERIT += "jobserver"` to the
local configuration.

Furthermore, one can configure an external jobserver (i.e. a server
shared between multiple builds), by configured 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 make 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 | 80 +++++++++++++++++++++++++++
 meta/conf/bitbake.conf                |  2 +-
 2 files changed, 81 insertions(+), 1 deletion(-)
 create mode 100644 meta/classes-global/jobserver.bbclass

Comments

Martin Hundebøll Aug. 28, 2023, 5:36 p.m. UTC | #1
Aug 28, 2023 14:48:46 Martin Hundebøll <martin@geanix.com>:

> Add a class to implement the gnu make fifo style jobserver. The class
> can be activated by symply adding an `INHERIT += "jobserver"` to the
> local configuration.
>
> Furthermore, one can configure an external jobserver (i.e. a server
> shared between multiple builds), by configured 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 make 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 | 80 +++++++++++++++++++++++++++
> meta/conf/bitbake.conf                |  2 +-
> 2 files changed, 81 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..c76909fe50
> --- /dev/null
> +++ b/meta/classes-global/jobserver.bbclass
> @@ -0,0 +1,80 @@
> +JOBSERVER_FIFO ?= ""
> +JOBSERVER_FIFO[doc] = "Path to external jobserver fifo to use instead of creating a per-build server."
> +
> +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"
> +
> +    # and 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
> +
> +    # 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")

I would like some feedback on this part, i.e. changing package dependencies depending on a pure build-configuration like this. I would prefer if the build didn't change at all when enabling the jobserver class, which would require adding virtual/make-native to the base dependencies regardless of this class. Would that be acceptable?

> +    # 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 joberserver 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 cf7ff3328c..8cf188270b 100644
> --- a/meta/conf/bitbake.conf
> +++ b/meta/conf/bitbake.conf
> @@ -946,7 +946,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.41.0
Randy MacLeod Nov. 17, 2023, 10:18 p.m. UTC | #2
Hi Martin,

I tested this RFC series briefly when it was posted but then some things 
came up and I haven't gotten back to it.

I was talking with Richard and he thought that his original poky-contrib 
branch commit, which it seems this work is based,
had a list of things to fix before the jobserver could be merged. Can 
you find and share that list here?

I may have time to do more testing but I think I had concluded that the 
default settings:
    INHERIT += "jobserver"
didn't have any impact on build performance, as expected, I suppose.

What testing have you done so far?

../Randy


On 2023-08-28 8:48 a.m., Martin Hundeb?ll via lists.openembedded.org wrote:
> Add a class to implement the gnu make fifo style jobserver. The class
> can be activated by symply adding an `INHERIT += "jobserver"` to the
> local configuration.
>
> Furthermore, one can configure an external jobserver (i.e. a server
> shared between multiple builds), by configured 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 make 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 | 80 +++++++++++++++++++++++++++
>   meta/conf/bitbake.conf                |  2 +-
>   2 files changed, 81 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..c76909fe50
> --- /dev/null
> +++ b/meta/classes-global/jobserver.bbclass
> @@ -0,0 +1,80 @@
> +JOBSERVER_FIFO ?= ""
> +JOBSERVER_FIFO[doc] = "Path to external jobserver fifo to use instead of creating a per-build server."
> +
> +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"
> +
> +    # and 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
> +
> +    # 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 joberserver 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 cf7ff3328c..8cf188270b 100644
> --- a/meta/conf/bitbake.conf
> +++ b/meta/conf/bitbake.conf
> @@ -946,7 +946,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 \
>
> -=-=-=-=-=-=-=-=-=-=-=-
> Links: You receive all messages sent to this group.
> View/Reply Online (#186824):https://lists.openembedded.org/g/openembedded-core/message/186824
> Mute This Topic:https://lists.openembedded.org/mt/101009092/3616765
> Group Owner:openembedded-core+owner@lists.openembedded.org
> Unsubscribe:https://lists.openembedded.org/g/openembedded-core/unsub  [randy.macleod@windriver.com]
> -=-=-=-=-=-=-=-=-=-=-=-
>
diff mbox series

Patch

diff --git a/meta/classes-global/jobserver.bbclass b/meta/classes-global/jobserver.bbclass
new file mode 100644
index 0000000000..c76909fe50
--- /dev/null
+++ b/meta/classes-global/jobserver.bbclass
@@ -0,0 +1,80 @@ 
+JOBSERVER_FIFO ?= ""
+JOBSERVER_FIFO[doc] = "Path to external jobserver fifo to use instead of creating a per-build server."
+
+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"
+
+    # and 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
+
+    # 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 joberserver 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 cf7ff3328c..8cf188270b 100644
--- a/meta/conf/bitbake.conf
+++ b/meta/conf/bitbake.conf
@@ -946,7 +946,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 \