From owner-freebsd-current@FreeBSD.ORG Thu Nov 11 23:36:56 2004 Return-Path: Delivered-To: freebsd-current@freebsd.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id 07DB616A4CE for ; Thu, 11 Nov 2004 23:36:56 +0000 (GMT) Received: from critter.freebsd.dk (critter.freebsd.dk [212.242.86.163]) by mx1.FreeBSD.org (Postfix) with ESMTP id 1890D43D2F for ; Thu, 11 Nov 2004 23:36:55 +0000 (GMT) (envelope-from phk@critter.freebsd.dk) Received: from critter.freebsd.dk (localhost [127.0.0.1]) by critter.freebsd.dk (8.13.1/8.13.1) with ESMTP id iABNarvn085043 for ; Fri, 12 Nov 2004 00:36:53 +0100 (CET) (envelope-from phk@critter.freebsd.dk) From: "Poul-Henning Kamp" In-Reply-To: Your message of "Thu, 11 Nov 2004 20:42:21 +0100." <80546.1100202141@critter.freebsd.dk> Date: Fri, 12 Nov 2004 00:36:53 +0100 Message-ID: <85042.1100216213@critter.freebsd.dk> Sender: phk@critter.freebsd.dk cc: current@freebsd.org Subject: Re: [TEST] make -j patch [take 2] X-BeenThere: freebsd-current@freebsd.org X-Mailman-Version: 2.1.1 Precedence: list List-Id: Discussions about the use of FreeBSD-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 11 Nov 2004 23:36:56 -0000 In message <80546.1100202141@critter.freebsd.dk>, Poul-Henning Kamp writes: Here is take two of my "make -j" patch. Further testing found a couple of buglets. If this survices further testing, it will be committed in a couple of days. With this patch "make -j N" will put the load average as close to N as the makefiles will allow. Included is also a patch I've been using to make the SUBDIR targets go parallel. Ruslan will have to fix all the mistakes I've made in that one before it gets committed. Poul-Henning Index: job.c =================================================================== RCS file: /home/ncvs/src/usr.bin/make/job.c,v retrieving revision 1.55 diff -u -r1.55 job.c --- job.c 11 Nov 2004 12:52:15 -0000 1.55 +++ job.c 11 Nov 2004 23:30:36 -0000 @@ -258,6 +258,10 @@ * jobs that were stopped due to concurrency * limits or externally */ +STATIC int fifoFd; /* Fd of our job fifo */ +STATIC char fifoName[] = "/tmp/make_fifo_XXXXXXXXX"; +STATIC int fifoTokens; /* How many we got */ +STATIC int fifoMaster; #if defined(USE_PGRP) && defined(SYSV) # define KILL(pid, sig) killpg(-(pid), (sig)) @@ -1091,6 +1095,8 @@ Punt("Cannot fork"); } else if (cpid == 0) { + if (fifoFd >= 0) + close(fifoFd); /* * Must duplicate the input stream down to the child's input and * reset it to the beginning (again). Since the stream was marked @@ -1891,9 +1897,10 @@ return; } - while ((pid = waitpid((pid_t) -1, &status, - (block?0:WNOHANG)|WUNTRACED)) > 0) - { + for (;;) { + pid = waitpid((pid_t) -1, &status, (block?0:WNOHANG)|WUNTRACED); + if (pid <= 0) + break; DEBUGF(JOB, ("Process %d exited or stopped.\n", pid)); jnode = Lst_Find(jobs, (void *)&pid, JobCmpPid); @@ -1915,8 +1922,18 @@ job = (Job *) Lst_Datum(jnode); (void) Lst_Remove(jobs, jnode); nJobs -= 1; - DEBUGF(JOB, ("Job queue is no longer full.\n")); - jobFull = FALSE; + if (fifoFd >= 0 && maxJobs > 1) { + fifoTokens--; + write(fifoFd, "+", 1); + maxJobs--; + if (nJobs >= maxJobs) + jobFull = TRUE; + else + jobFull = FALSE; + } else { + DEBUGF(JOB, ("Job queue is no longer full.\n")); + jobFull = FALSE; + } } JobFinish(job, &status); @@ -1940,7 +1957,7 @@ * ----------------------------------------------------------------------- */ void -Job_CatchOutput(void) +Job_CatchOutput(int flag) { int nfds; #ifdef USE_KQUEUE @@ -1982,23 +1999,28 @@ readfds = outputs; timeout.tv_sec = SEL_SEC; timeout.tv_usec = SEL_USEC; + if (flag && jobFull && fifoFd >= 0) + FD_SET(fifoFd, &readfds); - if ((nfds = select(FD_SETSIZE, &readfds, (fd_set *) 0, - (fd_set *) 0, &timeout)) <= 0) + nfds = select(FD_SETSIZE, &readfds, (fd_set *) 0, + (fd_set *) 0, &timeout); + if (nfds <= 0) return; - else { - if (Lst_Open(jobs) == FAILURE) { - Punt("Cannot open job table"); - } - while (nfds && (ln = Lst_Next(jobs)) != NULL) { - job = (Job *) Lst_Datum(ln); - if (FD_ISSET(job->inPipe, &readfds)) { - JobDoOutput(job, FALSE); - nfds -= 1; - } + if (fifoFd >= 0 && FD_ISSET(fifoFd, &readfds)) { + if (--nfds <= 0) + return; + } + if (Lst_Open(jobs) == FAILURE) { + Punt("Cannot open job table"); + } + while (nfds && (ln = Lst_Next(jobs)) != NULL) { + job = (Job *) Lst_Datum(ln); + if (FD_ISSET(job->inPipe, &readfds)) { + JobDoOutput(job, FALSE); + nfds -= 1; } - Lst_Close(jobs); } + Lst_Close(jobs); #endif /* !USE_KQUEUE */ } } @@ -2055,19 +2077,66 @@ Job_Init(int maxproc) { GNode *begin; /* node for commands to do at the very start */ + const char *env; + fifoFd = -1; jobs = Lst_Init(FALSE); stoppedJobs = Lst_Init(FALSE); - maxJobs = maxproc; + env = getenv("MAKE_JOBS_FIFO"); + + if (env == NULL && maxproc > 1) { + /* + * We did not find the environment variable so we are the leader. + * Create the fifo, open it, write one char per allowed job into + * the pipe. + */ + mktemp(fifoName); + if (!mkfifo(fifoName, 0600)) { + fifoFd = open(fifoName, O_RDWR | O_NONBLOCK, 0); + if (fifoFd >= 0) { + fifoMaster = 1; + fcntl(fifoFd, F_SETFL, O_NONBLOCK); + env = fifoName; + setenv("MAKE_JOBS_FIFO", env, 1); + while (maxproc-- > 0) { + write(fifoFd, "+", 1); + } + /*The master make does not get a magic token */ + jobFull = TRUE; + maxJobs = 0; + } else { + unlink(fifoName); + env = NULL; + } + } + } else if (env != NULL) { + /* + * We had the environment variable so we are a slave. + * Open fifo and give ourselves a magic token which represents + * the token our parent make has grabbed to start his make process. + * Otherwise the sub-makes would gobble up tokens and the proper + * number of tokens to specify to -j would depend on the depth of * the tree and the order of execution. + */ + fifoFd = open(env, O_RDWR, 0); + if (fifoFd >= 0) { + fcntl(fifoFd, F_SETFL, O_NONBLOCK); + maxJobs = 1; + jobFull = FALSE; + } + } + if (fifoFd <= 0) { + maxJobs = maxproc; + jobFull = FALSE; + } else { + } nJobs = 0; - jobFull = FALSE; aborting = 0; errors = 0; lastNode = NULL; - if (maxJobs == 1 || beVerbose == 0) { + if ((maxJobs == 1 && fifoFd < 0) || beVerbose == 0) { /* * If only one job can run at a time, there's no need for a banner, * no is there? @@ -2133,7 +2202,7 @@ if (begin != NULL) { JobStart(begin, JOB_SPECIAL, (Job *)0); while (nJobs) { - Job_CatchOutput(); + Job_CatchOutput(0); Job_CatchChildren(!usePipes); } } @@ -2157,7 +2226,20 @@ Boolean Job_Full(void) { - return(aborting || jobFull); + char c; + int i; + + if (aborting) + return(aborting); + if (fifoFd >= 0 && jobFull) { + i = read(fifoFd, &c, 1); + if (i > 0) { + fifoTokens++; + maxJobs++; + jobFull = FALSE; + } + } + return(jobFull); } /*- @@ -2453,7 +2535,7 @@ JobStart(interrupt, JOB_IGNDOTS, (Job *)0); while (nJobs) { - Job_CatchOutput(); + Job_CatchOutput(0); Job_CatchChildren(!usePipes); } } @@ -2480,11 +2562,19 @@ JobStart(postCommands, JOB_SPECIAL | JOB_IGNDOTS, NULL); while (nJobs) { - Job_CatchOutput(); + Job_CatchOutput(0); Job_CatchChildren(!usePipes); } } } + if (fifoFd >= 0) { + while (fifoTokens-- > 0) + write(fifoFd, "+", 1); + close(fifoFd); + fifoFd = -1; + if (fifoMaster) + unlink(fifoName); + } return(errors); } @@ -2507,7 +2597,7 @@ { aborting = ABORT_WAIT; while (nJobs != 0) { - Job_CatchOutput(); + Job_CatchOutput(0); Job_CatchChildren(!usePipes); } aborting = 0; Index: job.h =================================================================== RCS file: /home/ncvs/src/usr.bin/make/job.h,v retrieving revision 1.25 diff -u -r1.25 job.h --- job.h 11 Nov 2004 12:52:16 -0000 1.25 +++ job.h 11 Nov 2004 23:08:33 -0000 @@ -209,7 +209,7 @@ void Job_Touch(GNode *, Boolean); Boolean Job_CheckCommands(GNode *, void (*abortProc)(const char *, ...)); void Job_CatchChildren(Boolean); -void Job_CatchOutput(void); +void Job_CatchOutput(int flag); void Job_Make(GNode *); void Job_Init(int); Boolean Job_Full(void); Index: main.c =================================================================== RCS file: /home/ncvs/src/usr.bin/make/main.c,v retrieving revision 1.95 diff -u -r1.95 main.c --- main.c 11 Nov 2004 12:52:16 -0000 1.95 +++ main.c 11 Nov 2004 22:19:54 -0000 @@ -678,6 +678,8 @@ Var_Set(".CURDIR", curdir, VAR_GLOBAL); Var_Set(".OBJDIR", objdir, VAR_GLOBAL); + if (getenv("MAKE_JOBS_FIFO") != NULL) + forceJobs = TRUE; /* * Be compatible if user did not specify -j and did not explicitly * turned compatibility on @@ -863,6 +865,7 @@ * Compat_Init will take care of creating all the targets as * well as initializing the module. */ + Job_Init(maxJobs); Compat_Run(targs); outOfDate = 0; } Index: make.c =================================================================== RCS file: /home/ncvs/src/usr.bin/make/make.c,v retrieving revision 1.24 diff -u -r1.24 make.c --- make.c 23 Oct 2002 23:16:43 -0000 1.24 +++ make.c 11 Nov 2004 23:08:17 -0000 @@ -637,7 +637,7 @@ { GNode *gn; - while (!Job_Full() && !Lst_IsEmpty (toBeMade)) { + while (!Lst_IsEmpty (toBeMade) && !Job_Full()) { gn = (GNode *) Lst_DeQueue (toBeMade); DEBUGF(MAKE, ("Examining %s...", gn->name)); /* @@ -840,7 +840,7 @@ * keepgoing flag was given. */ while (!Job_Empty ()) { - Job_CatchOutput (); + Job_CatchOutput (!Lst_IsEmpty (toBeMade)); Job_CatchChildren (!usePipes); (void)MakeStartJobs(); } Index: bsd.subdir.mk =================================================================== RCS file: /home/ncvs/src/share/mk/bsd.subdir.mk,v retrieving revision 1.47 diff -u -r1.47 bsd.subdir.mk --- bsd.subdir.mk 7 Sep 2004 15:27:10 -0000 1.47 +++ bsd.subdir.mk 11 Nov 2004 22:05:13 -0000 @@ -40,21 +40,20 @@ .endfor .endif -_SUBDIR: .USE +_SUBDIR: .USE .if defined(SUBDIR) && !empty(SUBDIR) && !defined(NO_SUBDIR) - @${_+_}for entry in ${SUBDIR}; do \ - if test -d ${.CURDIR}/$${entry}.${MACHINE_ARCH}; then \ - ${ECHODIR} "===> ${DIRPRFX}$${entry}.${MACHINE_ARCH} (${.TARGET:realinstall=install})"; \ - edir=$${entry}.${MACHINE_ARCH}; \ - cd ${.CURDIR}/$${edir}; \ - else \ - ${ECHODIR} "===> ${DIRPRFX}$$entry (${.TARGET:realinstall=install})"; \ - edir=$${entry}; \ - cd ${.CURDIR}/$${edir}; \ - fi; \ - ${MAKE} ${.TARGET:realinstall=install} \ - DIRPRFX=${DIRPRFX}$$edir/; \ - done +.for entry in ${SUBDIR} + if test -d ${.CURDIR}/${entry}.${MACHINE_ARCH}; then \ + ${ECHODIR} "===> ${DIRPRFX}${entry}.${MACHINE_ARCH} (${.TARGET:realinstall=install})"; \ + edir=${entry}.${MACHINE_ARCH}; \ + cd ${.CURDIR}/${edir}; \ + else \ + ${ECHODIR} "===> ${DIRPRFX}${entry} (${.TARGET:realinstall=install})"; \ + edir=${entry}; \ + cd ${.CURDIR}/$${edir}; \ + fi; \ + ${MAKE} ${.TARGET:realinstall=install} DIRPRFX=${DIRPRFX}$$edir/; +.endfor .endif ${SUBDIR}:: -- Poul-Henning Kamp | UNIX since Zilog Zeus 3.20 phk@FreeBSD.ORG | TCP/IP since RFC 956 FreeBSD committer | BSD since 4.3-tahoe Never attribute to malice what can adequately be explained by incompetence.