Skip to content
108 changes: 89 additions & 19 deletions build.c
Original file line number Diff line number Diff line change
Expand Up @@ -28,11 +28,12 @@ struct job {
bool failed;
};

struct buildoptions buildopts = {.maxfail = 1};
struct buildoptions buildopts = {.maxfail = 1, .gmakepipe = {-1, -1} };
static struct edge *work;
static size_t nstarted, nfinished, ntotal;
static bool consoleused;
static struct timespec starttime;
static char gmaketokens[512], *gmakelatest = gmaketokens;

void
buildreset(void)
Expand All @@ -43,6 +44,13 @@ buildreset(void)
e->flags &= ~FLAG_WORK;
}

/* returns whether GNU make jobserver mode is enabled */
static inline bool
isjobserverclient(void)
{
return buildopts.gmakepipe[0] >= 0 && buildopts.gmakepipe[1] >= 0;
}

/* returns whether n1 is newer than n2, or false if n1 is NULL */
static bool
isnewer(struct node *n1, struct node *n2)
Expand Down Expand Up @@ -521,44 +529,92 @@ jobwork(struct job *j)
}

/* queries the system load average */
static double
queryload(void)
static bool
reachedload(void)
{
#ifdef HAVE_GETLOADAVG
double load;

if (getloadavg(&load, 1) == -1) {
warn("getloadavg:");
load = 100.0;
if (buildopts.maxload > 0) {
if (getloadavg(&load, 1) == -1) {
warn("getloadavg:");
return true;
}
return load > buildopts.maxload;
} else {
return false;
}

return load;
#else
return 0;
return false;
#endif
}

/* returns remaining GNU/tokens */
static ssize_t
returngmake(void)
{
ssize_t returned = 0;
if (isjobserverclient())
if ((returned = write(buildopts.gmakepipe[1], gmaketokens, gmakelatest - gmaketokens)) >= 0)
gmakelatest = gmaketokens;
return returned;
}

static void
gmakeatexit(void)
{
if (returngmake() < 0)
warn("last write to jobserver:");
}

static void
termsignal(int signum)
{
write(2, "terminating due to signal\n", 27);
(void)returngmake();
_exit(128 + signum);
}

void
build(void)
{
struct sigaction sact = { 0 };
struct job *jobs = NULL;
struct pollfd *fds = NULL;
size_t i, next = 0, jobslen = 0, maxjobs = buildopts.maxjobs, numjobs = 0, numfail = 0;
ssize_t gmakeread;
struct edge *e;

if (ntotal == 0) {
warn("nothing to do");
return;
}

fds = xmalloc(sizeof(fds[0]));
fds[0].fd = buildopts.gmakepipe[0];
fds[0].events = 0;
if (isjobserverclient()) {
fds[0].events |= POLLIN;
if (atexit(gmakeatexit) == 0) {
sact.sa_handler = termsignal;
sigaction(SIGTERM, &sact, NULL);
sigaction(SIGINT, &sact, NULL);
sigaction(SIGPIPE, &sact, NULL);
} else {
warn("unable to register an atexit() function");
}
}

clock_gettime(CLOCK_MONOTONIC, &starttime);
formatstatus(NULL, 0);

nstarted = 0;
for (;;) {
/* limit number of of jobs based on load */
if (buildopts.maxload)
maxjobs = queryload() > buildopts.maxload ? 1 : buildopts.maxjobs;
maxjobs = reachedload() ? 1 : buildopts.maxjobs;
/* limit number of jobs based on gmake tokens */
if (isjobserverclient())
maxjobs = maxjobs > 1 + (size_t)(gmakelatest - gmaketokens) ? 1 + (size_t)(gmakelatest - gmaketokens) : maxjobs;
/* start ready edges */
while (work && numjobs < maxjobs && numfail < buildopts.maxfail) {
e = work;
Expand All @@ -578,18 +634,19 @@ build(void)
if (jobslen > buildopts.maxjobs)
jobslen = buildopts.maxjobs;
jobs = xreallocarray(jobs, jobslen, sizeof(jobs[0]));
fds = xreallocarray(fds, jobslen, sizeof(fds[0]));
fds = xreallocarray(fds, 1 + jobslen, sizeof(fds[0]));
for (i = next; i < jobslen; ++i) {
jobs[i].buf.data = NULL;
jobs[i].buf.len = 0;
jobs[i].buf.cap = 0;
jobs[i].next = i + 1;
fds[i].fd = -1;
fds[i].events = POLLIN;
fds[1+i].fd = -1;
fds[1+i].events = POLLIN;
}
}
fds[next].fd = jobstart(&jobs[next], e);
if (fds[next].fd < 0) {

fds[1+next].fd = jobstart(&jobs[next], e);
if (fds[1+next].fd < 0) {
warn("job failed to start");
++numfail;
} else {
Expand All @@ -599,17 +656,30 @@ build(void)
}
if (numjobs == 0)
break;
if (poll(fds, jobslen, 5000) < 0)
if (poll(fds, 1 + jobslen, 5000) < 0)
fatal("poll:");
for (i = 0; i < jobslen; ++i) {
if (!fds[i].revents || jobwork(&jobs[i]))
if (!fds[1+i].revents || jobwork(&jobs[i]))
continue;
--numjobs;
jobs[i].next = next;
fds[i].fd = -1;
fds[1+i].fd = -1;
next = i;
if (jobs[i].failed)
++numfail;
/* return a token */
if (isjobserverclient() && gmakelatest > gmaketokens) {
if (write(buildopts.gmakepipe[1], --gmakelatest, 1) < 0)
warn("write to jobserver:");
}
}
if (fds[0].revents & POLLIN && isjobserverclient() && nstarted < ntotal && !reachedload()) {
gmakeread = read(buildopts.gmakepipe[0], gmakelatest, buildopts.maxjobs - numjobs);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is a race condition here:

  • poll returns POLLIN
  • another process steals the token that poll detected
  • read blocks
  • one or more children of samurai finish
  • the whole build cannot reuse the token(s) currently owned by samurai

The solution used by GNU Make is complex and relies on trapping SIGCHLD and dup-ing the file descriptor. It's complicated and it's why I just went for reading the jobserver file descriptor in a separate thread instead.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not even sure GNU Make's solution is 100% robust without an extra (void)alarm(1);, or only starting a new process as an absolute last resort.

I really have to rethink this implementation.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After reading that paper about the GNU make implementation, I'm convinced that we must use threads (or some sort of async I/O) to implement this correctly on samurai. The SIGCHLD/EBADF approach they use is clever, but won't work for us, since we buffer the output from jobs to avoid it getting jumbled.

The main issue is that you cannot predict whether a read on the jobserver fd will block or not. It doesn't matter for make since if it blocks, they have nothing to do anyway until a child exits, which can conveniently interrupt the read via SIGCHLD. samurai follows ninja in that it buffers job output, so to do the same trick we'd need the jobserver read to get interrupted when any of the pipes to jobs become readable.

Because of this, I'm going to close this PR in favor of #104. Thanks for your work getting started on this.

if (gmakeread < 0) {
warn("read from jobserver:");
gmakeread = 0;
}
gmakelatest += gmakeread;
}
}
for (i = 0; i < jobslen; ++i)
Expand Down
1 change: 1 addition & 0 deletions build.h
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ struct buildoptions {
_Bool verbose, explain, keepdepfile, keeprsp, dryrun;
const char *statusfmt;
double maxload;
int gmakepipe[2];
};

extern struct buildoptions buildopts;
Expand Down
45 changes: 45 additions & 0 deletions samu.c
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
#include <stdlib.h>
#include <string.h>
#include <unistd.h> /* for chdir */
#include <fcntl.h> /* for open */
#include "arg.h"
#include "build.h"
#include "deps.h"
Expand Down Expand Up @@ -90,6 +91,49 @@ jobsflag(const char *flag)
buildopts.maxjobs = num > 0 ? num : -1;
}

static void
jobserverflags(const char *flag)
{
int rfd, wfd;

if (!flag)
return;
if (sscanf(flag, "%d,%d", &rfd, &wfd) == 2) {
;
} else if (strncmp(flag, "fifo:", 5) == 0) {
if ((rfd = wfd = open(flag + 5, O_RDWR)) == -1)
fatal("unable to open jobserver fifo:");
} else {
fatal("invalid jobserver parameter");
}

buildopts.gmakepipe[0] = rfd;
buildopts.gmakepipe[1] = wfd;
warn("using GNU Make jobserver");
}

static void
parsegmakeflags(char *env) {
char *arg;

if (!env)
return;
env = xmemdup(env, strlen(env) + 1);

arg = strtok(env, " ");
while (arg) {
if (arg[0] && arg[0] != '-' && strchr(arg, 'n')) {
buildopts.dryrun = true;
} else if (strncmp(arg, "-j", 2) == 0) {
jobsflag(arg + 2);
} else if (strncmp(arg, "--jobserver-auth=", 17) == 0) {
jobserverflags(arg + 17);
}
arg = strtok(NULL, " ");
}
free(env);
}

static void
parseenvargs(char *env)
{
Expand Down Expand Up @@ -149,6 +193,7 @@ main(int argc, char *argv[])

argv0 = progname(argv[0], "samu");
parseenvargs(getenv("SAMUFLAGS"));
parsegmakeflags(getenv("MAKEFLAGS"));
ARGBEGIN {
case '-':
arg = EARGF(usage());
Expand Down