Re: [TEST] make -j patch [take 2]

From: Poul-Henning Kamp <phk_at_phk.freebsd.dk>
Date: Fri, 12 Nov 2004 00:36:53 +0100
In message <80546.1100202141_at_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
_at__at_ -258,6 +258,10 _at__at_
 				 * 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))
_at__at_ -1091,6 +1095,8 _at__at_
 	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
_at__at_ -1891,9 +1897,10 _at__at_
 	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);
_at__at_ -1915,8 +1922,18 _at__at_
 	    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);
_at__at_ -1940,7 +1957,7 _at__at_
  * -----------------------------------------------------------------------
  */
 void
-Job_CatchOutput(void)
+Job_CatchOutput(int flag)
 {
     int           	  nfds;
 #ifdef USE_KQUEUE
_at__at_ -1982,23 +1999,28 _at__at_
 	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 */
     }
 }
_at__at_ -2055,19 +2077,66 _at__at_
 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?
_at__at_ -2133,7 +2202,7 _at__at_
     if (begin != NULL) {
 	JobStart(begin, JOB_SPECIAL, (Job *)0);
 	while (nJobs) {
-	    Job_CatchOutput();
+	    Job_CatchOutput(0);
 	    Job_CatchChildren(!usePipes);
 	}
     }
_at__at_ -2157,7 +2226,20 _at__at_
 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);
 }
 
 /*-
_at__at_ -2453,7 +2535,7 _at__at_
 
 	    JobStart(interrupt, JOB_IGNDOTS, (Job *)0);
 	    while (nJobs) {
-		Job_CatchOutput();
+		Job_CatchOutput(0);
 		Job_CatchChildren(!usePipes);
 	    }
 	}
_at__at_ -2480,11 +2562,19 _at__at_
 	    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);
 }
 
_at__at_ -2507,7 +2597,7 _at__at_
 {
     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
_at__at_ -209,7 +209,7 _at__at_
 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
_at__at_ -678,6 +678,8 _at__at_
 	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
_at__at_ -863,6 +865,7 _at__at_
 			 * 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
_at__at_ -637,7 +637,7 _at__at_
 {
     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));
 	/*
_at__at_ -840,7 +840,7 _at__at_
      * 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
_at__at_ -40,21 +40,20 _at__at_
 .endfor
 .endif
 
-_SUBDIR: .USE
+_SUBDIR: .USE 
 .if defined(SUBDIR) && !empty(SUBDIR) && !defined(NO_SUBDIR)
-	_at_${_+_}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_at_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.
Received on Thu Nov 11 2004 - 22:36:56 UTC

This archive was generated by hypermail 2.4.0 : Wed May 19 2021 - 11:38:21 UTC