On Sat, Sep 14, 2002 at 12:17:53PM +0200, Poul-Henning Kamp wrote:
>
> This is just to note that I have updated the JKH page with a lot of new
> stuff, so if your coding-pencil itches:
>
> http://people.freebsd.org/~phk/TODO/
|Make -j improvement
|
|make(1) with -j option uses a select loop to wait for events, and every
|100msec it drops out to look for processes exited etc. A pure "make
|buildworld" on a single-CPU machine is up to 25% faster that the best
|"make -j N buildworld" time on the same hardware. Changing to timeout
|to be 10msec improves things about 10%.
|I think that make(1) should use kqueue(2) instead, since that would
|eliminate the need for timeouts.
Ok, here's what I came up with. However, with the patch applied, each
'make buildworld' on a SMP machine throws tons of
/freebsd/current/src/sys/vm/uma_core.c:1307: could sleep with "filedesc structure"
locked from /freebsd/current/src/sys/kern/kern_event.c:959
at me and freezes badly at some point (no breaking into ddb possible).
This is totally repeatable. Is anybody able to reproduce (and maybe
fix) this?
Regards,
Stefan Farfeleder
Index: job.c
===================================================================
RCS file: /home/ncvs/src/usr.bin/make/job.c,v
retrieving revision 1.43
diff -u -r1.43 job.c
--- job.c 29 Sep 2002 00:20:28 -0000 1.43
+++ job.c 2 Oct 2002 21:34:51 -0000
@@ -64,9 +64,8 @@
* Job_CatchOutput Print any output our children have produced.
* Should also be called fairly frequently to
* keep the user informed of what's going on.
- * If no output is waiting, it will block for
- * a time given by the SEL_* constants, below,
- * or until output is ready.
+ * If no output is waiting, it will block until
+ * a child terminates or until output is ready.
*
* Job_Init Called to intialize this module. in addition,
* any commands attached to the .BEGIN target
@@ -107,6 +106,8 @@
#include <sys/file.h>
#include <sys/time.h>
#include <sys/wait.h>
+#include <sys/event.h>
+#include <sys/time.h>
#include <err.h>
#include <errno.h>
#include <fcntl.h>
@@ -237,8 +238,7 @@
* (2) a job can only be run locally, but
* nLocal equals maxLocal */
#ifndef RMT_WILL_WATCH
-static fd_set outputs; /* Set of descriptors of pipes connected to
- * the output channels of children */
+static int kqfd; /* File descriptor obtained by kqueue() */
#endif
STATIC GNode *lastNode; /* The node for which output was most recently
@@ -692,8 +692,6 @@
if (usePipes) {
#ifdef RMT_WILL_WATCH
Rmt_Ignore(job->inPipe);
-#else
- FD_CLR(job->inPipe, &outputs);
#endif
if (job->outPipe != job->inPipe) {
(void) close(job->outPipe);
@@ -1265,14 +1263,25 @@
/*
* The first time a job is run for a node, we set the current
* position in the buffer to the beginning and mark another
- * stream to watch in the outputs mask
+ * stream to watch in the outputs mask. The termination of this
+ * job and the availability of new data in the pipe are
+ * registered in the kqueue.
*/
+ struct kevent kev[2];
+
job->curPos = 0;
#ifdef RMT_WILL_WATCH
Rmt_Watch(job->inPipe, JobLocalInput, job);
#else
- FD_SET(job->inPipe, &outputs);
+ EV_SET(&kev[0], job->inPipe, EVFILT_READ, EV_ADD, 0, 0, job);
+ EV_SET(&kev[1], job->pid, EVFILT_PROC, EV_ADD | EV_ONESHOT,
+ NOTE_EXIT, 0, NULL);
+ if (kevent(kqfd, kev, 2, NULL, 0, NULL) != 0) {
+ /* kevent() will fail if the job is already finished */
+ if (errno != EBADF && errno != ESRCH)
+ Punt("kevent: %s", strerror(errno));
+ }
#endif /* RMT_WILL_WATCH */
}
@@ -2229,12 +2238,12 @@
Job_CatchOutput()
{
int nfds;
- struct timeval timeout;
- fd_set readfds;
- LstNode ln;
- Job *job;
#ifdef RMT_WILL_WATCH
int pnJobs; /* Previous nJobs */
+#else
+ struct kevent kev[4]; /* 4 is somewhat arbitrary */
+ size_t kevsize = sizeof(kev) / sizeof(kev[0]);
+ int i;
#endif
(void) fflush(stdout);
@@ -2262,25 +2271,20 @@
}
#else
if (usePipes) {
- readfds = outputs;
- timeout.tv_sec = SEL_SEC;
- timeout.tv_usec = SEL_USEC;
-
- if ((nfds = select(FD_SETSIZE, &readfds, (fd_set *) 0,
- (fd_set *) 0, &timeout)) <= 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 ((nfds = kevent(kqfd, NULL, 0, kev, kevsize, NULL)) == -1) {
+ Punt("kevent: %s", strerror(errno));
+ } else {
+ for (i = 0; i < nfds; i++) {
+ switch (kev[i].filter) {
+ case EVFILT_READ:
+ JobDoOutput(kev[i].udata, FALSE);
+ break;
+ case EVFILT_PROC:
+ /* Just wake up and let Job_CatchChildren() collect the
+ * terminated job. */
+ break;
}
}
- Lst_Close(jobs);
}
}
#endif /* RMT_WILL_WATCH */
@@ -2408,6 +2412,13 @@
}
if (signal(SIGWINCH, SIG_IGN) != SIG_IGN) {
(void) signal(SIGWINCH, JobPassSig);
+ }
+#endif
+
+#ifndef RMT_WILL_WATCH
+
+ if ((kqfd = kqueue()) == -1) {
+ Punt("kqueue: %s", strerror(errno));
}
#endif
Index: job.h
===================================================================
RCS file: /home/ncvs/src/usr.bin/make/job.h,v
retrieving revision 1.18
diff -u -r1.18 job.h
--- job.h 26 Sep 2002 01:39:22 -0000 1.18
+++ job.h 1 Oct 2002 16:31:10 -0000
@@ -50,14 +50,6 @@
#define TMPPAT "/tmp/makeXXXXXXXXXX"
-/*
- * The SEL_ constants determine the maximum amount of time spent in select
- * before coming out to see if a child has finished. SEL_SEC is the number of
- * seconds and SEL_USEC is the number of micro-seconds
- */
-#define SEL_SEC 0
-#define SEL_USEC 100000
-
/*-
* Job Table definitions.
