Hi,
make has the concept of 'expensive' jobs: if the command line includes
the word "make" in it, the command is considered expensive and other
jobs are held until that job finishes. It does this to avoid exponential
behavior when parallelizing with -j.
Some other makes (eg. bmake and gmake) use a jobserver, ie. a pair of
pipes between child makes and the top-level make: the top-level make
provides job tokens to children via these pipes, which are returned when
the jobs complete. This ensures that recursive makes only run a certain
number of jobs in total at any one time, while achieving greater
resource utilisation (no need to wait for an 'expensive' job to finish
before starting others).
This diff implements a similar jobserver in make. Like bmake and gmake,
job tokens are provided by the top-level make (called "jobserver
master"), used by children ("jobserver slaves") when they run commands,
and returned when those commands finish. Like bmake, the internal,
undocumented command line option to tell (possibly indirect, via
MAKEFLAGS)) children about the file descriptor to use is -J, but unlike
bmake and gmake, we use socketpair(AF_UNIX, SOCK_STREAM) instead of
pipes, and therefore only need to pass one file descriptor to children
instead of two.
I haven't yet built the entire tree with this, just cautiously probing
the waters first to see if there is interest :)
Tested with the following Makefile structure:
$ cat recursive/Makefile
.PHONY: a b c d
all: a b c d
a:
make -C a
b:
make -C a
c:
make -C a
d:
make -C a
$ cat recursive/a/Makefile
all: 1 2 3 4
1:
sleep 1
2:
sleep 2
3:
sleep 3
4:
sleep 4
the speedup is as follows with -j4:
before:
make -C recursive -j4 0.01s user 0.10s system 0% cpu 16.097 total
after:
make -C recursive -j4 0.02s user 0.15s system 1% cpu 11.082 total
---
usr.bin/make/job.c | 317 +++++++++++++++++++++++++++++++++++---------
usr.bin/make/job.h | 7 +-
usr.bin/make/main.c | 31 ++++-
usr.bin/make/main.h | 3 +
4 files changed, 290 insertions(+), 68 deletions(-)
diff --git a/usr.bin/make/job.c b/usr.bin/make/job.c
index ebbae5306da..b36aaeacac8 100644
--- a/usr.bin/make/job.c
+++ b/usr.bin/make/job.c
@@ -90,11 +90,15 @@
* Job_Wait Wait for all running jobs to finish.
*/
+#include <sys/socket.h>
#include <sys/types.h>
#include <sys/wait.h>
+#include <assert.h>
#include <ctype.h>
#include <errno.h>
+#include <err.h>
#include <fcntl.h>
+#include <poll.h>
#include <signal.h>
#include <stdarg.h>
#include <stdio.h>
@@ -115,6 +119,7 @@
#include "memory.h"
#include "make.h"
#include "buf.h"
+#include "main.h"
static int aborting = 0; /* why is the make aborting? */
#define ABORT_ERROR 1 /* Because of an error */
@@ -123,12 +128,15 @@ static int aborting = 0; /* why is the make
aborting? */
static int maxJobs; /* The most children we can run at once */
static int nJobs; /* Number of jobs already allocated */
-static bool no_new_jobs; /* Mark recursive shit so we shouldn't start
- * something else at the same time
- */
+
+struct jobserver {
+ int master; /* master deposits job tokens into this fd */
+ int slave; /* slaves consume job tokens from this fd */
+ volatile sig_atomic_t tokens; /* currently available tokens */
+};
+static struct jobserver jobserver;
Job *runningJobs; /* Jobs currently running a process */
Job *errorJobs; /* Jobs in error at end */
-static Job *heldJobs; /* Jobs not running yet because of expensive */
static pid_t mypid; /* Used for printing debugging messages */
static volatile sig_atomic_t got_fatal;
@@ -144,11 +152,16 @@ static void postprocess_job(Job *, bool);
static Job *prepare_job(GNode *);
static void determine_job_next_step(Job *);
static void remove_job(Job *, bool);
-static void may_continue_job(Job *);
static void continue_job(Job *);
static Job *reap_finished_job(pid_t);
static bool reap_jobs(void);
+static void jobserver_init(unsigned, int);
+static bool jobserver_is_slave(void);
+static void jobserver_master_reclaim_tokens(void);
+static void jobserver_master_send_tokens(void);
+static void jobserver_acquire_token(Job *job);
+static void jobserver_release_token(Job *job);
static void loop_handle_running_jobs(void);
static bool expensive_job(Job *);
static bool expensive_command(const char *);
@@ -347,10 +360,9 @@ print_errors(void)
static void
setup_signal(int sig)
{
- if (signal(sig, SIG_IGN) != SIG_IGN) {
- (void)signal(sig, notice_signal);
- sigaddset(&sigset, sig);
- }
+ sigaction(sig, &(struct sigaction) { .sa_handler = notice_signal },
+ NULL);
+ sigaddset(&sigset, sig);
}
static void
@@ -551,20 +563,15 @@ postprocess_job(Job *job, bool okay)
Finish();
}
-/* expensive jobs handling: in order to avoid forking an exponential number
- * of jobs, make tries to figure out "recursive make" configurations.
- * It may err on the side of caution.
+/* expensive jobs handling: make tries to figure out "recursive make"
+ * configurations. It may err on the side of caution.
* Basically, a command is "expensive" if it's likely to fork an extra
* level of make: either by looking at the command proper, or if it has
* some specific qualities ('+cmd' are likely to be recursive, as are
* .MAKE: commands). It's possible to explicitly say some targets are
* expensive or cheap with .EXPENSIVE or .CHEAP.
*
- * While an expensive command is running, no_new_jobs
- * is set, so jobs that would fork new processes are accumulated in the
- * heldJobs list instead.
- *
- * This heuristics is also used on error exit: we display silent commands
+ * This heuristics is used on error exit: we display silent commands
* that failed, unless those ARE expensive commands: expensive commands
* are likely to not be failing by themselves, but to be the result of
* a cascade of failures in descendant makes.
@@ -574,7 +581,6 @@ determine_expensive_job(Job *job)
{
if (expensive_job(job)) {
job->flags |= JOB_IS_EXPENSIVE;
- no_new_jobs = true;
} else
job->flags &= ~JOB_IS_EXPENSIVE;
if (DEBUG(EXPENSIVE))
@@ -672,23 +678,14 @@ prepare_job(GNode *gn)
}
}
-static void
-may_continue_job(Job *job)
-{
- if (no_new_jobs) {
- if (DEBUG(EXPENSIVE))
- fprintf(stderr, "[%ld] expensive -> hold %s\n",
- (long)mypid, job->node->name);
- job->next = heldJobs;
- heldJobs = job;
- } else
- continue_job(job);
-}
-
static void
continue_job(Job *job)
{
- bool finished = job_run_next(job);
+ bool finished;
+
+ jobserver_acquire_token(job);
+
+ finished = job_run_next(job);
if (finished)
remove_job(job, true);
else
@@ -715,27 +712,19 @@ Job_Make(GNode *gn)
if (!job)
return;
nJobs++;
- may_continue_job(job);
+ continue_job(job);
}
static void
determine_job_next_step(Job *job)
{
bool okay;
- if (job->flags & JOB_IS_EXPENSIVE) {
- no_new_jobs = false;
- if (DEBUG(EXPENSIVE))
- fprintf(stderr, "[%ld] "
- "Returning from expensive target %s, "
- "allowing new jobs\n", (long)mypid,
- job->node->name);
- }
okay = job->exit_type == JOB_EXIT_OKAY;
if (!okay || job->next_cmd == NULL)
remove_job(job, okay);
else
- may_continue_job(job);
+ continue_job(job);
}
static void
@@ -743,17 +732,6 @@ remove_job(Job *job, bool okay)
{
nJobs--;
postprocess_job(job, okay);
- while (!no_new_jobs) {
- if (heldJobs != NULL) {
- job = heldJobs;
- heldJobs = heldJobs->next;
- if (DEBUG(EXPENSIVE))
- fprintf(stderr, "[%ld] cheap -> release %s\n",
- (long)mypid, job->node->name);
- continue_job(job);
- } else
- break;
- }
}
/*
@@ -793,6 +771,7 @@ reap_jobs(void)
while ((pid = waitpid(WAIT_ANY, &status, WNOHANG)) > 0) {
reaped = true;
job = reap_finished_job(pid);
+ jobserver_release_token(job);
if (job == NULL) {
Punt("Child (%ld) not in table?", (long)pid);
@@ -810,6 +789,16 @@ reap_jobs(void)
void
handle_running_jobs(void)
{
+ /*
+ * If we are the jobserver master and all jobs have already been
+ * started, close the inheritable slave socket if open; this way the
+ * master socket will become disconnected once all children have died.
+ */
+ if (no_jobs_left() && jobserver.master != -1 && jobserver.slave != -1) {
+ close(jobserver.slave);
+ jobserver.slave = -1;
+ }
+
sigset_t old;
/* reaping children in the presence of caught signals */
@@ -830,10 +819,33 @@ handle_running_jobs(void)
handle_all_signals();
if (reap_jobs())
break;
- /* okay, so it's safe to suspend, we have nothing to do but
- * wait...
- */
- sigsuspend(&emptyset);
+ if (jobserver_is_slave()) {
+ /* okay, so it's safe to suspend, we have nothing to do
+ * but wait...
+ */
+ sigsuspend(&emptyset);
+ }
+ else {
+ struct pollfd pfd = {
+ .fd = jobserver.master,
+ .events = POLLIN,
+ };
+ int r;
+
+ if (jobserver.tokens > 0)
+ pfd.events |= POLLOUT;
+
+ /* Wait for a signal or for a slave to communicate over
+ * the jobserver socket */
+ r = ppoll(&pfd, 1, NULL, &emptyset);
+ if (r < 0 && errno != EINTR)
+ err(1, "jobserver master ppoll");
+
+ if (pfd.revents & POLLIN)
+ jobserver_master_reclaim_tokens();
+ if (pfd.revents & POLLOUT)
+ jobserver_master_send_tokens();
+ }
}
sigprocmask(SIG_SETMASK, &old, NULL);
}
@@ -859,20 +871,200 @@ handle_one_job(Job *job)
}
static void
-loop_handle_running_jobs()
+loop_handle_running_jobs(void)
{
while (runningJobs != NULL)
handle_running_jobs();
}
+int
+jobserver_get_slave_fd(void)
+{
+ return jobserver.slave;
+}
+
+static void
+jobserver_init(unsigned maxtokens, int fd)
+{
+ int sock[2];
+
+ if (fd != -1) {
+ /* received fd from master via -J */
+ jobserver.master = -1;
+ jobserver.slave = fd;
+ return;
+ }
+
+ /* we are the master; create sockets */
+ if (socketpair(AF_UNIX, SOCK_STREAM, 0, sock) < 0)
+ err(1, "could not create jobserver sockets");
+ jobserver.master = sock[0];
+ jobserver.slave = sock[1];
+ jobserver.tokens = maxtokens;
+
+ if (fcntl(jobserver.master, F_SETFD, O_CLOEXEC) < 0)
+ err(1, "could not set jobserver master fd flags");
+}
+
+static bool
+jobserver_is_slave(void)
+{
+ return jobserver.master == -1 && jobserver.slave != -1;
+}
+
+static void
+jobserver_master_reclaim_tokens(void)
+{
+ ssize_t r;
+ char tok[64];
+ unsigned reclaimed;
+
+ assert(!jobserver_is_slave());
+
+ do {
+ if ((r = recv(jobserver.master, tok, sizeof(tok),
+ MSG_DONTWAIT)) < 0) {
+ if (errno == EAGAIN)
+ break;
+ err(1, "master could not reclaim tokens");
+ }
+ jobserver.tokens += r;
+ reclaimed += r;
+ } while (r > 0);
+
+ if (DEBUG(EXPENSIVE) && reclaimed > 0)
+ fprintf(stderr, "[%ld] master: reclaimed %u job tokens, %u "
+ "available\n", (long)mypid, reclaimed, jobserver.tokens);
+}
+
+static void
+jobserver_master_send_tokens(void)
+{
+ ssize_t r;
+
+ assert(!jobserver_is_slave());
+
+ while (jobserver.tokens > 0) {
+ if ((r = send(jobserver.master, ".", 1, MSG_NOSIGNAL)) < 0) {
+ if (errno == EPIPE) {
+ if (DEBUG(EXPENSIVE))
+ fprintf(stderr, "[%ld] no job "
+ "token consumers left\n",
+ (long)mypid);
+ return;
+ }
+ err(1, "master could not forward token");
+ }
+ if (r == 0) {
+ fprintf(stderr, "[%ld] master token not consumed\n",
+ (long)mypid);
+ break;
+ }
+ jobserver.tokens--;
+ }
+}
+
void
-Job_Init(int maxproc)
+jobserver_acquire_token(Job *job)
+{
+ ssize_t r;
+ char c;
+
+ if (!jobserver_is_slave()) {
+ /*
+ * This job will be a direct child of the master (this
+ * process). No need to communicate over sockets - just
+ * decrement the number of tokens.
+ *
+ * NOTE: we should always have a token here, since
+ * can_start_job must have returned true.
+ */
+ assert(jobserver.tokens > 0);
+ jobserver.tokens--;
+ if (DEBUG(EXPENSIVE))
+ fprintf(stderr, "[%ld] master: target %s: used token, "
+ "%u remaining\n", (long)mypid, job->node->name,
+ jobserver.tokens);
+ return;
+ }
+
+ if (nJobs == 1) {
+ /*
+ * The master has already allocated a job token for the job
+ * that eventually ran this child make. Use that same token for
+ * the first child job; a make process that just waits for a
+ * child should not consume a token.
+ */
+ if (DEBUG(EXPENSIVE))
+ fprintf(stderr, "[%ld] slave: target %s: no job token "
+ "needed\n", (long)mypid, job->node->name);
+ return;
+ }
+
+ if (DEBUG(EXPENSIVE))
+ fprintf(stderr, "[%ld] slave: target %s: waiting for job "
+ "token\n", (long)mypid, job->node->name);
+recv:
+ if ((r = recv(jobserver.slave, &c, 1, 0)) < 0) {
+ if (errno == EINTR) {
+ handle_running_jobs();
+ goto recv;
+ }
+ err(1, "could not recv jobserver token");
+ }
+ assert(r == 1);
+
+ if (DEBUG(EXPENSIVE))
+ fprintf(stderr, "[%ld] slave: target %s: acquired job token\n",
+ (long)mypid, job->node->name);
+}
+
+
+static void
+jobserver_release_token(Job *job)
+{
+ ssize_t r;
+
+ if (!jobserver_is_slave()) {
+ jobserver.tokens++;
+ return;
+ }
+
+ if (nJobs == 1) {
+ /*
+ * The first job did not need a token (see
+ * jobserver_acquire_token), so do not release a token for it
+ * either.
+ */
+ if (DEBUG(EXPENSIVE))
+ fprintf(stderr, "[%ld] slave: target %s: no token "
+ "release needed\n", (long)mypid, job->node->name);
+ return;
+ }
+
+ if (DEBUG(EXPENSIVE))
+ fprintf(stderr, "[%ld] slave: target %s: releasing job "
+ "token\n", (long)mypid, job->node->name);
+
+send:
+ if ((r = send(jobserver.slave, ".", 1, 0)) < 0) {
+ if (errno == EINTR) {
+ handle_running_jobs();
+ goto send;
+ }
+ err(1, "could not release jobserver token");
+ }
+ assert(r == 1);
+}
+
+void
+Job_Init(int maxproc, int fd)
{
runningJobs = NULL;
- heldJobs = NULL;
errorJobs = NULL;
maxJobs = maxproc;
mypid = getpid();
+ jobserver_init(maxproc, fd);
nJobs = 0;
@@ -885,8 +1077,9 @@ can_start_job(void)
{
if (aborting || nJobs >= maxJobs)
return false;
- else
- return true;
+ if (!jobserver_is_slave())
+ return jobserver.tokens > 0;
+ return true;
}
bool
diff --git a/usr.bin/make/job.h b/usr.bin/make/job.h
index e8152d18fd0..0a047e54673 100644
--- a/usr.bin/make/job.h
+++ b/usr.bin/make/job.h
@@ -49,10 +49,10 @@
* register a new job running commands associated with building gn.
*/
extern void Job_Make(GNode *);
-/* Job_Init(maxproc);
+/* Job_Init(maxproc, jobserverfd);
* setup job handling framework
*/
-extern void Job_Init(int);
+extern void Job_Init(int, int);
/* interface with the normal build in make.c */
/* okay = can_start_job();
@@ -79,6 +79,9 @@ extern void Job_Wait(void);
extern void Job_AbortAll(void);
extern void print_errors(void);
+/* return the jobserver communication fd that should be used by slaves */
+extern int jobserver_get_slave_fd(void);
+
/* handle_running_jobs();
* wait until something happens, like a job finishing running a command
* or a signal coming in.
diff --git a/usr.bin/make/main.c b/usr.bin/make/main.c
index 2eef3a2a040..257124fc317 100644
--- a/usr.bin/make/main.c
+++ b/usr.bin/make/main.c
@@ -41,6 +41,7 @@
#include <sys/utsname.h>
#include <err.h>
#include <errno.h>
+#include <fcntl.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
@@ -67,8 +68,6 @@
#include "make.h"
#include "dump.h"
-#define MAKEFLAGS ".MAKEFLAGS"
-
static LIST to_create; /* Targets to be made */
Lst create = &to_create;
bool allPrecious; /* .PRECIOUS given on line by itself */
@@ -77,6 +76,7 @@ static bool noBuiltins; /* -r flag */
static LIST makefiles; /* ordered list of makefiles to read */
static LIST varstoprint; /* list of variables to print */
int maxJobs; /* -j argument */
+static int jobserverfd = -1; /* -J argument */
bool compatMake; /* -B argument */
static bool forceJobs = false;
int debug; /* -d flag */
@@ -187,7 +187,7 @@ MainParseArgs(int argc, char **argv)
{
int c, optend;
-#define OPTFLAGS "BC:D:I:SV:d:ef:ij:km:npqrst"
+#define OPTFLAGS "BC:D:I:J:SV:d:ef:ij:km:npqrst"
#define OPTLETTERS "BSiknpqrst"
if (pledge("stdio rpath wpath cpath fattr proc exec", NULL) == -1)
@@ -218,6 +218,19 @@ MainParseArgs(int argc, char **argv)
Parse_AddIncludeDir(optarg);
record_option(c, optarg);
break;
+ case 'J': {
+ const char *errstr;
+
+ jobserverfd = strtonum(optarg, STDERR_FILENO + 1,
+ INT_MAX, &errstr);
+ if (errstr != NULL)
+ errx(1, "illegal argument -J%s: "
+ "fd number is %s", optarg, errstr);
+ if (fcntl(jobserverfd, F_GETFD, 0) < 0)
+ err(1, "illegal argument -J%d", jobserverfd);
+ record_option(c, optarg);
+ break;
+ }
case 'V':
Lst_AtEnd(&varstoprint, optarg);
record_option(c, optarg);
@@ -723,6 +736,17 @@ main(int argc, char **argv)
if (!forceJobs)
compatMake = true;
+ Job_Init(maxJobs, jobserverfd);
+ if (jobserverfd == -1) {
+ char optstr[16];
+
+ /* We are jobserver master; add -J for children */
+ if (snprintf(optstr, sizeof(optstr), "%d",
+ jobserver_get_slave_fd()) < 0)
+ err(1, "could not create -J option string");
+ record_option('J', optstr);
+ }
+
/* And set up everything for sub-makes */
Var_AddCmdline(MAKEFLAGS);
@@ -796,7 +820,6 @@ main(int argc, char **argv)
else
Targ_FindList(&targs, create);
- Job_Init(maxJobs);
/* If the user has defined a .BEGIN target, execute the commands
* attached to it. */
if (!queryFlag)
diff --git a/usr.bin/make/main.h b/usr.bin/make/main.h
index 469487ee058..41a74af9d0d 100644
--- a/usr.bin/make/main.h
+++ b/usr.bin/make/main.h
@@ -29,6 +29,9 @@
/* main
* User interface to make.
*/
+
+#define MAKEFLAGS ".MAKEFLAGS"
+
/* Main_ParseArgLine(string);
* Parse string as a line of arguments, and treats them as if they
* were given at make's invocation. Used to implement .MFLAGS. */
--
2.23.0
--
Lauri Tirkkonen | lotheac @ IRCnet
