[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] Windows jobserver updates for GNU make 4.1
From: |
Eli Zaretskii |
Subject: |
Re: [PATCH] Windows jobserver updates for GNU make 4.1 |
Date: |
Sat, 27 Feb 2016 11:00:48 +0200 |
> From: Troy Runkel <address@hidden>
> Date: Wed, 24 Feb 2016 17:27:57 +0000
> Cc: Troy Runkel <address@hidden>
>
> This patch expands the maximum number of simultaneous jobs on Windows from 63
> (limit dictated by
> Windows MAXIMUM_WAIT_OBJECTS) to 4095.
>
> It also fixes a situation where GNU make on Windows could deadlock and/or
> suffer memory corruption issues
> if –j or –j63 was used. The problem was due to the way that $(shell …)
> commands are handled.
Thanks.
I wonder how hard would it be to remove the limitation altogether?
If we are lifting the limitation, why not lift it entirely?
Another comment is about the busy-wait loop (with 10-msec sleep) that
this change uses -- it looks like, when there are 4095 jobs running,
we will be re-checking each process once every 640 msec, is that
right? That might be too large a delay, I think. Did you consider a
design that uses a separate thread for watching up to 64 processes? I
think such a design might scale up better; in particular, the response
time for checking the status of each job will not decrease as the
number of jobs goes up.
> +DWORD GmakeWaitForMultipleObjects(
> + DWORD nCount,
> + const HANDLE *lpHandles,
> + BOOL bWaitAll,
> + DWORD dwMilliseconds
> +)
> +{
> + assert(nCount <= GMAKE_MAXIMUM_WAIT_OBJECTS);
> +
> + if (nCount <= MAXIMUM_WAIT_OBJECTS) {
> + DWORD retVal = WaitForMultipleObjects(nCount, lpHandles, bWaitAll,
> dwMilliseconds);
> + return (retVal == WAIT_TIMEOUT) ? GMAKE_WAIT_TIMEOUT : retVal;
This GMAKE_WAIT_TIMEOUT and GMAKE_WAIT_ABANDONED_0 stuff looks like a
kludge to me; can we get rid of it?
> + switch (retVal) {
> + case WAIT_TIMEOUT:
> + retVal = GMAKE_WAIT_TIMEOUT;
> + continue;
> + break;
> + case WAIT_FAILED:
> + fprintf(stderr,"WaitForMultipleOjbects failed waiting with error
> %d\n", GetLastError());
> + break;
I guess this fprintf should go away, or be converted to a DB call?
Thank you for working on this.